[include-cleaner] Don't count references to operators as uses

[hokein]

Summary

Fixes https://github.com/llvm/llvm-project/issues/59655

[sammccall]

Yeah, I think this makes sense.
I think this sort of thing should ideally be captured in the include-cleaner design doc if there was one. (Doesn't make sense to create one just for this issue, but it seems like a doc that should exist).

clang-tools-extra/include-cleaner/lib/WalkAST.cpp

69

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)

71

maybe motivate this a little: (generally the type should provide them)

[hokein]

update

clang-tools-extra/include-cleaner/lib/WalkAST.cpp

69

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).

[sammccall]

clang-tools-extra/include-cleaner/lib/WalkAST.cpp

69

Yeah, sorry I was confused and thought we handled function refs in CallExpr instead of DeclRefExpr

75

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)

clang-tools-extra/include-cleaner/lib/WalkAST.cpp

NamedDecl *getMemberProvider(QualType Base) {
     if (const auto *TST = dyn_cast<TemplateSpecializationType>(Base))
       return resolveTemplateName(TST->getTemplateName());
     return Base->getAsRecordDecl();
   }
 
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
+    // Operators are always ADL extension points, by design references to them
+    // doesn't count as uses. Ignore them by traversing arguments instead of
+    // children.
+    for (auto *Arg : S->arguments())
+      if (!TraverseStmt(Arg))
+        return false;
+    return true;
+  }
+
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
     report(DRE->getLocation(), DRE->getFoundDecl());
     return true;
   }
 
   bool VisitMemberExpr(MemberExpr *E) {
     // Reporting a usage of the member decl would cause issues (e.g. force
     // including the base class for inherited members). Instead, we report a

clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

 
 TEST(WalkAST, ConstructExprs) {
   testWalk("struct $implicit^S {};", "S ^t;");
   testWalk("struct S { $implicit^S(); };", "S ^t;");
   testWalk("struct S { $explicit^S(int); };", "S ^t(42);");
   testWalk("struct S { $implicit^S(int); };", "S t = ^42;");
 }
 
+TEST(WalkAST, Operator) {
+  // References to operators are not counted as uses.
+  testWalk("struct string {}; int operator+(string, string);",
+           "int k = string() ^+ string();");
+  testWalk("struct string {int operator+(string); }; ",
+           "int k = string() ^+ string();");
+  testWalk("struct string { friend int operator+(string, string); }; ",
+           "int k = string() ^+ string();");
+}
+
 TEST(WalkAST, Functions) {
   // Definition uses declaration, not the other way around.
   testWalk("void $explicit^foo();", "void ^foo() {}");
   testWalk("void foo() {}", "void ^foo();");
 
   // Unresolved calls marks all the overloads.
   testWalk("void $ambiguous^foo(int); void $ambiguous^foo(char);",
            "template <typename T> void bar() { ^foo(T{}); }");