kwk updated this revision to Diff 266101.
kwk added a comment.
- Move comment about FinalizeWithSemicolon into code
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80531/new/
https://reviews.llvm.org/D80531
Files:
clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
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
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp
===================================================================
--- /dev/null
+++ 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'
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst
===================================================================
--- /dev/null
+++ 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
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
@@ -217,6 +217,7 @@
`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"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ 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
+ <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.
Index: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h
===================================================================
--- /dev/null
+++ 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
Index: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp
===================================================================
--- /dev/null
+++ 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<ReplaceDisallowCopyAndAssignMacroCallbacks>(
+ *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
Index: clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ 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<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>(
Index: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ 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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
