sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:75
+ // doesn't count as uses (generally the type should provide them).
+ // Ignore them by traversing arguments instead of children.
+ for (auto *Arg : S->arguments())
----------------
This comment doesn't mention the callee at all, and the callee is the point.
Maybe: "Don't traverse the callee." and add a newline before the arguments()
loop?
(because this comment isn't really related to that part, arguments() is just
part of the "everything else" that we're still doing)
================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:69
+ bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
+ // Operators are always ADL extension points, by design references to them
----------------
hokein wrote:
> sammccall wrote:
> > Rather than override TraverseCXXOperatorCallExpr to do everything *except*
> > call WalkUpFrom, maybe override WalkUpFromCXXOperatorCallExpr to call
> > VisitCXXOperatorCallExpr but not WalkUpFromCallExpr? (With a comment that
> > we don't want to treat operators as calls)
> >
> > Rather than override TraverseCXXOperatorCallExpr to do everything *except*
> > call WalkUpFrom.
>
> Ah, not really (it is my fault that I missed to call `WalkUpFrom`, but it is
> not the key point) .
> The key point is to traverse every child of `CXXOperatorCallExpr` *except*
> the operator. E.g. for the AST node below, we don't want to traverse its
> first child which is the operator.
>
> ```
> -CXXOperatorCallExpr 0x5591a97f5280 <col:9, col:27> 'int' '+'
> |-ImplicitCastExpr 0x5591a97f5268 <col:18> 'int (*)(string, string)'
> <FunctionToPointerDecay>
> | `-DeclRefExpr 0x5591a97f51e8 <col:18> 'int (string, string)' lvalue
> Function 0x5591a97f4878 'operator+' 'int (string, string)'
> |-CXXTemporaryObjectExpr 0x5591a97f5068 <col:9, col:16> 'string':'string'
> 'void () noexcept' zeroing
> `-CXXTemporaryObjectExpr 0x5591a97f51b8 <col:20, col:27>
> ```
>
> I added the `WalkUpFromCXXOperatorCallExpr` back (I think it doesn't have any
> effect on the current code because we don't do anything on the
> `CXXOperatorCallExpr` hierarchy classes).
Yeah, sorry I was confused and thought we handled function refs in CallExpr
instead of DeclRefExpr
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140551/new/
https://reviews.llvm.org/D140551
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
