kwk added a comment.
@njames93 I've addressed all your comments and hope the patch is good to land
now :)
================
Comment at:
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:25
+ const SourceManager &SM)
+ : Check(Check), PP(PP), SM(SM) {}
+
----------------
njames93 wrote:
> nit: You don't need to store a reference to the `SourceManager` as the
> `Preprocessor` holds a reference to it.
@njames93 thank you for letting me know. I passed it along because the
´ClangTidyCheck::registerPPCallbacks` that I override actually accepts both,
`SourceManager` and the `PP`:
```
lang=c++
virtual void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) {}
```
Other checks like `MakeSmartPtrCheck`, `PassByValueCheck` also do this similar
to mine.
Anyway, I've changed the code to your liking.
================
Comment at:
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h:52
+
+ const std::string MacroName;
+};
----------------
njames93 wrote:
> Make this private with a get function and I'll be happy.
Done. But since the variable is `const` having it public shouldn't allow a
caller do anything but read the macro name. But I understand that convention is
king ;)
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
