[clang-tools-extra] 16b07c8 - [clang-tidy] Add check for initialization of `absl::Cleanup`.
Yitzhak Mandelbaum via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 8 07:59:32 PST 2021
Author: CJ Johnson
Date: 2021-11-08T15:57:32Z
New Revision: 16b07c866ae74e0d02038574fe1c37a6cb55e233
URL: https://github.com/llvm/llvm-project/commit/16b07c866ae74e0d02038574fe1c37a6cb55e233
DIFF: https://github.com/llvm/llvm-project/commit/16b07c866ae74e0d02038574fe1c37a6cb55e233.diff
LOG: [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
Added:
clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h
clang-tools-extra/docs/clang-tidy/checks/abseil-cleanup-ctad.rst
clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp
Modified:
clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp b/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp
index 7d592d7e3e55..5d99e6b50075 100644
--- a/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp
+++ b/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>(
diff --git a/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt b/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
index b6ea21879daf..e7c86fc8107d 100644
--- a/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
+++ b/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
diff --git a/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp b/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
new file mode 100644
index 000000000000..bc152f1dafa7
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h b/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h
new file mode 100644
index 000000000000..ce4e5c6be9d8
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 5cd131e18ac8..41934d8fb905 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/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.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/abseil-cleanup-ctad.rst b/clang-tools-extra/docs/clang-tidy/checks/abseil-cleanup-ctad.rst
new file mode 100644
index 000000000000..b8afc8b8a848
--- /dev/null
+++ b/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()>([] {});
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index edb656bc1e48..c9b4daf42ef0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/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"
+
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp b/clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp
new file mode 100644
index 000000000000..c023521bb261
--- /dev/null
+++ b/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()>([] {});{{$}}
+}
More information about the cfe-commits
mailing list