[clang] e321c53 - [C2x] Relaxing requirements for va_start
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 8 04:36:19 PST 2022
Author: Aaron Ballman
Date: 2022-12-08T07:36:07-05:00
New Revision: e321c53f7bb8d61f64b33e8b1f6a97eb32eaaa69
URL: https://github.com/llvm/llvm-project/commit/e321c53f7bb8d61f64b33e8b1f6a97eb32eaaa69
DIFF: https://github.com/llvm/llvm-project/commit/e321c53f7bb8d61f64b33e8b1f6a97eb32eaaa69.diff
LOG: [C2x] Relaxing requirements for va_start
This implements WG14 N2975 relaxing requirements for va_start
(https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2975.pdf), which
does two things:
1) Allows the declaration of a variadic function without any named
arguments. e.g., void f(...) is now valid, as in C++.
2) Modified the signature of the va_start macro to be a variadic macro
that accepts one or more arguments; the second (and later) arguments
are not expanded or evaluated.
I followed the GCC implementation in terms of not modifying the
behavior of `__builtin_va_start` (it still requires exactly two
arguments), but this approach has led to several QoI issues that I've
documented with FIXME comments in the test. Specifically, the
requirement that we do not evaluate *or expand* the second and later
arguments means it's no longer possible to issue diagnostics for
compatibility with older C versions and C++. I am reaching out to
folks in WG14 to see if we can get an NB comment to address these
concerns (the US comment period has already closed, so I cannot file
the comment myself), so the diagnostic behavior may change in the
future.
I took this opportunity to add some documentation for all the related
builtins in this area, since there was no documentation for them yet.
Differential Revision: https://reviews.llvm.org/D139436
Added:
clang/test/C/C2x/n2975.c
Modified:
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Headers/stdarg.h
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaType.cpp
clang/www/c_status.html
Removed:
################################################################################
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index 2adf7ee421f4d..ad6e10cf9c6c2 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3229,6 +3229,52 @@ despite these functions accepting an argument of type ``const void*``.
Support for constant expression evaluation for the above builtins can be detected
with ``__has_feature(cxx_constexpr_string_builtins)``.
+Variadic function builtins
+--------------------------
+
+Clang provides several builtins for working with variadic functions from the C
+standard library ``<stdarg.h>`` header:
+
+* ``__builtin_va_list``
+
+A predefined typedef for the target-specific ``va_list`` type.
+
+* ``void __builtin_va_start(__builtin_va_list list, <parameter-name>)``
+
+A builtin function for the target-specific ``va_start`` function-like macro.
+The ``parameter-name`` argument is the name of the parameter preceding the
+ellipsis (``...``) in the function signature. Alternatively, in C2x mode or
+later, it may be the integer literal ``0`` if there is no parameter preceding
+the ellipsis. This function initializes the given ``__builtin_va_list`` object.
+It is undefined behavior to call this function on an already initialized
+``__builin_va_list`` object.
+
+* ``void __builtin_va_end(__builtin_va_list list)``
+
+A builtin function for the target-specific ``va_end`` function-like macro. This
+function finalizes the given ``__builtin_va_list`` object such that it is no
+longer usable unless re-initialized with a call to ``__builtin_va_start`` or
+``__builtin_va_copy``. It is undefined behavior to call this function with a
+``list`` that has not been initialized by either ``__builtin_va_start`` or
+``__builtin_va_copy``.
+
+* ``<type-name> __builtin_va_arg(__builtin_va_list list, <type-name>)``
+
+A builtin function for the target-specific ``va_arg`` function-like macro. This
+function returns the value of the next variadic argument to the call. It is
+undefined behavior to call this builtin when there is no next varadic argument
+to retrieve or if the next variadic argument does not have a type compatible
+with the given ``type-name``. The return type of the function is the
+``type-name`` given as the second argument. It is undefined behavior to call
+this function with a ``list`` that has not been initialized by either
+``__builtin_va_start`` or ``__builtin_va_copy``.
+
+* ``void __builtin_va_copy(__builtin_va_list dest, __builtin_va_list src)``
+
+A builtin function for the target-specific ``va_copy`` function-like macro.
+This function initializes ``dest`` as a copy of ``src``. It is undefined
+behavior to call this function with an already initialized ``dest`` argument.
+
Memory builtins
---------------
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 99782d74b165b..6bd038fbdc16f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -614,6 +614,20 @@ C2x Feature Support
void func(nullptr_t);
func(0); // Rejected in C, accepted in C++, Clang rejects
+- Implemented `WG14 N2975 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2975.pdf>`_,
+ Relax requirements for va_start. In C2x mode, functions can now be declared
+ fully variadic and the ``va_start`` macro no longer requires passing a second
+ argument (though it accepts a second argument for backwards compatibility).
+ If a second argument is passed, it is neither expanded nor evaluated in C2x
+ mode.
+
+ .. code-block:: c
+
+ void func(...) { // Invalid in C17 and earlier, valid in C2x and later.
+ va_list list;
+ va_start(list); // Invalid in C17 and earlier, valid in C2x and later.
+ va_end(list);
+ }
C++ Language Changes in Clang
-----------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9f19730bfd72b..2f7002e6e3a82 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9854,6 +9854,9 @@ def err_ms_va_start_used_in_sysv_function : Error<
def warn_second_arg_of_va_start_not_last_named_param : Warning<
"second argument to 'va_start' is not the last named parameter">,
InGroup<Varargs>;
+def warn_c17_compat_ellipsis_only_parameter : Warning<
+ "'...' as the only parameter of a function is incompatible with C standards "
+ "before C2x">, DefaultIgnore, InGroup<CPre2xCompat>;
def warn_va_start_type_is_undefined : Warning<
"passing %select{an object that undergoes default argument promotion|"
"an object of reference type|a parameter declared with the 'register' "
diff --git a/clang/lib/Headers/stdarg.h b/clang/lib/Headers/stdarg.h
index 4fbfe0985a160..ba978721f1f3d 100644
--- a/clang/lib/Headers/stdarg.h
+++ b/clang/lib/Headers/stdarg.h
@@ -22,7 +22,16 @@ typedef __builtin_va_list __gnuc_va_list;
typedef __builtin_va_list va_list;
#define _VA_LIST
#endif
+
+/* FIXME: This is using the placeholder dates Clang produces for these macros
+ in C2x mode; switch to the correct values once they've been published. */
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202000L
+/* C2x does not require the second parameter for va_start. */
+#define va_start(ap, ...) __builtin_va_start(ap, 0)
+#else
+/* Versions before C2x do require the second parameter. */
#define va_start(ap, param) __builtin_va_start(ap, param)
+#endif
#define va_end(ap) __builtin_va_end(ap)
#define va_arg(ap, type) __builtin_va_arg(ap, type)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index c89d55b58f5c2..4b3b52db52956 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -7228,6 +7228,9 @@ bool Sema::SemaBuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall) {
if (checkVAStartABI(*this, BuiltinID, Fn))
return true;
+ // In C2x mode, va_start only needs one argument. However, the builtin still
+ // requires two arguments (which matches the behavior of the GCC builtin),
+ // <stdarg.h> passes `0` as the second argument in C2x mode.
if (checkArgCount(*this, TheCall, 2))
return true;
@@ -7241,9 +7244,15 @@ bool Sema::SemaBuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall) {
return true;
// Verify that the second argument to the builtin is the last argument of the
- // current function or method.
+ // current function or method. In C2x mode, if the second argument is an
+ // integer constant expression with value 0, then we don't bother with this
+ // check.
bool SecondArgIsLastNamedArgument = false;
const Expr *Arg = TheCall->getArg(1)->IgnoreParenCasts();
+ if (Optional<llvm::APSInt> Val =
+ TheCall->getArg(1)->getIntegerConstantExpr(Context);
+ Val && LangOpts.C2x && *Val == 0)
+ return false;
// These are valid if SecondArgIsLastNamedArgument is false after the next
// block.
@@ -7284,7 +7293,6 @@ bool Sema::SemaBuiltinVAStart(unsigned BuiltinID, CallExpr *TheCall) {
Diag(ParamLoc, diag::note_parameter_type) << Type;
}
- TheCall->setType(Context.VoidTy);
return false;
}
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 100827ca1eb17..e7be7db890885 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -5371,14 +5371,19 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
} else {
// We allow a zero-parameter variadic function in C if the
// function is marked with the "overloadable" attribute. Scan
- // for this attribute now.
- if (!FTI.NumParams && FTI.isVariadic && !LangOpts.CPlusPlus)
- if (!D.getDeclarationAttributes().hasAttribute(
- ParsedAttr::AT_Overloadable) &&
- !D.getAttributes().hasAttribute(ParsedAttr::AT_Overloadable) &&
- !D.getDeclSpec().getAttributes().hasAttribute(
- ParsedAttr::AT_Overloadable))
+ // for this attribute now. We also allow it in C2x per WG14 N2975.
+ if (!FTI.NumParams && FTI.isVariadic && !LangOpts.CPlusPlus) {
+ if (LangOpts.C2x)
+ S.Diag(FTI.getEllipsisLoc(),
+ diag::warn_c17_compat_ellipsis_only_parameter);
+ else if (!D.getDeclarationAttributes().hasAttribute(
+ ParsedAttr::AT_Overloadable) &&
+ !D.getAttributes().hasAttribute(
+ ParsedAttr::AT_Overloadable) &&
+ !D.getDeclSpec().getAttributes().hasAttribute(
+ ParsedAttr::AT_Overloadable))
S.Diag(FTI.getEllipsisLoc(), diag::err_ellipsis_first_param);
+ }
if (FTI.NumParams && FTI.Params[0].Param == nullptr) {
// C99 6.7.5.3p3: Reject int(x,y,z) when it's not a function
diff --git a/clang/test/C/C2x/n2975.c b/clang/test/C/C2x/n2975.c
new file mode 100644
index 0000000000000..f5b94c256d00f
--- /dev/null
+++ b/clang/test/C/C2x/n2975.c
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -verify -ffreestanding -Wpre-c2x-compat -std=c2x %s
+
+/* WG14 N2975: partial
+ * Relax requirements for va_start
+ */
+
+#include <stdarg.h>
+
+#define DERP this is an error
+
+void func(...) { // expected-warning {{'...' as the only parameter of a function is incompatible with C standards before C2x}}
+ // Show that va_start doesn't require the second argument in C2x mode.
+ va_list list;
+ va_start(list); // FIXME: it would be nice to issue a portability warning to C17 and earlier here.
+ va_end(list);
+
+ // Show that va_start doesn't expand or evaluate the second argument.
+ va_start(list, DERP);
+ va_end(list);
+
+ // FIXME: it would be kinder to diagnose this instead of silently accepting it.
+ va_start(list, 1, 2);
+ va_end(list);
+
+ // We didn't change the behavior of __builtin_va_start (and neither did GCC).
+ __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__(__builtin_va_start(list, 0)), void), "");
+}
+
+// Show that function pointer types also don't need an argument before the
+// ellipsis.
+typedef void (*fp)(...); // expected-warning {{'...' as the only parameter of a function is incompatible with C standards before C2x}}
+
+// Passing something other than the argument before the ... is still not valid.
+void diag(int a, int b, ...) {
+ va_list list;
+ // FIXME: the call to va_start should also diagnose the same way as the call
+ // to __builtin_va_start. However, because va_start is not allowed to expand
+ // or evaluate the second argument, we can't pass it along to
+ // __builtin_va_start to get that diagnostic. So in C17 and earlier, we will
+ // diagnose this use through the macro, but in C2x and later we've lost the
+ // diagnostic entirely. GCC has the same issue currently.
+ va_start(list, a);
+ // However, the builtin itself is under no such constraints regarding
+ // expanding or evaluating the second argument, so it can still diagnose.
+ __builtin_va_start(list, a); // expected-warning {{second argument to 'va_start' is not the last named parameter}}
+ va_end(list);
+}
+
+void foo(int a...); // expected-error {{C requires a comma prior to the ellipsis in a variadic function type}}
+
+void use(void) {
+ // Demonstrate that we can actually call the variadic function when it has no
+ // formal parameters.
+ func(1, '2', 3.0, "4");
+ func();
+
+ // And that assignment still works as expected.
+ fp local = func;
+
+ // ...including conversion errors.
+ fp other_local = diag; // expected-error {{incompatible function pointer types initializing 'fp' (aka 'void (*)(...)') with an expression of type 'void (int, int, ...)'}}
+}
diff --git a/clang/www/c_status.html b/clang/www/c_status.html
index 82a40f2ec1d72..7ed2f67acbcfd 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -1162,7 +1162,7 @@ <h2 id="c2x">C2x implementation status</h2>
<tr>
<td>Relax requirements for va_start</td>
<td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2975.pdf">N2975</a></td>
- <td class="none" align="center">No</td>
+ <td class="unreleased" align="center">Clang 16</td>
</tr>
<tr>
<td>Enhanced enumerations</td>
More information about the cfe-commits
mailing list