ilya-biryukov added inline comments.
================
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:107
+class CollectMainFileMacroExpansions : public PPCallbacks {
+ const SourceManager &SM;
----------------
jvikstrom wrote:
> ilya-biryukov wrote:
> > Maybe make this part of `CollectMainFileMacros`? It looks like a natural
> > fit there and we won't need another instance of `PPCallbacks`.
> But `CollectMainFileMacros` is only used for `buildPreamble`.
> I think this one needs to run in `ParsedAST::build`?
>
> Is it safe to add a `CollectMainFileMacros in `ParsedAST::build`?
Ah, good point, LG then.
Could you put a small comment beside the class that explicitly mentions this?
It's obvious when reading through the whole code, but not obvious when peeking
at different parts of it from a distance.
Something like
```
// CollectMainFileMacros and CollectMainFileMacroExpansions are two different
classes because
// the latter is only used when building preamble and the former only when
building the AST for the main file.
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66928/new/
https://reviews.llvm.org/D66928
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
