diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -19,6 +19,7 @@ RawStringLiteralCheck.cpp RedundantVoidArgCheck.cpp ReplaceAutoPtrCheck.cpp + ReplaceDisallowCopyAndAssignMacroCheck.cpp ReplaceRandomShuffleCheck.cpp ReturnBracedInitListCheck.cpp ShrinkToFitCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -21,6 +21,7 @@ #include "RawStringLiteralCheck.h" #include "RedundantVoidArgCheck.h" #include "ReplaceAutoPtrCheck.h" +#include "ReplaceDisallowCopyAndAssignMacroCheck.h" #include "ReplaceRandomShuffleCheck.h" #include "ReturnBracedInitListCheck.h" #include "ShrinkToFitCheck.h" @@ -67,6 +68,8 @@ "modernize-redundant-void-arg"); CheckFactories.registerCheck( "modernize-replace-auto-ptr"); + CheckFactories.registerCheck( + "modernize-replace-disallow-copy-and-assign-macro"); CheckFactories.registerCheck( "modernize-replace-random-shuffle"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h b/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h @@ -0,0 +1,59 @@ +//===--- ReplaceDisallowCopyAndAssignMacroCheck.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_MODERNIZE_REPLACEDISALLOWCOPYANDASSIGNMACROCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACEDISALLOWCOPYANDASSIGNMACROCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace modernize { + +/// This check finds macro expansions of ``DISALLOW_COPY_AND_ASSIGN(Type)`` and +/// replaces them with a deleted copy constructor and a deleted assignment +/// operator. +/// +/// Before: +/// ~~~{.cpp} +/// class Foo { +/// private: +/// DISALLOW_COPY_AND_ASSIGN(Foo); +/// }; +/// ~~~ +/// +/// After: +/// ~~~{.cpp} +/// class Foo { +/// private: +/// Foo(const Foo &) = delete; +/// const Foo &operator=(const Foo &) = delete; +/// }; +/// ~~~ +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.html +class ReplaceDisallowCopyAndAssignMacroCheck : public ClangTidyCheck { +public: + ReplaceDisallowCopyAndAssignMacroCheck(StringRef Name, + ClangTidyContext *Context); + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + + const std::string MacroName; +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REPLACEDISALLOWCOPYANDASSIGNMACROCHECK_H diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp @@ -0,0 +1,92 @@ +//===--- ReplaceDisallowCopyAndAssignMacroCheck.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 "ReplaceDisallowCopyAndAssignMacroCheck.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/MacroArgs.h" +#include "llvm/Support/FormatVariadic.h" + +namespace clang { +namespace tidy { +namespace modernize { + +namespace { + +class ReplaceDisallowCopyAndAssignMacroCallbacks : public PPCallbacks { +public: + explicit ReplaceDisallowCopyAndAssignMacroCallbacks( + ReplaceDisallowCopyAndAssignMacroCheck &Check, Preprocessor &PP, + const SourceManager &SM) + : Check(Check), PP(PP), SM(SM) {} + + void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, + SourceRange Range, const MacroArgs *Args) override { + IdentifierInfo *Info = MacroNameTok.getIdentifierInfo(); + if (!Info || !Args || Args->getNumMacroArguments() != 1) + return; + if (Info->getNameStart() != Check.MacroName) + return; + // The first argument to the DISALLOW_COPY_AND_ASSIGN macro is exptected to + // be the class name. + const Token *ClassNameTok = Args->getUnexpArgument(0); + if (Args->ArgNeedsPreexpansion(ClassNameTok, PP)) + // For now we only support simple argument that don't need to be + // pre-expanded. + return; + clang::IdentifierInfo *ClassIdent = ClassNameTok->getIdentifierInfo(); + if (!ClassIdent) + return; + + // FIXME: Maybe someday I will know how to expand the macro and not use my + // pre-written code snippet. But for now, this is okay. + std::string Replacement = llvm::formatv( + R"cpp({0}(const {0} &) = delete; +const {0} &operator=(const {0} &) = delete{1})cpp", + ClassIdent->getNameStart(), shouldAppendSemi(Range) ? ";" : ""); + + Check.diag(MacroNameTok.getLocation(), "using copy and assign macro '%0'") + << Check.MacroName + << FixItHint::CreateReplacement( + PP.getSourceManager().getExpansionRange(Range), Replacement); + } + +private: + /// \returns \c true if the next token after the given \a MacroLoc is \b not a + /// semicolon. + bool shouldAppendSemi(SourceRange MacroLoc) { + llvm::Optional Next = + Lexer::findNextToken(MacroLoc.getEnd(), SM, PP.getLangOpts()); + return !(Next && Next->is(tok::semi)); + } + + ReplaceDisallowCopyAndAssignMacroCheck &Check; + Preprocessor &PP; + const SourceManager &SM; +}; +} // namespace + +ReplaceDisallowCopyAndAssignMacroCheck::ReplaceDisallowCopyAndAssignMacroCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + MacroName(Options.get("MacroName", "DISALLOW_COPY_AND_ASSIGN")) {} + +void ReplaceDisallowCopyAndAssignMacroCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks( + ::std::make_unique( + *this, *ModuleExpanderPP, SM)); +} + +void ReplaceDisallowCopyAndAssignMacroCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "MacroName", MacroName); +} + +} // namespace modernize +} // 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 @@ -129,6 +129,12 @@ Finds includes of system libc headers not provided by the compiler within llvm-libc implementations. +- New :doc:`modernize-replace-disallow-copy-and-assign-macro + ` check. + + Finds macro expansions of ``DISALLOW_COPY_AND_ASSIGN`` and replaces them with + a deleted copy constructor and a deleted assignment operator. + - New :doc:`objc-dealloc-in-category ` check. 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 @@ -217,6 +217,7 @@ `modernize-raw-string-literal `_, "Yes" `modernize-redundant-void-arg `_, "Yes" `modernize-replace-auto-ptr `_, "Yes" + `modernize-replace-disallow-copy-and-assign-macro `_, "Yes" `modernize-replace-random-shuffle `_, "Yes" `modernize-return-braced-init-list `_, "Yes" `modernize-shrink-to-fit `_, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst @@ -0,0 +1,52 @@ +.. title:: clang-tidy - modernize-replace-disallow-copy-and-assign-macro + +modernize-replace-disallow-copy-and-assign-macro +================================================ + +Finds macro expansions of ``DISALLOW_COPY_AND_ASSIGN(Type)`` and replaces them +with a deleted copy constructor and a deleted assignment operator. + +Before the ``delete`` keyword was introduced in C++11 it was common practice to +declare a copy constructor and an assignment operator as a private members. This +effectively makes them unusable to the public API of a class. + +With the advent of the ``delete`` keyword in C++11 we can abandon the +``private`` access of the copy constructor and the assignment operator and +delete the methods entirely. + +When running this check on a code like this + +.. code-block:: c++ + + class Foo { + private: + DISALLOW_COPY_AND_ASSIGN(Foo); + }; + +It will be transformed to this + +.. code-block:: c++ + + class Foo { + private: + Foo(const Foo &) = delete; + const Foo &operator=(const Foo &) = delete; + }; + +Known Limitations +----------------- + +* Notice that the migration example above leaves the ``private`` access + specification untouched. You might want to run the check :doc:`modernize-use-equals-delete + ` to get warnings for deleted + functions in private sections. + +Options +------- + +.. option:: MacroName + + A string specifying the macro name whose expansion will be replaced. + Default is `DISALLOW_COPY_AND_ASSIGN`. + +See: https://en.cppreference.com/w/cpp/language/function#Deleted_functions diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp @@ -0,0 +1,79 @@ +// RUN: %check_clang_tidy -format-style=LLVM -check-suffix=DEFAULT %s \ +// RUN: modernize-replace-disallow-copy-and-assign-macro %t + +// RUN: %check_clang_tidy -format-style=LLVM -check-suffix=DIFFERENT-NAME %s \ +// RUN: modernize-replace-disallow-copy-and-assign-macro %t \ +// RUN: -config="{CheckOptions: [ \ +// RUN: {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, \ +// RUN: value: MY_MACRO_NAME}]}" + +// RUN: %check_clang_tidy -format-style=LLVM -check-suffix=FINALIZE %s \ +// RUN: modernize-replace-disallow-copy-and-assign-macro %t \ +// RUN: -config="{CheckOptions: [ \ +// RUN: {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, \ +// RUN: value: DISALLOW_COPY_AND_ASSIGN_FINALIZE}]}" + +// RUN: clang-tidy %s -checks="-*,modernize-replace-disallow-copy-and-assign-macro" \ +// RUN: -config="{CheckOptions: [ \ +// RUN: {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, \ +// RUN: value: DISALLOW_COPY_AND_ASSIGN_MORE_AGUMENTS}]}" | count 0 + +// RUN: clang-tidy %s -checks="-*,modernize-replace-disallow-copy-and-assign-macro" \ +// RUN: -config="{CheckOptions: [ \ +// RUN: {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, \ +// RUN: value: DISALLOW_COPY_AND_ASSIGN_NEEDS_PREEXPANSION}]}" | count 0 + +// Note: the last two tests expect no diagnostics, but FileCheck cannot handle +// that, hence the use of | count 0. + +#define DISALLOW_COPY_AND_ASSIGN(TypeName) + +class TestClass1 { +private: + DISALLOW_COPY_AND_ASSIGN(TestClass1); +}; +// CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:3: warning: using copy and assign macro 'DISALLOW_COPY_AND_ASSIGN' [modernize-replace-disallow-copy-and-assign-macro] +// CHECK-FIXES-DEFAULT: {{^}} TestClass1(const TestClass1 &) = delete;{{$}} +// CHECK-FIXES-DEFAULT-NEXT: {{^}} const TestClass1 &operator=(const TestClass1 &) = delete;{{$}} + +#define MY_MACRO_NAME(TypeName) + +class TestClass2 { +private: + MY_MACRO_NAME(TestClass2); +}; +// CHECK-MESSAGES-DIFFERENT-NAME: :[[@LINE-2]]:3: warning: using copy and assign macro 'MY_MACRO_NAME' [modernize-replace-disallow-copy-and-assign-macro] +// CHECK-FIXES-DIFFERENT-NAME: {{^}} TestClass2(const TestClass2 &) = delete;{{$}} +// CHECK-FIXES-DIFFERENT-NAME-NEXT: {{^}} const TestClass2 &operator=(const TestClass2 &) = delete;{{$}} + +#define DISALLOW_COPY_AND_ASSIGN_FINALIZE(TypeName) \ + TypeName(const TypeName &) = delete; \ + const TypeName &operator=(const TypeName &) = delete; + +class TestClass3 { +private: + // Notice, that the macro allows to be used without a semicolon because the + // macro definition already contains one above. Therefore our replacement must + // contain a semicolon at the end. + DISALLOW_COPY_AND_ASSIGN_FINALIZE(TestClass3) +}; +// CHECK-MESSAGES-FINALIZE: :[[@LINE-2]]:3: warning: using copy and assign macro 'DISALLOW_COPY_AND_ASSIGN_FINALIZE' [modernize-replace-disallow-copy-and-assign-macro] +// CHECK-FIXES-FINALIZE: {{^}} TestClass3(const TestClass3 &) = delete;{{$}} +// CHECK-FIXES-FINALIZE-NEXT: {{^}} const TestClass3 &operator=(const TestClass3 &) = delete;{{$}} + +#define DISALLOW_COPY_AND_ASSIGN_MORE_AGUMENTS(A, B) + +class TestClass4 { +private: + DISALLOW_COPY_AND_ASSIGN_MORE_AGUMENTS(TestClass4, TestClass4); +}; +// CHECK-MESSAGES-MORE-ARGUMENTS-NOT: warning: using copy and assign macro 'DISALLOW_COPY_AND_ASSIGN_MORE_AGUMENTS' + +#define DISALLOW_COPY_AND_ASSIGN_NEEDS_PREEXPANSION(A) +#define TESTCLASS TestClass5 + +class TestClass5 { +private: + DISALLOW_COPY_AND_ASSIGN_NEEDS_PREEXPANSION(TESTCLASS); +}; +// CHECK-MESSAGES-MORE-ARGUMENTS-NOT: warning: using copy and assign macro 'DISALLOW_COPY_AND_ASSIGN_NEEDS_PREEXPANSION'