kadircet added a comment.
Maybe I am missing some context here, but why are we trying to filter out
references outside of the main file?
ATM there's no concept of main file in neither walkUsed, it just receives some
ast nodes for analyzing references to declarations inside these nodes, and the
second use is directly figuring out what macro is referenced at particular
locations, so I believe that filtering shouldn't happen for macros at all.
For decls, I can see some reasoning to filter out references that occur outside
these AST nodes, rather than some notion of main file. Moreover we seem to be
doing it in both users of walkAST, so maybe it would be better to do that there
once instead?
This limiting the references to be under ast nodes behaviour has the nasty
wrinkle of macros defined inside the main file, e.g:
#define FOO foo()
void bar() {
FOO;
}
we should say that `bar` uses `foo` despite the reference location lying
outside of the AST node.
so maybe we should change the mapping policy from "just use spelling locations"
to "use spelling location if it is in the same file as the expansion location
and ignore the reference otherwise"?
================
Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:20
void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
llvm::ArrayRef<SymbolReference> MacroRefs,
----------------
as discussed offline we're introducing this extra constraint on references
being provided, so let's mention that in the documentation as well. apart from
that I agree with rest of the comments from Sam
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138779/new/
https://reviews.llvm.org/D138779
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
