https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/188474
>From 30e9d5ccb280d2f8737dba77ecb9574631277abf Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Wed, 25 Mar 2026 13:25:01 +0100 Subject: [PATCH 1/5] [clang-tidy] New checker: modernize-use-va-opt to replace the `, ##__VAR_ARG__` GNU extension --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ModernizeTidyModule.cpp | 2 + .../clang-tidy/modernize/UseVaOptCheck.cpp | 71 +++++++++++++++++++ .../clang-tidy/modernize/UseVaOptCheck.h | 33 +++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/modernize/use-va-opt.rst | 19 +++++ .../checkers/modernize/use-va-opt.cpp | 37 ++++++++++ 8 files changed, 169 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-va-opt.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-va-opt.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 2c5c44db587fe..71aebcaf1feca 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -57,6 +57,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseTransparentFunctorsCheck.cpp UseUncaughtExceptionsCheck.cpp UseUsingCheck.cpp + UseVaOptCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index cc13da7535bcb..e45538213c6e7 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -57,6 +57,7 @@ #include "UseTransparentFunctorsCheck.h" #include "UseUncaughtExceptionsCheck.h" #include "UseUsingCheck.h" +#include "UseVaOptCheck.h" using namespace clang::ast_matchers; @@ -145,6 +146,7 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseUncaughtExceptionsCheck>( "modernize-use-uncaught-exceptions"); CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using"); + CheckFactories.registerCheck<UseVaOptCheck>("modernize-use-va-opt"); } }; diff --git a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp new file mode 100644 index 0000000000000..2ea4acf20f740 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp @@ -0,0 +1,71 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseVaOptCheck.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +namespace { +class VaOptPPCallbacks : public PPCallbacks { +public: + VaOptPPCallbacks(Preprocessor *PP, UseVaOptCheck *Check) + : PP(PP), Check(Check) {} + + void MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) override { + const MacroInfo *MI = MD->getMacroInfo(); + if (!MI->isVariadic()) + return; + + std::optional<Token> PrevComma; + bool PrevHashHash = false; + ; + for (Token Tok : MI->tokens()) { + if (PrevHashHash) { + if (const auto *II = Tok.getIdentifierInfo(); + II && II->getName() == "__VA_ARGS__") { + // FIXME: The replacement really should be " __VA_OPT__(,) + // __VA_ARGS__", but this breaks the fixit which removes the , in that + // case o_O. + Check->diag(Tok.getLocation(), + "Use __VA_OPT__ instead of GNU extension to __VA_ARGS__") + << FixItHint::CreateReplacement( + SourceRange(PrevComma->getLocation(), Tok.getLocation()), + " __VA_OPT__(',') __VA_ARGS__"); + } + PrevComma = std::nullopt; + PrevHashHash = false; + } else if (PrevComma) { + if (Tok.is(tok::hashhash)) + PrevHashHash = true; + else + PrevComma = std::nullopt; + } else if (Tok.is(tok::comma)) { + PrevComma = Tok; + } + } + } + +private: + Preprocessor *PP; + UseVaOptCheck *Check; +}; +} // namespace + +void UseVaOptCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks(std::make_unique<VaOptPPCallbacks>(PP, this)); +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.h b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.h new file mode 100644 index 0000000000000..11493f298d842 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.h @@ -0,0 +1,33 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEVAOPTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEVAOPTCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { + +/// Use __VA_OPT__ instead of ##__VA_ARGS__ GNU extension. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-va-opt.html +class UseVaOptCheck : public ClangTidyCheck { +public: + UseVaOptCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus20 || LangOpts.C23; + } +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEVAOPTCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 49901f8a706c6..c646d5281c0b5 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -145,6 +145,11 @@ New checks Finds places where structured bindings could be used to decompose pairs and suggests replacing them. +- New :doc:`modernize-use-va-opt + <clang-tidy/checks/modernize/use-va-opt>` check. + + Use __VA_OPT__ instead of ##__VA_ARGS__ GNU extension. + - New :doc:`performance-string-view-conversions <clang-tidy/checks/performance/string-view-conversions>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index ceab1e9414951..2441f9e63fcff 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -336,6 +336,7 @@ Clang-Tidy Checks :doc:`modernize-use-transparent-functors <modernize/use-transparent-functors>`, "Yes" :doc:`modernize-use-uncaught-exceptions <modernize/use-uncaught-exceptions>`, "Yes" :doc:`modernize-use-using <modernize/use-using>`, "Yes" + :doc:`modernize-use-va-opt <modernize/use-va-opt>`, "Yes" :doc:`mpi-buffer-deref <mpi/buffer-deref>`, "Yes" :doc:`mpi-type-mismatch <mpi/type-mismatch>`, "Yes" :doc:`objc-assert-equals <objc/assert-equals>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-va-opt.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-va-opt.rst new file mode 100644 index 0000000000000..af631ff18419a --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-va-opt.rst @@ -0,0 +1,19 @@ +.. title:: clang-tidy - modernize-use-va-opt + +modernize-use-va-opt +==================== + +Suggest using ``__VA_OPT__(,)`` instead of ``, ##__VA_ARGS__`` when implementing +variadic macro. ``, ##__VA_ARGS__`` is a GNU extension. + +.. code:: c++ + + extern int bar(...); + #define FOO(a, ...) bar(a, ##__VA_ARGS__) + +becomes: + +.. code:: c++ + + extern int bar(...); + #define FOO(a, ...) bar(a __VA_OPT__(,) __VA_ARGS__) diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-va-opt.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-va-opt.cpp new file mode 100644 index 0000000000000..b0fde0000de23 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-va-opt.cpp @@ -0,0 +1,37 @@ +// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-va-opt %t + +extern void foo(...); + + +// CHECK-MESSAGES: :[[@LINE+2]]:26: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] +// CHECK-FIXES: #define M0(...) foo(1 __VA_OPT__(',') __VA_ARGS__) +#define M0(...) foo(1, ##__VA_ARGS__) + +// CHECK-MESSAGES: :[[@LINE+2]]:27: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] +// CHECK-FIXES: #define M1(...) foo(1 __VA_OPT__(',') __VA_ARGS__) +#define M1(...) foo(1, ## __VA_ARGS__) + +// CHECK-MESSAGES: :[[@LINE+2]]:27: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] +// CHECK-FIXES: #define M2(...) foo(1 __VA_OPT__(',') __VA_ARGS__) +#define M2(...) foo(1 ,## __VA_ARGS__) + +// CHECK-MESSAGES: :[[@LINE+2]]:28: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] +// CHECK-FIXES: #define M3(...) foo(1 __VA_OPT__(',') __VA_ARGS__) +#define M3(...) foo(1 , ## __VA_ARGS__) + +// CHECK-MESSAGES: :[[@LINE+3]]:28: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] +// CHECK-MESSAGES: :[[@LINE+2]]:44: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] +// CHECK-FIXES: #define M4(...) foo(1 __VA_OPT__(',') __VA_ARGS__ __VA_OPT__(',') __VA_ARGS__) +#define M4(...) foo(1 , ## __VA_ARGS__, ## __VA_ARGS__) + +// No message, this will never add a comma before __VA_ARGS__ +#define P0(...) foo(1 ##__VA_ARGS__) + +// No message, this will never add a comma before __VA_ARGS__ +#define P1(...) foo(__VA_ARGS__) + +// No message, this will never add a comma before __VA_ARGS__ +#define P2(...) foo(##__VA_ARGS__) + +// No message, this will never add a comma before __VA_ARGS__ +#define P3(...) foo(, __VA_ARGS__) >From eba0f40290441d23a0799502331e2a3d1305e229 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Wed, 25 Mar 2026 13:42:35 +0100 Subject: [PATCH 2/5] fixup! [clang-tidy] New checker: modernize-use-va-opt to replace the `, ##__VAR_ARG__` GNU extension --- clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp index 2ea4acf20f740..921038aa15afa 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp @@ -29,9 +29,9 @@ class VaOptPPCallbacks : public PPCallbacks { std::optional<Token> PrevComma; bool PrevHashHash = false; - ; - for (Token Tok : MI->tokens()) { + for (const Token Tok : MI->tokens()) { if (PrevHashHash) { + assert(PrevComma); if (const auto *II = Tok.getIdentifierInfo(); II && II->getName() == "__VA_ARGS__") { // FIXME: The replacement really should be " __VA_OPT__(,) @@ -46,11 +46,13 @@ class VaOptPPCallbacks : public PPCallbacks { PrevComma = std::nullopt; PrevHashHash = false; } else if (PrevComma) { + assert(!PrevHashHash); if (Tok.is(tok::hashhash)) PrevHashHash = true; else PrevComma = std::nullopt; } else if (Tok.is(tok::comma)) { + assert(!PrevHashHash); PrevComma = Tok; } } >From d1c61e30969afa28be9cb46b21e465fddc453dde Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Wed, 25 Mar 2026 13:44:13 +0100 Subject: [PATCH 3/5] fixup! fixup! [clang-tidy] New checker: modernize-use-va-opt to replace the `, ##__VAR_ARG__` GNU extension --- .../clang-tidy/modernize/UseVaOptCheck.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp index 921038aa15afa..8c01d7d9985f2 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp @@ -18,8 +18,7 @@ namespace clang::tidy::modernize { namespace { class VaOptPPCallbacks : public PPCallbacks { public: - VaOptPPCallbacks(Preprocessor *PP, UseVaOptCheck *Check) - : PP(PP), Check(Check) {} + VaOptPPCallbacks(UseVaOptCheck &Check) : Check(Check) {} void MacroDefined(const Token &MacroNameTok, const MacroDirective *MD) override { @@ -37,8 +36,8 @@ class VaOptPPCallbacks : public PPCallbacks { // FIXME: The replacement really should be " __VA_OPT__(,) // __VA_ARGS__", but this breaks the fixit which removes the , in that // case o_O. - Check->diag(Tok.getLocation(), - "Use __VA_OPT__ instead of GNU extension to __VA_ARGS__") + Check.diag(Tok.getLocation(), + "Use __VA_OPT__ instead of GNU extension to __VA_ARGS__") << FixItHint::CreateReplacement( SourceRange(PrevComma->getLocation(), Tok.getLocation()), " __VA_OPT__(',') __VA_ARGS__"); @@ -59,15 +58,14 @@ class VaOptPPCallbacks : public PPCallbacks { } private: - Preprocessor *PP; - UseVaOptCheck *Check; + UseVaOptCheck &Check; }; } // namespace void UseVaOptCheck::registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - PP->addPPCallbacks(std::make_unique<VaOptPPCallbacks>(PP, this)); + PP->addPPCallbacks(std::make_unique<VaOptPPCallbacks>(*this)); } } // namespace clang::tidy::modernize >From 41e26d5b9c28c7575aa2f1ae8694af2849df45b5 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Wed, 25 Mar 2026 13:47:28 +0100 Subject: [PATCH 4/5] fixup! fixup! fixup! [clang-tidy] New checker: modernize-use-va-opt to replace the `, ##__VAR_ARG__` GNU extension --- .../clang-tidy/modernize/UseVaOptCheck.cpp | 5 +---- .../test/clang-tidy/checkers/modernize/use-va-opt.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp index 8c01d7d9985f2..8fde502bc5872 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp @@ -33,14 +33,11 @@ class VaOptPPCallbacks : public PPCallbacks { assert(PrevComma); if (const auto *II = Tok.getIdentifierInfo(); II && II->getName() == "__VA_ARGS__") { - // FIXME: The replacement really should be " __VA_OPT__(,) - // __VA_ARGS__", but this breaks the fixit which removes the , in that - // case o_O. Check.diag(Tok.getLocation(), "Use __VA_OPT__ instead of GNU extension to __VA_ARGS__") << FixItHint::CreateReplacement( SourceRange(PrevComma->getLocation(), Tok.getLocation()), - " __VA_OPT__(',') __VA_ARGS__"); + " __VA_OPT__(,) __VA_ARGS__"); } PrevComma = std::nullopt; PrevHashHash = false; diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-va-opt.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-va-opt.cpp index b0fde0000de23..c405b076809ca 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-va-opt.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-va-opt.cpp @@ -4,24 +4,24 @@ extern void foo(...); // CHECK-MESSAGES: :[[@LINE+2]]:26: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] -// CHECK-FIXES: #define M0(...) foo(1 __VA_OPT__(',') __VA_ARGS__) +// CHECK-FIXES: #define M0(...) foo(1 __VA_OPT__(,) __VA_ARGS__) #define M0(...) foo(1, ##__VA_ARGS__) // CHECK-MESSAGES: :[[@LINE+2]]:27: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] -// CHECK-FIXES: #define M1(...) foo(1 __VA_OPT__(',') __VA_ARGS__) +// CHECK-FIXES: #define M1(...) foo(1 __VA_OPT__(,) __VA_ARGS__) #define M1(...) foo(1, ## __VA_ARGS__) // CHECK-MESSAGES: :[[@LINE+2]]:27: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] -// CHECK-FIXES: #define M2(...) foo(1 __VA_OPT__(',') __VA_ARGS__) +// CHECK-FIXES: #define M2(...) foo(1 __VA_OPT__(,) __VA_ARGS__) #define M2(...) foo(1 ,## __VA_ARGS__) // CHECK-MESSAGES: :[[@LINE+2]]:28: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] -// CHECK-FIXES: #define M3(...) foo(1 __VA_OPT__(',') __VA_ARGS__) +// CHECK-FIXES: #define M3(...) foo(1 __VA_OPT__(,) __VA_ARGS__) #define M3(...) foo(1 , ## __VA_ARGS__) // CHECK-MESSAGES: :[[@LINE+3]]:28: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] // CHECK-MESSAGES: :[[@LINE+2]]:44: warning: Use __VA_OPT__ instead of GNU extension to __VA_ARGS__ [modernize-use-va-opt] -// CHECK-FIXES: #define M4(...) foo(1 __VA_OPT__(',') __VA_ARGS__ __VA_OPT__(',') __VA_ARGS__) +// CHECK-FIXES: #define M4(...) foo(1 __VA_OPT__(,) __VA_ARGS__ __VA_OPT__(,) __VA_ARGS__) #define M4(...) foo(1 , ## __VA_ARGS__, ## __VA_ARGS__) // No message, this will never add a comma before __VA_ARGS__ >From ac0690895c6288d1c90496881276fac087ac86c0 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Wed, 25 Mar 2026 14:01:55 +0100 Subject: [PATCH 5/5] fixup! fixup! fixup! fixup! [clang-tidy] New checker: modernize-use-va-opt to replace the `, ##__VAR_ARG__` GNU extension --- clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp index 8fde502bc5872..1aa048872a95f 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseVaOptCheck.cpp @@ -30,7 +30,9 @@ class VaOptPPCallbacks : public PPCallbacks { bool PrevHashHash = false; for (const Token Tok : MI->tokens()) { if (PrevHashHash) { - assert(PrevComma); + // FIXME: An assert should be enough, this is just to please the linter. + if (!PrevComma) + llvm_unreachable("PrevComma cannot be unset if PrevHashHash is set"); if (const auto *II = Tok.getIdentifierInfo(); II && II->getName() == "__VA_ARGS__") { Check.diag(Tok.getLocation(), _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
