CJ-Johnson created this revision. CJ-Johnson added a reviewer: ymandel. Herald added a subscriber: carlosgalvezp. CJ-Johnson requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.
This change applies two fixes to the abseil-cleanup-ctad check. It uses hasSingleDecl() to ensure only declStmt()s with one varDecl() are matched (leaving compount declStmt()s unchanged). It also addresses a bug in the handling of comments that surround the absl::MakeCleanup() calls by switching to the callArgs() combinator from Clang Transformer. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D115452 Files: clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp Index: clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp +++ clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp @@ -81,35 +81,25 @@ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup's class template argument deduction pattern in C++17 and higher // CHECK-FIXES: {{^}} absl::Cleanup a = [] {};{{$}} - // Removes extra parens - auto b = absl::MakeCleanup(([] {})); + auto b = absl::MakeCleanup(std::function<void()>([] {})); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} absl::Cleanup b = [] {};{{$}} + // CHECK-FIXES: {{^}} absl::Cleanup b = std::function<void()>([] {});{{$}} - auto c = absl::MakeCleanup(std::function<void()>([] {})); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} absl::Cleanup c = std::function<void()>([] {});{{$}} - - // Removes extra parens - auto d = absl::MakeCleanup((std::function<void()>([] {}))); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} absl::Cleanup d = std::function<void()>([] {});{{$}} - - const auto e = absl::MakeCleanup([] {}); + const auto c = absl::MakeCleanup([] {}); // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} const absl::Cleanup e = [] {};{{$}} + // CHECK-FIXES: {{^}} const absl::Cleanup c = [] {};{{$}} - // Removes extra parens - const auto f = absl::MakeCleanup(([] {})); + const auto d = absl::MakeCleanup(std::function<void()>([] {})); // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} const absl::Cleanup f = [] {};{{$}} + // CHECK-FIXES: {{^}} const absl::Cleanup d = std::function<void()>([] {});{{$}} - const auto g = absl::MakeCleanup(std::function<void()>([] {})); - // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} const absl::Cleanup g = std::function<void()>([] {});{{$}} + // Preserves extra parens + auto e = absl::MakeCleanup(([] {})); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher + // CHECK-FIXES: {{^}} absl::Cleanup e = ([] {});{{$}} - // Removes extra parens - const auto h = absl::MakeCleanup((std::function<void()>([] {}))); - // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} const absl::Cleanup h = std::function<void()>([] {});{{$}} + // Preserves comments + auto f = /* a */ absl::MakeCleanup(/* b */ [] { /* c */ } /* d */) /* e */; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher + // CHECK-FIXES: {{^}} absl::Cleanup f = /* a */ /* b */ [] { /* c */ } /* d */ /* e */;{{$}} } Index: clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp +++ clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp @@ -28,16 +28,14 @@ "deduction pattern in C++17 and higher"); return makeRule( - declStmt(has(varDecl( + declStmt(hasSingleDecl(varDecl( hasType(autoType()), hasTypeLoc(typeLoc().bind("auto_type_loc")), - hasInitializer(traverse( - clang::TK_IgnoreUnlessSpelledInSource, + hasInitializer(hasDescendant( callExpr(callee(functionDecl(hasName("absl::MakeCleanup"))), - argumentCountIs(1), - hasArgument(0, expr().bind("make_cleanup_argument"))) + argumentCountIs(1)) .bind("make_cleanup_call")))))), {changeTo(node("auto_type_loc"), cat("absl::Cleanup")), - changeTo(node("make_cleanup_call"), cat(node("make_cleanup_argument")))}, + changeTo(node("make_cleanup_call"), cat(callArgs("make_cleanup_call")))}, warning_message); }
Index: clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp +++ clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp @@ -81,35 +81,25 @@ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup's class template argument deduction pattern in C++17 and higher // CHECK-FIXES: {{^}} absl::Cleanup a = [] {};{{$}} - // Removes extra parens - auto b = absl::MakeCleanup(([] {})); + auto b = absl::MakeCleanup(std::function<void()>([] {})); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} absl::Cleanup b = [] {};{{$}} + // CHECK-FIXES: {{^}} absl::Cleanup b = std::function<void()>([] {});{{$}} - auto c = absl::MakeCleanup(std::function<void()>([] {})); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} absl::Cleanup c = std::function<void()>([] {});{{$}} - - // Removes extra parens - auto d = absl::MakeCleanup((std::function<void()>([] {}))); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} absl::Cleanup d = std::function<void()>([] {});{{$}} - - const auto e = absl::MakeCleanup([] {}); + const auto c = absl::MakeCleanup([] {}); // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} const absl::Cleanup e = [] {};{{$}} + // CHECK-FIXES: {{^}} const absl::Cleanup c = [] {};{{$}} - // Removes extra parens - const auto f = absl::MakeCleanup(([] {})); + const auto d = absl::MakeCleanup(std::function<void()>([] {})); // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} const absl::Cleanup f = [] {};{{$}} + // CHECK-FIXES: {{^}} const absl::Cleanup d = std::function<void()>([] {});{{$}} - const auto g = absl::MakeCleanup(std::function<void()>([] {})); - // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} const absl::Cleanup g = std::function<void()>([] {});{{$}} + // Preserves extra parens + auto e = absl::MakeCleanup(([] {})); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher + // CHECK-FIXES: {{^}} absl::Cleanup e = ([] {});{{$}} - // Removes extra parens - const auto h = absl::MakeCleanup((std::function<void()>([] {}))); - // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher - // CHECK-FIXES: {{^}} const absl::Cleanup h = std::function<void()>([] {});{{$}} + // Preserves comments + auto f = /* a */ absl::MakeCleanup(/* b */ [] { /* c */ } /* d */) /* e */; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher + // CHECK-FIXES: {{^}} absl::Cleanup f = /* a */ /* b */ [] { /* c */ } /* d */ /* e */;{{$}} } Index: clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp +++ clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp @@ -28,16 +28,14 @@ "deduction pattern in C++17 and higher"); return makeRule( - declStmt(has(varDecl( + declStmt(hasSingleDecl(varDecl( hasType(autoType()), hasTypeLoc(typeLoc().bind("auto_type_loc")), - hasInitializer(traverse( - clang::TK_IgnoreUnlessSpelledInSource, + hasInitializer(hasDescendant( callExpr(callee(functionDecl(hasName("absl::MakeCleanup"))), - argumentCountIs(1), - hasArgument(0, expr().bind("make_cleanup_argument"))) + argumentCountIs(1)) .bind("make_cleanup_call")))))), {changeTo(node("auto_type_loc"), cat("absl::Cleanup")), - changeTo(node("make_cleanup_call"), cat(node("make_cleanup_argument")))}, + changeTo(node("make_cleanup_call"), cat(callArgs("make_cleanup_call")))}, warning_message); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits