This revision was automatically updated to reflect the committed changes. Closed by commit rG16b07c866ae7: [clang-tidy] Add check for initialization of `absl::Cleanup`. (authored by CJ-Johnson, committed by ymandel).
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113195/new/ 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,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()>([] {});{{$}} +} 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" @@ -448,3 +449,4 @@ `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 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,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 Index: clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.cpp =================================================================== --- /dev/null +++ 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 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