[Clang] Update missing varargs arg extension warnings #84520

Sirraide · 2024-03-08T17:23:40Z

This updates a few warnings that were diagnosing no arguments for a ... variadic macro parameter as a GNU extension when it actually is a C++20/C23 extension now.

This fixes #84495.

llvmbot · 2024-03-08T17:24:11Z

@llvm/pr-subscribers-clang

Author: None (Sirraide)

Changes

This updates a few warnings that were diagnosing no arguments for a ... variadic macro parameter as a GNU extension when it actually is a C++20/C23 extension now.

This fixes #84495.

Full diff: https://github.com/llvm/llvm-project/pull/84520.diff

7 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+3)
(modified) clang/include/clang/Basic/DiagnosticLexKinds.td (+10-3)
(modified) clang/lib/Lex/PPMacroExpansion.cpp (+10-3)
(modified) clang/test/C/C2x/n2975.c (+2-2)
(modified) clang/test/Lexer/gnu-flags.c (-2)
(modified) clang/test/Preprocessor/empty_va_arg.cpp (+11-7)
(modified) clang/test/Preprocessor/macro_fn.c (+2-2)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0ff4a93b15ea8f..ffc88e79961ce6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -212,6 +212,9 @@ Improvements to Clang's diagnostics
 - Clang now diagnoses lambda function expressions being implicitly cast to boolean values, under ``-Wpointer-bool-conversion``.
   Fixes #GH82512.
 
+- Clang now correctly diagnoses no arguments to a variadic macro parameter as a C23/C++20 extension.
+  Fixes #GH84495.
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index d7c172e6546351..ad6bacfb118d49 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -465,9 +465,16 @@ def err_embedded_directive : Error<
 def ext_embedded_directive : Extension<
   "embedding a directive within macro arguments has undefined behavior">,
   InGroup<DiagGroup<"embedded-directive">>;
