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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D41648#1253647, @JonasToth wrote:

> @aaron.ballman What do you think of the current version of this check?
>  As migration strategy the option for `CAPS_ONLY` is provided, otherwise the filtering is provided to silence necessary macros (which kind of enforces a common prefix for macros). This does implement both the cppcoreguidelines and allows migration.


I think that's a reasonable approach.



================
Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:78
+        "function-like macro used; consider a 'constexpr' template function";
+  if (Info->isVariadic())
+    DiagnosticMessage = "variadic macro used; consider using a 'constexpr' "
----------------
Aren't all vararg macros also function-like macros, so this will trigger two diagnostics for all variadic macro definitions?


================
Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:26
+
+    Boolean flag to warn all macros except those with CAPS_ONLY names.
+    This option is intended to ease introduction of this check into older
----------------
warn all macros -> warn on all macros


================
Comment at: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp:14
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
----------------
Please add a test like:
```
#define PROBLEMATIC_VARIADIC_TWO(x, ...) (__VA_ARGS__)
```
To ensure that you don't get two diagnostics (one for function-like and one for variadic).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648