Skip to content

[C2y] Claim conformance to WG14 N3363 #130980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 12, 2025

Conversation

AaronBallman
Copy link
Collaborator

This paper clarifies two things:

  • a call to va_start must be within the body of a variadic function,
  • the va_list arguments must be lvalues

Clang has correctly diagnosed the first point since Clang 5.0. Clang generally diagnoses the second point, except for va_copy. However, the second point is not a constraint violation, so a diagnostic is not strictly required. That said, it would be good for us to properly diagnose va_copy.

This paper clarifies two things:
  * a call to va_start must be within the body of a variadic function,
  * the va_list arguments must be lvalues

Clang has correctly diagnosed the first point since Clang 5.0. Clang
generally diagnoses the second point, except for va_copy. However, the
second point is not a constraint violation, so a diagnostic is not
strictly required. That said, it would be good for us to properly
diagnose va_copy.
@AaronBallman AaronBallman added clang Clang issues not falling into any other category website clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer c2y labels Mar 12, 2025
@AaronBallman
Copy link
Collaborator Author

Posting this one to get precommit CI to validate before pushing. No functional changes are here.

@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2025

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

This paper clarifies two things:

  • a call to va_start must be within the body of a variadic function,
  • the va_list arguments must be lvalues

Clang has correctly diagnosed the first point since Clang 5.0. Clang generally diagnoses the second point, except for va_copy. However, the second point is not a constraint violation, so a diagnostic is not strictly required. That said, it would be good for us to properly diagnose va_copy.


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

2 Files Affected:

  • (added) clang/test/C/C2y/n3363.c (+40)
  • (modified) clang/www/c_status.html (+1-1)
diff --git a/clang/test/C/C2y/n3363.c b/clang/test/C/C2y/n3363.c
new file mode 100644
index 0000000000000..de73fd443227e
--- /dev/null
+++ b/clang/test/C/C2y/n3363.c
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux -verify=expected,array-type -std=c2y -Wall -pedantic -ffreestanding %s
+// RUN: %clang_cc1 -triple aarch64-pc-win32 -verify=expected,char-ptr-type -std=c2y -Wall -pedantic -ffreestanding %s
+
+/* WG14 N3363: Clang 5
+ * stdarg.h wording... v3
+ *
+ * Clarifies that va_start can only be used within a function body, and that
+ * the macros and functions require an lvalue of type va_list instead of an
+ * rvalue.
+ *
+ * Clang has diagnosed this correctly in all cases except va_copy since
+ * Clang 5. va_copy still needs to be updated to issue a diagnostic about use
+ * of an rvalue. However, our lack of a diagnostic is still conforming because
+ * a diagnostic is not required (it's not a constraint violation).
+ */
+
+#include <stdarg.h>
+
+// The va_list argument given to every macro defined in this subclause shall be
+// an lvalue of this type or the result of array-to-pointer decay of such an
+// lvalue.
+void f(int a, ...) {
+  va_list rvalue(); // array-type-error {{function cannot return array type 'va_list' (aka '__builtin_va_list')}}
+  va_start(rvalue()); // char-ptr-type-error {{'va_list' argument to 'va_start' is required to be an lvalue}}
+  va_arg(rvalue(), int); // char-ptr-type-error {{expression is not assignable}}
+  // FIXME: this should get two diagnostics about use of a non-lvalue.
+  va_copy(rvalue(), rvalue());
+  va_end(rvalue()); // char-ptr-type-error {{non-const lvalue reference to type '__builtin_va_list' cannot bind to a temporary of type 'va_list' (aka 'char *')}}
+}
+
+// The va_start macro may only be invoked in the compound-statement of the body
+// of a variadic function.
+void g(va_list ap, int [(va_start(ap), 1)], ...) { // expected-error {{'va_start' cannot be used outside a function}}
+  va_end(ap);
+}
+
+va_list ap;
+struct S {
+  int array[(va_start(ap), 1)]; // expected-error {{'va_start' cannot be used outside a function}}
+};
diff --git a/clang/www/c_status.html b/clang/www/c_status.html
index 4432e62b47520..6080ed060cfbb 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -263,7 +263,7 @@ <h2 id="c2y">C2y implementation status</h2>
     <tr>
       <td>stdarg.h wording... v3</td>
       <td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3363.pdf">N3363</a></td>
-      <td class="unknown" align="center">Unknown</td>
+      <td class="full" align="center">Clang 5</td>
 	</tr>
     <tr>
       <td>Preprocessor integer expressions, v. 2</td>

@AaronBallman AaronBallman merged commit ab53e1c into llvm:main Mar 12, 2025
9 of 10 checks passed
@AaronBallman AaronBallman deleted the aballman-wg14-n3363 branch March 12, 2025 18:38
frederik-h pushed a commit to frederik-h/llvm-project that referenced this pull request Mar 18, 2025
This paper clarifies two things:
  * a call to va_start must be within the body of a variadic function,
  * the va_list arguments must be lvalues

Clang has correctly diagnosed the first point since Clang 5.0. Clang
generally diagnoses the second point, except for va_copy. However, the
second point is not a constraint violation, so a diagnostic is not
strictly required. That said, it would be good for us to properly
diagnose va_copy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c2y clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang Clang issues not falling into any other category website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants