[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro
Konrad Wilhelm Kleine via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu May 28 13:45:47 PDT 2020
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
More information about the cfe-commits
mailing list