-def ext_missing_varargs_arg : Extension<
-  "must specify at least one argument for '...' parameter of variadic macro">,
-  InGroup<GNUZeroVariadicMacroArguments>;
+def ext_c_missing_varargs_arg : Extension<
+  "passing no argument for the '...' parameter of a variadic macro is "
+  "a C23 extension">, InGroup<C23>;
+def ext_cxx_missing_varargs_arg : Extension<
+  "passing no argument for the '...' parameter of a variadic macro is "
+  "a C++20 extension">, InGroup<CXX20>;
+def warn_c17_compat_missing_varargs_arg : Warning<
+  "passing no argument for the '...' parameter of a variadic macro is "
+  "incompatible with C standards before C23">,
+  InGroup<CPre23Compat>, DefaultIgnore;
 def warn_cxx17_compat_missing_varargs_arg : Warning<
   "passing no argument for the '...' parameter of a variadic macro is "
   "incompatible with C++ standards before C++20">,
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index 3017461dc66e86..3cde001e84760e 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -995,9 +995,16 @@ MacroArgs *Preprocessor::ReadMacroCallArgumentList(Token &MacroName,
       if (!MI->hasCommaPasting()) {
         // C++20 allows this construct, but standards before C++20 and all C
         // standards do not allow the construct (we allow it as an extension).
-        Diag(Tok, getLangOpts().CPlusPlus20
-                      ? diag::warn_cxx17_compat_missing_varargs_arg
-                      : diag::ext_missing_varargs_arg);
+        unsigned ID;
+        if (getLangOpts().CPlusPlus20)
+          ID = diag::warn_cxx17_compat_missing_varargs_arg;
+        else if (getLangOpts().CPlusPlus)
+          ID = diag::ext_cxx_missing_varargs_arg;
+        else if (getLangOpts().C23)
+          ID = diag::warn_c17_compat_missing_varargs_arg;
+        else
+          ID = diag::ext_c_missing_varargs_arg;
+        Diag(Tok, ID);
         Diag(MI->getDefinitionLoc(), diag::note_macro_here)
           << MacroName.getIdentifierInfo();
       }
diff --git a/clang/test/C/C2x/n2975.c b/clang/test/C/C2x/n2975.c
index 5fc641dd66e781..2269400fe47c34 100644
--- a/clang/test/C/C2x/n2975.c
+++ b/clang/test/C/C2x/n2975.c
@@ -11,7 +11,7 @@
 void func(...) { // expected-warning {{'...' as the only parameter of a function is incompatible with C standards before C23}}
   // Show that va_start doesn't require the second argument in C23 mode.
   va_list list;
-  va_start(list); // FIXME: it would be nice to issue a portability warning to C17 and earlier here.
+  va_start(list); // expected-warning {{passing no argument for the '...' parameter of a variadic macro is incompatible with C standards before C23}} expected-note@* {{macro 'va_start' defined here}}
   va_end(list);
 
   // Show that va_start doesn't expand or evaluate the second argument.
@@ -26,7 +26,7 @@ void func(...) { // expected-warning {{'...' as the only parameter of a function
   __builtin_va_start(list); // expected-error {{too few arguments to function call, expected 2, have 1}}
 
   // Verify that the return type of a call to va_start is 'void'.
-  _Static_assert(__builtin_types_compatible_p(__typeof__(va_start(list)), void), "");
+  _Static_assert(__builtin_types_compatible_p(__typeof__(va_start(list)), void), ""); // expected-warning {{passing no argument for the '...' parameter of a variadic macro is incompatible with C standards before C23}} expected-note@* {{macro 'va_start' defined here}}
   _Static_assert(__builtin_types_compatible_p(__typeof__(__builtin_va_start(list, 0)), void), "");
 }
 
diff --git a/clang/test/Lexer/gnu-flags.c b/clang/test/Lexer/gnu-flags.c
index 384339fc859429..4d6d216b101f4a 100644
--- a/clang/test/Lexer/gnu-flags.c
+++ b/clang/test/Lexer/gnu-flags.c
@@ -17,8 +17,6 @@
 
 
 #if ALL || ZEROARGS
-// expected-warning@+9 {{must specify at least one argument for '...' parameter of variadic macro}}
-// expected-note@+4 {{macro 'efoo' defined here}}
 // expected-warning@+3 {{token pasting of ',' and __VA_ARGS__ is a GNU extension}}
 #endif
 
diff --git a/clang/test/Preprocessor/empty_va_arg.cpp b/clang/test/Preprocessor/empty_va_arg.cpp
index 2ee431f6bde838..7c7d49d8fb1632 100644
--- a/clang/test/Preprocessor/empty_va_arg.cpp
+++ b/clang/test/Preprocessor/empty_va_arg.cpp
@@ -1,12 +1,16 @@
-// RUN: %clang_cc1 -Eonly -std=c++17 -pedantic -verify %s
-// RUN: %clang_cc1 -Eonly -std=c17 -pedantic -verify -x c %s
-// RUN: %clang_cc1 -Eonly -std=c++20 -pedantic -Wpre-c++20-compat -verify=compat %s
+// RUN: %clang_cc1 -Eonly -std=c17 -pedantic -verify=c17,expected -x c %s
+// RUN: %clang_cc1 -Eonly -std=c23 -pedantic -Wpre-c23-compat -verify=c23,expected -x c %s
+// RUN: %clang_cc1 -Eonly -std=c++17 -pedantic -verify=cxx17,expected %s
+// RUN: %clang_cc1 -Eonly -std=c++20 -pedantic -Wpre-c++20-compat -verify=cxx20,expected %s
 
-#define FOO(x, ...) // expected-note {{macro 'FOO' defined here}} \
-                    // compat-note {{macro 'FOO' defined here}}
+// silent-no-diagnostics
+
+#define FOO(x, ...) // expected-note  {{macro 'FOO' defined here}}
 
 int main() {
-  FOO(42) // expected-warning {{must specify at least one argument for '...' parameter of variadic macro}} \
-          // compat-warning {{passing no argument for the '...' parameter of a variadic macro is incompatible with C++ standards before C++20}}
+  FOO(42) // c17-warning {{passing no argument for the '...' parameter of a variadic macro is a C23 extension}} \
+          // cxx17-warning {{passing no argument for the '...' parameter of a variadic macro is a C++20 extension}} \
+          // c23-warning {{passing no argument for the '...' parameter of a variadic macro is incompatible with C standards before C23}} \
+          // cxx20-warning {{passing no argument for the '...' parameter of a variadic macro is incompatible with C++ standards before C++20}}
 }
 
diff --git a/clang/test/Preprocessor/macro_fn.c b/clang/test/Preprocessor/macro_fn.c
index 5f4ea0e26d5d8b..81d83632140788 100644
--- a/clang/test/Preprocessor/macro_fn.c
+++ b/clang/test/Preprocessor/macro_fn.c
@@ -37,8 +37,8 @@ e(x)
 e()
 
 zero_dot()
-one_dot(x)  /* empty ... argument: expected-warning {{must specify at least one argument for '...' parameter of variadic macro}}  */
-one_dot()   /* empty first argument, elided ...: expected-warning {{must specify at least one argument for '...' parameter of variadic macro}} */
+one_dot(x)  /* empty ... argument: expected-warning {{passing no argument for the '...' parameter of a variadic macro is a C23 extension}}  */
+one_dot()   /* empty first argument, elided ...: expected-warning {{passing no argument for the '...' parameter of a variadic macro is a C23 extension}} */
 
 
 /* Crash with function-like macro test at end of directive. */

shafik

Just a nit here.

clang/lib/Lex/PPMacroExpansion.cpp

AaronBallman

LGTM aside from a comment request

clang/lib/Lex/PPMacroExpansion.cpp

This updates a few warnings that were diagnosing no arguments for a `...` variadic macro parameter as a GNU extension when it actually is a C++20/C23 extension now. This fixes llvm#84495.

They began complaining since #84520.

zibi2 · 2024-04-19T19:11:27Z

@Sirraide, I noticed the following build issue when new compiler with this change (bootstrap compiler) is being used to build one of the files for clangd:

.../clang-tools-extra/clangd/unittests/FindTargetTests.cpp:430:29: error: passing no argument for the '...' parameter of a variadic macro is a C++20 extension [-Werror,-Wc++20-extensions]
  430 |   EXPECT_DECLS("AutoTypeLoc");
      |                             ^
.../clang-tools-extra/clangd/unittests/FindTargetTests.cpp:98:9: note: macro 'EXPECT_DECLS' defined here
   98 | #define EXPECT_DECLS(NodeType, ...)                                            \
      |         ^

Do you have suggestion how to fix it or even better provie the fix?
Thank you,
Zibi

Sirraide · 2024-04-19T21:43:49Z

@Sirraide, I noticed the following build issue when new compiler with this change (bootstrap compiler) is being used to build one of the files for clangd:
.../clang-tools-extra/clangd/unittests/FindTargetTests.cpp:430:29: error: passing no argument for the '...' parameter of a variadic macro is a C++20 extension [-Werror,-Wc++20-extensions]
  430 |   EXPECT_DECLS("AutoTypeLoc");
      |                             ^
.../clang-tools-extra/clangd/unittests/FindTargetTests.cpp:98:9: note: macro 'EXPECT_DECLS' defined here
   98 | #define EXPECT_DECLS(NodeType, ...)                                            \
      |         ^
Do you have suggestion how to fix it or even better provie the fix? Thank you, Zibi

I mean, from what I can tell, this isn’t a Clang issue but rather just a result of this code here not being standards-compliant C++17. Before C++20, afaik the ‘fix’ for this is to have two different macros: one variadic, one not variadic. It’s not pretty, but that’s why that got changed in C++20.

Sirraide · 2024-04-19T21:46:04Z

Actually, a trailing comma in the macro invocation might work too. It’s been a while since I last had to deal w/ this.

zibi2 · 2024-04-22T13:11:54Z

I agree the code is not C++17 compliant. An alternative fix would be to compile this file with -Wno-c++20-extensions.

zibi2 · 2024-04-22T17:02:57Z

Actually, a trailing comma in the macro invocation might work too. It’s been a while since I last had to deal w/ this.

Yes, it does and I think it's cleaner than my proposal. Thank you.

This PR fixes the build errors for one of the `clangd` unit tests bucket similar to the following: ``` .../clang-tools-extra/clangd/unittests/FindTargetTests.cpp:430:29: error: passing no argument for the '...' parameter of a variadic macro is a C++20 extension [-Werror,-Wc++20-extensions] 430 | EXPECT_DECLS("AutoTypeLoc"); | ^ .../clang-tools-extra/clangd/unittests/FindTargetTests.cpp:98:9: note: macro 'EXPECT_DECLS' defined here 98 | #define EXPECT_DECLS(NodeType, ...) \ | ^ ``` This happens when using a build compiler with #84520. The fix is to include commas to compensate for empty vararg macro arguments in a few instances.

[Clang] Update missing varargs arg extension warnings

5026928

Sirraide added c23 clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer labels Mar 8, 2024

Sirraide requested review from cor3ntin, AaronBallman and Endilll March 8, 2024 17:23

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 8, 2024

[NFC] Update comment

f5b663d

shafik reviewed Mar 9, 2024

View reviewed changes

clang/lib/Lex/PPMacroExpansion.cpp Show resolved Hide resolved

AaronBallman approved these changes Mar 20, 2024

View reviewed changes

clang/lib/Lex/PPMacroExpansion.cpp Show resolved Hide resolved

Sirraide added 2 commits March 20, 2024 19:16

[Clang] Add standards citation

0af09b3

Merge branch 'main' into 84495

83094c0

Sirraide merged commit 5231005 into llvm:main Mar 20, 2024

Sirraide deleted the 84495 branch March 20, 2024 18:19

chapuni added a commit that referenced this pull request Mar 25, 2024

compiler-rt: Fix variadic macro warnings [-Wc++20-extensions]

8f86c6b

They began complaining since #84520.

zibi2 mentioned this pull request Apr 22, 2024

[clangd] Fix unittests in TargetDeclTest bucket #89630

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang] Update missing varargs arg extension warnings #84520

[Clang] Update missing varargs arg extension warnings #84520

Uh oh!

Sirraide commented Mar 8, 2024

Uh oh!

llvmbot commented Mar 8, 2024

Uh oh!

shafik left a comment

Uh oh!

Uh oh!

AaronBallman left a comment

Uh oh!

Uh oh!

zibi2 commented Apr 19, 2024

Uh oh!

Sirraide commented Apr 19, 2024

Uh oh!

Sirraide commented Apr 19, 2024

Uh oh!

zibi2 commented Apr 22, 2024

Uh oh!

zibi2 commented Apr 22, 2024

Uh oh!

Uh oh!

[Clang] Update missing varargs arg extension warnings #84520

[Clang] Update missing varargs arg extension warnings #84520

Uh oh!

Conversation

Sirraide commented Mar 8, 2024

Uh oh!

llvmbot commented Mar 8, 2024

Uh oh!

shafik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zibi2 commented Apr 19, 2024

Uh oh!

Sirraide commented Apr 19, 2024

Uh oh!

Sirraide commented Apr 19, 2024

Uh oh!

zibi2 commented Apr 22, 2024

Uh oh!

zibi2 commented Apr 22, 2024

Uh oh!

Uh oh!