CJ-Johnson created this revision.
CJ-Johnson added a reviewer: ymandel.
Herald added subscribers: carlosgalvezp, mgorny.
CJ-Johnson requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Suggests switching the initialization pattern of absl::Cleanup instances from
the factory function to class template argument deduction (CTAD) in C++17 and
higher.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D113195
Files:
clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/abseil-cleanup-ctad.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/abseil-cleanup-ctad.cpp
@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s abseil-cleanup-ctad -std=c++17 %t
+
+namespace absl {
+
+struct Tag {};
+
+template <typename, typename T>
+class Cleanup {
+public:
+ Cleanup(T) {}
+ Cleanup(Cleanup &&) {}
+};
+
+template <typename Callback>
+Cleanup(Callback callback) -> Cleanup<Tag, Callback>;
+
+template <typename T>
+absl::Cleanup<Tag, T> MakeCleanup(T t) {
+ return absl::Cleanup<Tag, T>(t);
+}
+
+} // namespace absl
+
+namespace std {
+
+template <typename>
+class function {
+public:
+ template <typename T>
+ function(T) {}
+ function(const function &) {}
+};
+
+} // namespace std
+
+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 = [] {};{{$}}
+
+ auto b = 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 b = [] {};{{$}}
+
+ const auto c = absl::MakeCleanup([] {});
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup's class template argument deduction pattern in C++17 and higher
+ // CHECK-FIXES: {{^}} const absl::Cleanup c = [] {};{{$}}
+
+ const auto d = absl::MakeCleanup(([] {}));
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup's class template argument deduction pattern in C++17 and higher
+ // CHECK-FIXES: {{^}} const absl::Cleanup d = [] {};{{$}}
+
+ auto e = absl::MakeCleanup(std::function<void()>([] {}));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup's class template argument deduction pattern in C++17 and higher
+ // CHECK-FIXES: {{^}} absl::Cleanup e = std::function<void()>([] {});{{$}}
+
+ auto f = absl::MakeCleanup((std::function<void()>([] {})));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer absl::Cleanup's class template argument deduction pattern in C++17 and higher
+ // CHECK-FIXES: {{^}} absl::Cleanup f = std::function<void()>([] {});{{$}}
+
+ const auto g = absl::MakeCleanup(std::function<void()>([] {}));
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup's class template argument deduction pattern in C++17 and higher
+ // CHECK-FIXES: {{^}} const absl::Cleanup g = std::function<void()>([] {});{{$}}
+
+ const auto h = absl::MakeCleanup((std::function<void()>([] {})));
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: prefer absl::Cleanup's class template argument deduction pattern in C++17 and higher
+ // CHECK-FIXES: {{^}} const absl::Cleanup h = std::function<void()>([] {});{{$}}
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -12,6 +12,7 @@
.. 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"
@@ -114,13 +115,12 @@
`cert-dcl50-cpp <cert-dcl50-cpp.html>`_,
`cert-dcl58-cpp <cert-dcl58-cpp.html>`_,
`cert-env33-c <cert-env33-c.html>`_,
+ `cert-err33-c <cert-err33-c.html>`_,
`cert-err34-c <cert-err34-c.html>`_,
`cert-err52-cpp <cert-err52-cpp.html>`_,
`cert-err58-cpp <cert-err58-cpp.html>`_,
`cert-err60-cpp <cert-err60-cpp.html>`_,
- `cert-exp42-c <cert-exp42-c.html>`_,
`cert-flp30-c <cert-flp30-c.html>`_,
- `cert-flp37-c <cert-flp37-c.html>`_,
`cert-mem57-cpp <cert-mem57-cpp.html>`_,
`cert-msc50-cpp <cert-msc50-cpp.html>`_,
`cert-msc51-cpp <cert-msc51-cpp.html>`_,
@@ -288,6 +288,7 @@
`readability-const-return-type <readability-const-return-type.html>`_, "Yes"
`readability-container-size-empty <readability-container-size-empty.html>`_, "Yes"
`readability-convert-member-functions-to-static <readability-convert-member-functions-to-static.html>`_,
+ `readability-data-pointer <readability-data-pointer.html>`_,
`readability-delete-null-pointer <readability-delete-null-pointer.html>`_, "Yes"
`readability-else-after-return <readability-else-after-return.html>`_, "Yes"
`readability-function-cognitive-complexity <readability-function-cognitive-complexity.html>`_,
@@ -333,13 +334,14 @@
`cert-dcl03-c <cert-dcl03-c.html>`_, `misc-static-assert <misc-static-assert.html>`_, "Yes"
`cert-dcl16-c <cert-dcl16-c.html>`_, `readability-uppercase-literal-suffix <readability-uppercase-literal-suffix.html>`_, "Yes"
`cert-dcl37-c <cert-dcl37-c.html>`_, `bugprone-reserved-identifier <bugprone-reserved-identifier.html>`_, "Yes"
- `cert-err33-c <cert-err33-c.html>`_, `bugprone-unused-return-value <bugprone-unused-return-value.html>`_,
`cert-dcl51-cpp <cert-dcl51-cpp.html>`_, `bugprone-reserved-identifier <bugprone-reserved-identifier.html>`_, "Yes"
`cert-dcl54-cpp <cert-dcl54-cpp.html>`_, `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
`cert-dcl59-cpp <cert-dcl59-cpp.html>`_, `google-build-namespaces <google-build-namespaces.html>`_,
`cert-err09-cpp <cert-err09-cpp.html>`_, `misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
`cert-err61-cpp <cert-err61-cpp.html>`_, `misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
+ `cert-exp42-c <cert-exp42-c.html>`_, `bugprone-suspicious-memory-comparison <bugprone-suspicious-memory-comparison.html>`_,
`cert-fio38-c <cert-fio38-c.html>`_, `misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
+ `cert-flp37-c <cert-flp37-c.html>`_, `bugprone-suspicious-memory-comparison <bugprone-suspicious-memory-comparison.html>`_,
`cert-msc30-c <cert-msc30-c.html>`_, `cert-msc50-cpp <cert-msc50-cpp.html>`_,
`cert-msc32-c <cert-msc32-c.html>`_, `cert-msc51-cpp <cert-msc51-cpp.html>`_,
`cert-oop11-cpp <cert-oop11-cpp.html>`_, `performance-move-constructor-init <performance-move-constructor-init.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/abseil-cleanup-ctad.rst
===================================================================
--- /dev/null
+++ 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()>([] {});
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -76,6 +76,13 @@
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.
Index: clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h
@@ -0,0 +1,34 @@
+//===--- 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;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_CLEANUPCTADCHECK_H
Index: clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp
@@ -0,0 +1,53 @@
+//===--- 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_typeLoc")),
+ hasInitializer(ignoringImpCasts(
+ callExpr(callee(functionDecl(hasName("absl::MakeCleanup"))),
+ argumentCountIs(1),
+ hasArgument(0, expr().bind("make_cleanup_argument")))
+ .bind("make_cleanup_call")))))),
+ {changeTo(node("auto_typeLoc"), 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) {}
+
+bool CleanupCtadCheck::isLanguageVersionSupported(
+ const LangOptions &LangOpts) const {
+ return LangOpts.CPlusPlus17;
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/abseil/CMakeLists.txt
@@ -5,6 +5,7 @@
add_clang_library(clangTidyAbseilModule
AbseilTidyModule.cpp
+ CleanupCtadCheck.cpp
DurationAdditionCheck.cpp
DurationComparisonCheck.cpp
DurationConversionCastCheck.cpp
Index: clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp
+++ 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 @@
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>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
