kadircet updated this revision to Diff 510474.
kadircet marked an inline comment as done.
kadircet added a comment.

- Update to treat operator calls to members as "explicit" fater offline 
discussions. This better aligns with what we do for regular member exprs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147144/new/

https://reviews.llvm.org/D147144

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -291,13 +291,18 @@
 }
 
 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); }; ",
+  // Operator calls are marked as implicit references as they're ADL-used and
+  // type should be providing them.
+  testWalk(
+      "struct string { friend int $implicit^operator+(string, string); }; ",
+      "int k = string() ^+ string();");
+  // Unless they're members, we treat them as regular member expr calls.
+  testWalk("struct $explicit^string {int operator+(string); }; ",
            "int k = string() ^+ string();");
+  // Make sure usage is attributed to the alias.
+  testWalk(
+      "struct string {int operator+(string); }; using $explicit^foo = string;",
+      "int k = foo() ^+ string();");
 }
 
 TEST(WalkAST, VarDecls) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -91,14 +91,23 @@
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  // Operators are almost always ADL extension points and by design references
+  // to them doesn't count as uses (generally the type should provide them, so
+  // ignore them).
+  // Unless we're using an operator defined as a member, in such cases treat
+  // this as a regular reference.
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
     if (!WalkUpFromCXXOperatorCallExpr(S))
       return false;
-
-    // Operators are always ADL extension points, by design references to them
-    // doesn't count as uses (generally the type should provide them).
-    // Don't traverse the callee.
-
+    if (auto *CD = S->getCalleeDecl()) {
+      if (llvm::isa<CXXMethodDecl>(CD)) {
+        // Treat this as a regular member reference.
+        report(S->getOperatorLoc(), 
getMemberProvider(S->getArg(0)->getType()));
+      } else {
+        report(S->getOperatorLoc(), llvm::dyn_cast<NamedDecl>(CD),
+               RefType::Implicit);
+      }
+    }
     for (auto *Arg : S->arguments())
       if (!TraverseStmt(Arg))
         return false;


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -291,13 +291,18 @@
 }
 
 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); }; ",
+  // Operator calls are marked as implicit references as they're ADL-used and
+  // type should be providing them.
+  testWalk(
+      "struct string { friend int $implicit^operator+(string, string); }; ",
+      "int k = string() ^+ string();");
+  // Unless they're members, we treat them as regular member expr calls.
+  testWalk("struct $explicit^string {int operator+(string); }; ",
            "int k = string() ^+ string();");
+  // Make sure usage is attributed to the alias.
+  testWalk(
+      "struct string {int operator+(string); }; using $explicit^foo = string;",
+      "int k = foo() ^+ string();");
 }
 
 TEST(WalkAST, VarDecls) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -91,14 +91,23 @@
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  // Operators are almost always ADL extension points and by design references
+  // to them doesn't count as uses (generally the type should provide them, so
+  // ignore them).
+  // Unless we're using an operator defined as a member, in such cases treat
+  // this as a regular reference.
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
     if (!WalkUpFromCXXOperatorCallExpr(S))
       return false;
-
-    // Operators are always ADL extension points, by design references to them
-    // doesn't count as uses (generally the type should provide them).
-    // Don't traverse the callee.
-
+    if (auto *CD = S->getCalleeDecl()) {
+      if (llvm::isa<CXXMethodDecl>(CD)) {
+        // Treat this as a regular member reference.
+        report(S->getOperatorLoc(), getMemberProvider(S->getArg(0)->getType()));
+      } else {
+        report(S->getOperatorLoc(), llvm::dyn_cast<NamedDecl>(CD),
+               RefType::Implicit);
+      }
+    }
     for (auto *Arg : S->arguments())
       if (!TraverseStmt(Arg))
         return false;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to