[PATCH] D147144: [include-cleaner] Report references to operator calls as implicit
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
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
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
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
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