kadircet added inline comments.
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:55
+ bool VisitMemberExpr(MemberExpr *E) {
+ auto *MD = E->getMemberDecl();
+ report(E->getMemberLoc(), MD);
----------------
sammccall wrote:
> sammccall wrote:
> > nit: inline?
> should this be the founddecl instead of the memberdecl?
right, forgot about these. also added a test.
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:66
+ bool VisitOverloadExpr(OverloadExpr *E) {
+ // Mark all candidates as used when overload is not resolved.
+ llvm::for_each(E->decls(),
----------------
sammccall wrote:
> sammccall wrote:
> > I think we need a policy knob for this, to decide whether to over or
> > underestimate.
> > This would be the wrong behavior for missing-includes analysis.
> comment echoes the code, say why instead
i agree that this needs a knob. it's just unclear at which level currently, i
am putting together a doc to have a better decision here (mostly to post vs pre
filter).
i'd rather move forward with this version, to prepare grounds for the tidy
check and clangd usage based on this library, and address these issues in a new
iteration.
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:72
+
+ bool VisitUsingDecl(UsingDecl *UD) {
+ for (const auto *Shadow : UD->shadows())
----------------
sammccall wrote:
> I wonder if this is correct enough.
>
> This brings a set of overloads into scope, the intention may be to bring a
> particular overload with others as harmless side effects: consider `using
> std::swap`.
> In this case, inserting includes for all the overloads that happened to be
> visible would be too much.
>
> Possible behaviors:
> - what we do here
> - only do this if overestimate=true
> - if overestimate=false, only include those USDs marked as `referenced` (not
> sure if they actually get marked appropriately, the bit exists)
i agree, basically the same things as I mentioned above, we should definitely
have a way to filter these.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132110/new/
https://reviews.llvm.org/D132110
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
