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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
