[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h:46
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
----------------
I think check should work in C++11 or newer.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:192
    `llvm-twine-local <llvm-twine-local.html>`_, "Yes"
-   `llvmlibc-callee-namespace <llvmlibc-calle-namespace.html>`_,
+   `llvmlibc-callee-namespace <llvmlibc-callee-namespace.html>`_,
    `llvmlibc-implementation-in-namespace <llvmlibc-implementation-in-namespace.html>`_,
----------------
I think this and some other changes are unrelated.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:6
+
+This check finds macro expansions of ``DISALLOW_COPY_AND_ASSIGN(Type)`` and
+replaces them with a deleted copy constructor and a deleted assignment operator.
----------------
Just //Finds//.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:31
+* Notice that the migration example above leaves the ``private`` access
+  specification untouched. This opens room for improvement, yes I know.
+
----------------
modernize-use-equals-delete takes care about this.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:33
+
+* Maybe someday I will know how to expand the macro and not use my pre-written
+  code snippet. But for now, this is okay.
----------------
Please move this to comments into code.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:36
+
+* The ``FinalizeWithSemicolon`` option (see below) maybe could be automated.
+
----------------
Please use single back-ticks for options.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:44
+   A string specifying the macro name whose expansion will be replaced.
+   Default is ``DISALLOW_COPY_AND_ASSIGN``.
+
----------------
Please use single back-ticks for option values.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:48
+
+   A boolean value that tells the replacement to put a semicolon at the end or
+   not. Default is ``false``.
----------------
Please use single back-ticks for option values.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80531/new/

https://reviews.llvm.org/D80531