[PATCH] D115452: Prevent abseil-cleanup-ctad check from stomping on surrounding context

[CJ Johnson via Phabricator via cfe-commits]

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);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115452.393193.patch
Type: text/x-patch
Size: 4246 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211209/0276fd2e/attachment-0001.bin>