diff --git a/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp b/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp --- 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 @@ class AbseilModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck("abseil-cleanup-ctad"); CheckFactories.registerCheck( "abseil-duration-addition"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt b/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h b/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h new file mode 100644 --- /dev/null +++ b/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 diff --git a/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp b/clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp new file mode 100644 --- /dev/null +++ b/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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -76,6 +76,13 @@ New checks ^^^^^^^^^^ +- New :doc:`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 ` 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 --- /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([] {})); + +becomes + +.. code-block:: c++ + + absl::Cleanup c1 = [] {}; + + const absl::Cleanup c2 = std::function([] {}); diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -12,6 +12,7 @@ .. csv-table:: :header: "Name", "Offers fixes" + `abseil-cleanup-ctad `_, "Yes" `abseil-duration-addition `_, "Yes" `abseil-duration-comparison `_, "Yes" `abseil-duration-conversion-cast `_, "Yes" @@ -114,13 +115,12 @@ `cert-dcl50-cpp `_, `cert-dcl58-cpp `_, `cert-env33-c `_, + `cert-err33-c `_, `cert-err34-c `_, `cert-err52-cpp `_, `cert-err58-cpp `_, `cert-err60-cpp `_, - `cert-exp42-c `_, `cert-flp30-c `_, - `cert-flp37-c `_, `cert-mem57-cpp `_, `cert-msc50-cpp `_, `cert-msc51-cpp `_, @@ -288,6 +288,7 @@ `readability-const-return-type `_, "Yes" `readability-container-size-empty `_, "Yes" `readability-convert-member-functions-to-static `_, + `readability-data-pointer `_, `readability-delete-null-pointer `_, "Yes" `readability-else-after-return `_, "Yes" `readability-function-cognitive-complexity `_, @@ -333,13 +334,14 @@ `cert-dcl03-c `_, `misc-static-assert `_, "Yes" `cert-dcl16-c `_, `readability-uppercase-literal-suffix `_, "Yes" `cert-dcl37-c `_, `bugprone-reserved-identifier `_, "Yes" - `cert-err33-c `_, `bugprone-unused-return-value `_, `cert-dcl51-cpp `_, `bugprone-reserved-identifier `_, "Yes" `cert-dcl54-cpp `_, `misc-new-delete-overloads `_, `cert-dcl59-cpp `_, `google-build-namespaces `_, `cert-err09-cpp `_, `misc-throw-by-value-catch-by-reference `_, `cert-err61-cpp `_, `misc-throw-by-value-catch-by-reference `_, + `cert-exp42-c `_, `bugprone-suspicious-memory-comparison `_, `cert-fio38-c `_, `misc-non-copyable-objects `_, + `cert-flp37-c `_, `bugprone-suspicious-memory-comparison `_, `cert-msc30-c `_, `cert-msc50-cpp `_, `cert-msc32-c `_, `cert-msc51-cpp `_, `cert-oop11-cpp `_, `performance-move-constructor-init `_, 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 --- /dev/null +++ b/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 +class Cleanup { +public: + Cleanup(T) {} + Cleanup(Cleanup &&) {} +}; + +template +Cleanup(Callback callback) -> Cleanup; + +template +absl::Cleanup MakeCleanup(T t) { + return absl::Cleanup(t); +} + +} // namespace absl + +namespace std { + +template +class function { +public: + template + 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([] {})); + // 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([] {});{{$}} + + auto f = absl::MakeCleanup((std::function([] {}))); + // 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([] {});{{$}} + + const auto g = absl::MakeCleanup(std::function([] {})); + // 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([] {});{{$}} + + const auto h = absl::MakeCleanup((std::function([] {}))); + // 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([] {});{{$}} +}