sammccall 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);
----------------
nit: inline?
================
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:
> nit: inline?
should this be the founddecl instead of the memberdecl?
================
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(),
----------------
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 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:
> 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
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:72
+
+ bool VisitUsingDecl(UsingDecl *UD) {
+ for (const auto *Shadow : UD->shadows())
----------------
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)
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:79
+ bool VisitFunctionDecl(FunctionDecl *FD) {
+ // Only mark the declaration from a definition.
+ if (FD->isThisDeclarationADefinition())
----------------
comment echoes the code, say why instead
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:110
+ bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
+ // FIXME: Handle specializations.
+ return handleTemplateName(TL.getTemplateNameLoc(),
----------------
nit: *explicit* specializations?
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
