[PATCH] D153259: [clangd] Store offsets in MacroOccurrence

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!



================
Comment at: clang-tools-extra/clangd/CollectMacros.cpp:37
   Out.Names.insert(Name);
-  auto Range = halfOpenToRange(
-      SM, CharSourceRange::getCharRange(Loc, MacroNameTok.getEndLoc()));
+  size_t Start = SM.getDecomposedSpellingLoc(Loc).second;
+  size_t End = SM.getDecomposedSpellingLoc(MacroNameTok.getEndLoc()).second;
----------------
nit: `SM.getFileOffset`


================
Comment at: clang-tools-extra/clangd/CollectMacros.h:26
 struct MacroOccurrence {
-  // Instead of storing SourceLocation, we have to store the token range because
-  // SourceManager from preamble is not available when we build the AST.
-  Range Rng;
+  // Represents an occurrence in main-file, a half-open range.
+  size_t StartOffset, EndOffset;
----------------
`Represents an occurrence in main-file` sounds like the documentation for the struct not for these fields.
what about `// Half-open range (end offset is exclusive) inside the main file.`. can you also move `EndOffset` to its own line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153259