[PATCH] D80198: [clangd] locateMacroAt handles patched macros

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Looks great, ship it!



================
Comment at: clang-tools-extra/clangd/Preamble.cpp:154
+    OS << "\\\n" << std::string(TargetColumn, ' ');
+    // Decrement because returned string has a line break before directive now.
+    --DirectiveLine;
----------------
nit: not "before the directive", just before the DirectiveRange.begin()


================
Comment at: clang-tools-extra/clangd/Preamble.cpp:157
+  } else {
+    // There is enough space for Prefix and space before directive, use it.
+    // We try to squeeze the Prefix into the same line whenever we can, as
----------------
nit: I think it'd be easier to understand the other case if this one came first, as it's the fallback


================
Comment at: clang-tools-extra/clangd/Preamble.h:136
+/// using presumed locations. Returns \p Loc if it isn't inside preamble patch.
+SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
+                                              const SourceManager &SM);
----------------
Can we have a unit-test for this function?
I think the minimal thing (least entangled with PreamblePatch construction) is to manually write a snippet that looks like a preamble patch like
```
# line 3 main.cpp
int foo();
```
 map it into a TestTU with AdditionalFiles, set `ExtraArgs = {"-include","patch.h"}` and then get the location with `findDecl`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80198/new/

https://reviews.llvm.org/D80198