[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Aaron Ballman via Phabricator via cfe-commits]

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