njames93 added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:32
+ return;
+ if (std::string(Info->getNameStart()) != Check.MacroName)
+ return;
----------------
```
if (Info->getName() != Check.MacroName)```
Avoid constructing the string if you don't have to.
================
Comment at:
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:36
+ // be the class name.
+ const Token *classNameTok = Args->getUnexpArgument(0);
+ if (Args->ArgNeedsPreexpansion(classNameTok, PP))
----------------
s/classNameTok/ClassNameTok
================
Comment at:
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:41
+ return;
+ clang::IdentifierInfo *classIdent = classNameTok->getIdentifierInfo();
+ if (!classIdent)
----------------
s/classIndent/ClassIndent
================
Comment at:
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:49
+ R"cpp({0}(const {0} &) = delete;
+const {0} &operator=(const {0} &) = delete{1})cpp",
+ classIdent->getNameStart(), shouldAppendSemi(Range) ? ";" : "");
----------------
if you wanted to, you could format this quite easily to avoid the need of
specifying a formatter in the checks
```{2}const {0} &operator=(const {0} &) = delete{1})cpp",...
Lexer::getIndentationForLine(Range.getBegin(), SM))```
================
Comment at:
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h:52
+
+ const std::string MacroName;
+};
----------------
I'd prefer this to be private with a getter function
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp:36-38
+// 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{{$}}
----------------
These really aren't important to test for, so long as the diagnostic and fix it
are checked, that's all you need to worry about. Like how you have it for
`TestClass2`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80531/new/
https://reviews.llvm.org/D80531
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
