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,60 @@ +//===--- 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; + + std::string MacroName; + bool FinalizeWithSemicolon; +}; + +} // 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,96 @@ +//===--- 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" + +namespace clang { +namespace tidy { +namespace modernize { + +namespace { +class ReplaceDisallowCopyAndAssignMacroCallbacks : public PPCallbacks { +public: + explicit ReplaceDisallowCopyAndAssignMacroCallbacks( + ClangTidyCheck &Check, Preprocessor &PP, const SourceManager &SM, + const std::string &MacroName, const bool FinalizeWithSemicolon) + : Check(Check), PP(PP), SM(SM), MacroName(MacroName), + FinalizeWithSemicolon(FinalizeWithSemicolon) {} + + void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, + SourceRange Range, const MacroArgs *Args) override { + IdentifierInfo *identifierInfo = MacroNameTok.getIdentifierInfo(); + if (!identifierInfo) + return; + if (!Args) + return; + if (Args->getNumMacroArguments() != 1) + return; + if (std::string(identifierInfo->getNameStart()) != 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; + std::string className(classIdent->getNameStart()); + + // 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 = + className + "(const " + className + " &) = delete;"; + Replacement += "const " + className + " &operator=(const " + className + + " &) = delete"; + // FIXME: The `FinalizeWithSemicolon` option (see below) maybe could be + // automated. + if (FinalizeWithSemicolon) + Replacement += ";"; + + Check.diag(MacroNameTok.getLocation(), "using copy and assign macro '%0'") + << MacroName + << FixItHint::CreateReplacement( + PP.getSourceManager().getExpansionRange(Range), Replacement); + } + + ClangTidyCheck &Check; + Preprocessor &PP; + const SourceManager &SM; + + const std::string MacroName; + const bool FinalizeWithSemicolon; +}; +} // namespace + +ReplaceDisallowCopyAndAssignMacroCheck::ReplaceDisallowCopyAndAssignMacroCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + MacroName(Options.get("MacroName", "DISALLOW_COPY_AND_ASSIGN")), + FinalizeWithSemicolon(Options.get("FinalizeWithSemicolon", false)) {} + +void ReplaceDisallowCopyAndAssignMacroCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks( + ::std::make_unique( + *this, *ModuleExpanderPP, SM, MacroName, FinalizeWithSemicolon)); +} + +void ReplaceDisallowCopyAndAssignMacroCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "MacroName", MacroName); + Options.store(Opts, "FinalizeWithSemicolon", FinalizeWithSemicolon); +} + +} // 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,46 @@ +.. 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. + +Migration example: + +.. code-block:: c++ + + class Foo { + private: + - DISALLOW_COPY_AND_ASSIGN(Foo); + + Foo(const Foo &) = delete; + + const Foo &operator=(const Foo &) = delete; + }; + +Known Limitations +----------------- +* Notice that the migration example above leaves the ``private`` access + specification untouched. This opens room for improvement, yes I know. + +Options +------- + +.. option:: MacroName + + A string specifying the macro name whose expansion will be replaced. + Default is `DISALLOW_COPY_AND_ASSIGN`. + +.. option:: FinalizeWithSemicolon + + A boolean value that tells the replacement to put a semicolon at the end or + not. Default is `false`. + +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,81 @@ +// RUN: %check_clang_tidy -format-style=LLVM -check-suffix=DEFAULT %s modernize-replace-disallow-copy-and-assign-macro %t + +// RUN: %check_clang_tidy -format-style=LLVM -check-suffix=DIFFERENT-NAME %s 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 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: {key: modernize-replace-disallow-copy-and-assign-macro.FinalizeWithSemicolon, \ +// RUN: value: 1}]}" + +// 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: :32:3: warning: using copy and assign macro 'DISALLOW_COPY_AND_ASSIGN' [modernize-replace-disallow-copy-and-assign-macro] +// CHECK-MESSAGES-NEXT-DEFAULT: {{^}} DISALLOW_COPY_AND_ASSIGN(TestClass1);{{$}} +// CHECK-MESSAGES-NEXT-DEFAULT: {{^}} ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~{{$}} +// CHECK-MESSAGES-NEXT-DEFAULT: {{^}} TestClass1(const TestClass1 &) = delete;const TestClass1 &operator=(const TestClass1 &) = delete{{$}} +// 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: :45: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: :60: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'