[PATCH] D146279: [clangd] Extend CollectMainFileMacros.

[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!



================
Comment at: clang-tools-extra/clangd/CollectMacros.h:48
 public:
-  explicit CollectMainFileMacros(const SourceManager &SM, MainFileMacros &Out)
-      : SM(SM), Out(Out) {}
+  explicit CollectMainFileMacros(const SourceManager &SM,
+                                 const Preprocessor &PP, MainFileMacros &Out)
----------------
nit: you can get SM out of PP, no need to pass both.


================
Comment at: clang-tools-extra/clangd/CollectMacros.h:70
 
   void Ifdef(SourceLocation Loc, const Token &MacroName,
              const MacroDefinition &MD) override {
----------------
nit: can you actually move macro bodies out-of-line ? it's unfortunate that we need to build half of the project everytime we touch this header.


================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:615
+      std::make_unique<CollectMainFileMacros>(
+          Clang->getSourceManager(), Clang->getPreprocessor(), Macros));
 
----------------
nit: we seem to be calling `Clang->getPreprocessor()` in many places, maybe extract it into a variable instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146279