[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

2023-04-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG26ff268b80c5: [include-cleaner] Report references to 
operator calls as implicit (authored by kadircet).

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(CD)) {
+// Treat this as a regular member reference.
+report(S->getOperatorLoc(), 
getMemberProvider(S->getArg(0)->getType()));
+  } else {
+report(S->getOperatorLoc(), llvm::dyn_cast(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, 

[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

2023-04-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.

still LG.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147144

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

2023-04-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
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(CD)) {
+// Treat this as a regular member reference.
+report(S->getOperatorLoc(), 
getMemberProvider(S->getArg(0)->getType()));
+  } else {
+report(S->getOperatorLoc(), llvm::dyn_cast(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
+  // 

[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

2023-03-29 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:98
   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.
-
+report(S->getOperatorLoc(), llvm::cast(S->getCalleeDecl()),
+   RefType::Implicit);

I will be more defensive for nullptr (llvm::cast doesn't allow nullptr), use 
the `cast_if_present` in case where `S->getCalleeDecl()` returns a nullptr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147144

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit

2023-03-29 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Missing these references can result in false negatives in the used-ness
analysis and break builds.


Repository:
  rG LLVM Github Monorepo

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
@@ -292,12 +292,13 @@
 
 TEST(WalkAST, Operator) {
   // References to operators are not counted as uses.
-  testWalk("struct string {}; int operator+(string, string);",
+  testWalk("struct string {}; int $implicit^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); }; ",
+  testWalk("struct string {int $implicit^operator+(string); }; ",
"int k = string() ^+ string();");
+  testWalk(
+  "struct string { friend int $implicit^operator+(string, string); }; ",
+  "int k = string() ^+ string();");
 }
 
 TEST(WalkAST, Functions) {
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
@@ -87,14 +87,16 @@
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  // Operators are always ADL extension points, by design references to them
+  // doesn't count as uses (generally the type should provide them). Hence
+  // treat these as implicit references.
+  // We do this in traverse to make sure rest of the visitor doesn't see
+  // operator calls.
   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.
-
+report(S->getOperatorLoc(), llvm::cast(S->getCalleeDecl()),
+   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
@@ -292,12 +292,13 @@
 
 TEST(WalkAST, Operator) {
   // References to operators are not counted as uses.
-  testWalk("struct string {}; int operator+(string, string);",
+  testWalk("struct string {}; int $implicit^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); }; ",
+  testWalk("struct string {int $implicit^operator+(string); }; ",
"int k = string() ^+ string();");
+  testWalk(
+  "struct string { friend int $implicit^operator+(string, string); }; ",
+  "int k = string() ^+ string();");
 }
 
 TEST(WalkAST, Functions) {
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
@@ -87,14 +87,16 @@
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  // Operators are always ADL extension points, by design references to them
+  // doesn't count as uses (generally the type should provide them). Hence
+  // treat these as implicit references.
+  // We do this in traverse to make sure rest of the visitor doesn't see
+  // operator calls.
   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.
-
+report(S->getOperatorLoc(), llvm::cast(S->getCalleeDecl()),
+   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