[clang-tidy] Add check for initialization of `absl::Cleanup`.

Suggests switching the initialization pattern of `absl::Cleanup` instances from the factory function to class template argument deduction (CTAD) in C++17 and higher.

Reviewed By: ymandel

Differential Revision: https://reviews.llvm.org/D113195

clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp

@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "CleanupCtadCheck.h"
 #include "DurationAdditionCheck.h"
 #include "DurationComparisonCheck.h"
 #include "DurationConversionCastCheck.h"
@@ -35,6 +36,7 @@ namespace abseil {
 class AbseilModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<CleanupCtadCheck>("abseil-cleanup-ctad");
     CheckFactories.registerCheck<DurationAdditionCheck>(
         "abseil-duration-addition");
     CheckFactories.registerCheck<DurationComparisonCheck>(

clang-tools-extra/clang-tidy/abseil/CMakeLists.txt

@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
 
 add_clang_library(clangTidyAbseilModule
   AbseilTidyModule.cpp
+  CleanupCtadCheck.cpp
   DurationAdditionCheck.cpp
   DurationComparisonCheck.cpp
   DurationConversionCastCheck.cpp

clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp

@@ -0,0 +1,49 @@
+//===--- CleanupCtadCheck.cpp - clang-tidy --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CleanupCtadCheck.h"
+#include "../utils/TransformerClangTidyCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
+#include "clang/Tooling/Transformer/Stencil.h"
+#include "llvm/ADT/StringRef.h"
+
+using namespace ::clang::ast_matchers;
+using namespace ::clang::transformer;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+RewriteRule CleanupCtadCheckImpl() {
+  auto warning_message = cat("prefer absl::Cleanup's class template argument "
+                             "deduction pattern in C++17 and higher");
+
+  return makeRule(
+      declStmt(has(varDecl(
+          hasType(autoType()), hasTypeLoc(typeLoc().bind("auto_type_loc")),
+          hasInitializer(traverse(
+              clang::TK_IgnoreUnlessSpelledInSource,
+              callExpr(callee(functionDecl(hasName("absl::MakeCleanup"))),
+                       argumentCountIs(1),
+                       hasArgument(0, expr().bind("make_cleanup_argument")))
+                  .bind("make_cleanup_call")))))),
+      {changeTo(node("auto_type_loc"), cat("absl::Cleanup")),
+       changeTo(node("make_cleanup_call"), cat(node("make_cleanup_argument")))},
+      warning_message);
+}
+
+CleanupCtadCheck::CleanupCtadCheck(StringRef Name, ClangTidyContext *Context)
+    : utils::TransformerClangTidyCheck(CleanupCtadCheckImpl(), Name, Context) {}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h

@@ -0,0 +1,37 @@
+//===--- CleanupCtadCheck.h - clang-tidy ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_CLEANUPCTADCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_CLEANUPCTADCHECK_H
+
+#include "../utils/TransformerClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Suggests switching the initialization pattern of `absl::Cleanup`
+/// instances from the factory function to class template argument
+/// deduction (CTAD), in C++17 and higher.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-cleanup-ctad.html
+class CleanupCtadCheck : public utils::TransformerClangTidyCheck {
+public:
+  CleanupCtadCheck(StringRef Name, ClangTidyContext *Context);
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus17;
+  }
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_CLEANUPCTADCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -76,6 +76,13 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`abseil-cleanup-ctad
+  <clang-tidy/checks/abseil-cleanup-ctad>` check.
+
+  Suggests switching the initialization pattern of ``absl::Cleanup``
+  instances from the factory function to class template argument
+  deduction (CTAD), in C++17 and higher.
+
 - New :doc:`bugprone-suspicious-memory-comparison
   <clang-tidy/checks/bugprone-suspicious-memory-comparison>` check.
 

clang-tools-extra/docs/clang-tidy/checks/abseil-cleanup-ctad.rst

@@ -0,0 +1,22 @@
+.. title:: clang-tidy - abseil-cleanup-ctad
+
+abseil-cleanup-ctad
+===================
+
+Suggests switching the initialization pattern of ``absl::Cleanup``
+instances from the factory function to class template argument
+deduction (CTAD), in C++17 and higher.
+
+.. code-block:: c++
+
+  auto c1 = absl::MakeCleanup([] {});
+
+  const auto c2 = absl::MakeCleanup(std::function<void()>([] {}));
+
+becomes
+
+.. code-block:: c++
+
+  absl::Cleanup c1 = [] {};
+
+  const absl::Cleanup c2 = std::function<void()>([] {});

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -12,6 +12,7 @@ Clang-Tidy Checks
 .. csv-table::
    :header: "Name", "Offers fixes"
 
+   `abseil-cleanup-ctad <abseil-cleanup-ctad.html>`_, "Yes"
    `abseil-duration-addition <abseil-duration-addition.html>`_, "Yes"
    `abseil-duration-comparison <abseil-duration-comparison.html>`_, "Yes"
    `abseil-duration-conversion-cast <abseil-duration-conversion-cast.html>`_, "Yes"
@@ -448,3 +449,4 @@ Clang-Tidy Checks
    `hicpp-vararg <hicpp-vararg.html>`_, `cppcoreguidelines-pro-type-vararg <cppcoreguidelines-pro-type-vararg.html>`_,
    `llvm-else-after-return <llvm-else-after-return.html>`_, `readability-else-after-return <readability-else-after-return.html>`_, "Yes"
    `llvm-qualified-auto <llvm-qualified-auto.html>`_, `readability-qualified-auto <readability-qualified-auto.html>`_, "Yes"
+   

clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp

@@ -0,0 +1,115 @@
+// RUN: %check_clang_tidy %s abseil-cleanup-ctad -std=c++17 %t
+
+namespace std {
+
+template <typename, typename>
+struct is_same {
+  static const bool value = false;
+};
+
+template <typename T>
+struct is_same<T, T> { static const bool value = true; };
+
+template <typename>
+class function {
+public:
+  template <typename T>
+  function(T) {}
+  function(const function &) {}
+};
+
+} // namespace std
+
+namespace absl {
+
+namespace cleanup_internal {
+
+struct Tag {};
+
+template <typename Callback>
+class Storage {
+public:
+  Storage() = delete;
+
+  explicit Storage(Callback callback) {}
+
+  Storage(Storage &&other) {}
+
+  Storage(const Storage &other) = delete;
+
+  Storage &operator=(Storage &&other) = delete;
+
+  Storage &operator=(const Storage &other) = delete;
+
+private:
+  bool is_callback_engaged_;
+  alignas(Callback) char callback_buffer_[sizeof(Callback)];
+};
+
+} // namespace cleanup_internal
+
+template <typename Arg, typename Callback = void()>
+class Cleanup final {
+public:
+  Cleanup(Callback callback) // NOLINT
+      : storage_(static_cast<Callback &&>(callback)) {}
+
+  Cleanup(Cleanup &&other) = default;
+
+  void Cancel() &&;
+
+  void Invoke() &&;
+
+  ~Cleanup();
+
+private:
+  cleanup_internal::Storage<Callback> storage_;
+};
+
+template <typename Callback>
+Cleanup(Callback callback) -> Cleanup<cleanup_internal::Tag, Callback>;
+
+template <typename... Args, typename Callback>
+absl::Cleanup<cleanup_internal::Tag, Callback> MakeCleanup(Callback callback) {
+  return {static_cast<Callback &&>(callback)};
+}
+
+} // namespace absl
+
+void test() {
+  auto a = absl::MakeCleanup([] {});
+  // 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(([] {}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup{{.*}}C++17 and higher
+  // CHECK-FIXES: {{^}}  absl::Cleanup b = [] {};{{$}}
+
+  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([] {});
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher
+  // CHECK-FIXES: {{^}}  const absl::Cleanup e = [] {};{{$}}
+
+  // Removes extra parens
+  const auto f = absl::MakeCleanup(([] {}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup{{.*}}C++17 and higher
+  // CHECK-FIXES: {{^}}  const absl::Cleanup f = [] {};{{$}}
+
+  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()>([] {});{{$}}
+
+  // 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()>([] {});{{$}}
+}