[PATCH] D94477: [clangd] Add main file macros into the main-file index.

[Sam McCall via Phabricator via cfe-commits]

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

Ah, this is a bit ugly... I'm not really happy with our representation of info extracted from the preprocessor, but that's something to tackle another time.

Thanks for fixing this!



================
Comment at: clang-tools-extra/clangd/CollectMacros.h:31
   // SourceManager from preamble is not available when we build the AST.
-  llvm::DenseMap<SymbolID, std::vector<Range>> MacroRefs;
+  llvm::DenseMap<SymbolID, std::vector<std::pair<Range, bool>>> MacroRefs;
   // Somtimes it is not possible to compute the SymbolID for the Macro, e.g. a
----------------
this pair should be a struct instead - MacroOccurrence?


================
Comment at: clang-tools-extra/clangd/CollectMacros.h:35
   // highlighting.
   std::vector<Range> UnknownMacros;
   // Ranges skipped by the preprocessor due to being inactive.
----------------
this should probably also be MacroOccurrence?


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:398
       R.Location.FileURI = MainFileURI.c_str();
       // FIXME: Add correct RefKind information to MainFileMacros.
+      R.Kind = IsDefinition ? RefKind::Definition : RefKind::Reference;
----------------
this fixme is now addressed


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:405
+        auto StartLoc = sourceLocationInMainFile(SM, Range.start);
+        assert(StartLoc);
+        auto EndLoc = sourceLocationInMainFile(SM, Range.end);
----------------
nit: prefer SourceLocation StartLoc = cantFail(sourceLocationMainFile(...));


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:408
+        assert(EndLoc);
+        S.Name = Lexer::getSourceText(
+            CharSourceRange::getCharRange(SourceRange(*StartLoc, *EndLoc)), SM,
----------------
nit: we have `toSourceCode(SourceRange(StartLoc, EndLoc))` for this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94477