kbobyrev added inline comments.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:140
+// locations for the found macros.
+void findReferencedMacros(ParsedAST &AST, ReferencedLocations &Result) {
+ auto Tokens =
----------------
sammccall wrote:
> sammccall wrote:
> > can you add a trace for this and verify that it's not a signficant amount
> > of time on a macro-heavy file (e.g. unittests?)
> >
> > locateMacroAt wasn't designed to be called in a loop like this, but it
> > seems plausible it'll be fine
> >
> iterating over all the tokens and examining them individually seems like
> we're doing too much work.
>
> ParsedAST.MainFileMacros.Names already knows the names of all macros
> referenced from the main file.
> PP.getLocalMacroDirectiveHistory() will give you the last seen definition for
> a given macro name. If we want, we can also get the previous definitions.
>
> This seems like it should be close enough, and wade through many fewer tokens?
Hmm, this doesn't really work for "unmaterialized" macros, i.e.
`ParsedAST::getMacros().Names` doesn't have `FOO` in this case:
```
// foo.h
#define FOO
// foo.cpp
#define BAR FOO
```
Is that a bug in `CollectMainFileMacros` we want to fix or is this intended
behavior and we'd need to iterate through the tokens?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112447/new/
https://reviews.llvm.org/D112447
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
