[clang-tools-extra] e636e6b - [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro
Konrad Kleine via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 3 13:56:16 PDT 2020
Author: Konrad Kleine
Date: 2020-06-03T16:56:03-04:00
New Revision: e636e6b79ac06b13059e46b49acb4d9de204c75b
URL: https://github.com/llvm/llvm-project/commit/e636e6b79ac06b13059e46b49acb4d9de204c75b
DIFF: https://github.com/llvm/llvm-project/commit/e636e6b79ac06b13059e46b49acb4d9de204c75b.diff
LOG: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro
Summary:
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 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:
```
lang=dif
class Foo {
private:
- DISALLOW_COPY_AND_ASSIGN(Foo);
+ Foo(const Foo &) = delete;
+ const Foo &operator=(const Foo &) = delete;
};
```
Reviewers: alexfh, hokein, aaron.ballman, njames93
Reviewed By: njames93
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, cfe-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D80531
Added:
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst
clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp
Modified:
clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 8fcbfbe40b23..c74c4051ade7 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -19,6 +19,7 @@ add_clang_library(clangTidyModernizeModule
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
index 6280f9c991e8..d9ccd2cd0ad7 100644
--- 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 @@ class ModernizeModule : public ClangTidyModule {
"modernize-redundant-void-arg");
CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
"modernize-replace-auto-ptr");
+ CheckFactories.registerCheck<ReplaceDisallowCopyAndAssignMacroCheck>(
+ "modernize-replace-disallow-copy-and-assign-macro");
CheckFactories.registerCheck<ReplaceRandomShuffleCheck>(
"modernize-replace-random-shuffle");
CheckFactories.registerCheck<ReturnBracedInitListCheck>(
diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp
new file mode 100644
index 000000000000..2219a3c477b3
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp
@@ -0,0 +1,90 @@
+//===--- 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)
+ : Check(Check), PP(PP) {}
+
+ 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->getName() != Check.getMacroName())
+ 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 Replacement = llvm::formatv(
+ R"cpp({0}(const {0} &) = delete;
+const {0} &operator=(const {0} &) = delete{1})cpp",
+ ClassIdent->getName(), shouldAppendSemi(Range) ? ";" : "");
+
+ Check.diag(MacroNameTok.getLocation(),
+ "prefer deleting copy constructor and assignment operator over "
+ "using macro '%0'")
+ << Check.getMacroName()
+ << FixItHint::CreateReplacement(
+ PP.getSourceManager().getExpansionRange(Range), Replacement);
+ }
+
+private:
+ /// \returns \c true if the next token after the given \p MacroLoc is \b not a
+ /// semicolon.
+ bool shouldAppendSemi(SourceRange MacroLoc) {
+ llvm::Optional<Token> Next = Lexer::findNextToken(
+ MacroLoc.getEnd(), PP.getSourceManager(), PP.getLangOpts());
+ return !(Next && Next->is(tok::semi));
+ }
+
+ ReplaceDisallowCopyAndAssignMacroCheck &Check;
+ Preprocessor &PP;
+};
+} // 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<ReplaceDisallowCopyAndAssignMacroCallbacks>(
+ *this, *ModuleExpanderPP));
+}
+
+void ReplaceDisallowCopyAndAssignMacroCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "MacroName", MacroName);
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h b/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h
new file mode 100644
index 000000000000..818b6aa270fd
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h
@@ -0,0 +1,62 @@
+//===--- 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 &getMacroName() const { return MacroName; }
+
+private:
+ 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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8da24a93d7f4..bd898de446b9 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -136,6 +136,12 @@ New checks
Finds includes of system libc headers not provided by the compiler within
llvm-libc implementations.
+- New :doc:`modernize-replace-disallow-copy-and-assign-macro
+ <clang-tidy/checks/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
<clang-tidy/checks/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
index 3794aa5bc3d8..17331605aa64 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -218,6 +218,7 @@ Clang-Tidy Checks
`modernize-raw-string-literal <modernize-raw-string-literal.html>`_, "Yes"
`modernize-redundant-void-arg <modernize-redundant-void-arg.html>`_, "Yes"
`modernize-replace-auto-ptr <modernize-replace-auto-ptr.html>`_, "Yes"
+ `modernize-replace-disallow-copy-and-assign-macro <modernize-replace-disallow-copy-and-assign-macro.html>`_, "Yes"
`modernize-replace-random-shuffle <modernize-replace-random-shuffle.html>`_, "Yes"
`modernize-return-braced-init-list <modernize-return-braced-init-list.html>`_, "Yes"
`modernize-shrink-to-fit <modernize-shrink-to-fit.html>`_, "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
index 000000000000..6717c928506a
--- /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
+ <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
index 000000000000..50b0c57d3b52
--- /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: prefer deleting copy constructor and assignment operator over using 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: prefer deleting copy constructor and assignment operator over using 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: prefer deleting copy constructor and assignment operator over using 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: prefer deleting copy constructor and assignment operator over using 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: prefer deleting copy constructor and assignment operator over using macro 'DISALLOW_COPY_AND_ASSIGN_NEEDS_PREEXPANSION'
More information about the cfe-commits
mailing list