aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM aside from some minor nits.
================
Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:71
+void MacroUsageCheck::warnMacro(const MacroDirective *MD) {
+ auto MessageFactory = [](const MacroInfo *Info) {
+ /// A variadic macro is function-like at the same time. Therefore variadic
----------------
It seems unnecessarily complicated, to me, to use a lambda here. Why not use a
StringRef local variable?
```
StringRef Msg = "blah";
if (Info->isVariadic())
Msg = "blah";
else if (Info->isFunctionLike())
Msg = "blah";
```
================
Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:78
+ "variadic template function";
+ else if (Info->isFunctionLike())
+ return "function-like macro used; consider a 'constexpr' template "
----------------
Nit: `else` after `return`
================
Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:81
+ "function";
+ else
+ return "macro used to declare a constant; consider using a 'constexpr' "
----------------
Nit: same
================
Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.h:34-35
+ void registerPPCallbacks(CompilerInstance &Compiler) override;
+ void warnMacro(const MacroDirective *MD);
+ void warnNaming(const MacroDirective *MD);
+
----------------
Nit: these can be private functions instead.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41648
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
