aaron.ballman created this revision.
aaron.ballman added reviewers: clang-language-wg, efriedma, jyknight,
hubert.reinterpretcast.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D139436
Files:
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticCommonKinds.td
clang/lib/Headers/stdarg.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaType.cpp
clang/test/C/C2x/n2975.c
clang/www/c_status.html
Index: clang/www/c_status.html
===================================================================
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -1162,7 +1162,7 @@
<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>
Index: clang/test/C/C2x/n2975.c
===================================================================
--- /dev/null
+++ clang/test/C/C2x/n2975.c
@@ -0,0 +1,47 @@
+// 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}}
+}
+
+// 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);
+}
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5371,14 +5371,19 @@
} 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_missing_comma_before_ellipsis);
+ 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
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -7203,6 +7203,9 @@
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;
@@ -7216,9 +7219,15 @@
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 the
+ // integer literal 0, then we don't bother with this check.
bool SecondArgIsLastNamedArgument = false;
const Expr *Arg = TheCall->getArg(1)->IgnoreParenCasts();
+ bool SecondArgIsElided = false;
+ if (Optional<llvm::APSInt> Val =
+ TheCall->getArg(1)->getIntegerConstantExpr(Context);
+ Val)
+ SecondArgIsElided = LangOpts.C2x && *Val == 0;
// These are valid if SecondArgIsLastNamedArgument is false after the next
// block.
@@ -7237,10 +7246,11 @@
}
}
- if (!SecondArgIsLastNamedArgument)
- Diag(TheCall->getArg(1)->getBeginLoc(),
- diag::warn_second_arg_of_va_start_not_last_named_param);
- else if (IsCRegister || Type->isReferenceType() ||
+ if (!SecondArgIsLastNamedArgument) {
+ if (!SecondArgIsElided)
+ Diag(TheCall->getArg(1)->getBeginLoc(),
+ diag::warn_second_arg_of_va_start_not_last_named_param);
+ } else if (IsCRegister || Type->isReferenceType() ||
Type->isSpecificBuiltinType(BuiltinType::Float) || [=] {
// Promotable integers are UB, but enumerations need a bit of
// extra checking to see what their promotable type actually is.
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -7352,9 +7352,14 @@
if (TryConsumeToken(tok::ellipsis, EllipsisLoc)) {
if (!getLangOpts().CPlusPlus) {
// We have ellipsis without a preceding ',', which is ill-formed
- // in C. Complain and provide the fix.
- Diag(EllipsisLoc, diag::err_missing_comma_before_ellipsis)
- << FixItHint::CreateInsertion(EllipsisLoc, ", ");
+ // in C before C2x. Complain and provide the fix. We do not support
+ // this as an extension in earlier C language modes.
+ if (getLangOpts().C2x)
+ Diag(EllipsisLoc,
+ diag::warn_c17_compat_missing_comma_before_ellipsis);
+ else
+ Diag(EllipsisLoc, diag::err_missing_comma_before_ellipsis)
+ << FixItHint::CreateInsertion(EllipsisLoc, ", ");
} else if (ParmDeclarator.getEllipsisLoc().isValid() ||
Actions.containsUnexpandedParameterPacks(ParmDeclarator)) {
// It looks like this was supposed to be a parameter pack. Warn and
Index: clang/lib/Headers/stdarg.h
===================================================================
--- clang/lib/Headers/stdarg.h
+++ clang/lib/Headers/stdarg.h
@@ -22,7 +22,16 @@
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)
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -126,6 +126,9 @@
def warn_missing_type_specifier : Warning<
"type specifier missing, defaults to 'int'">,
InGroup<ImplicitInt>, DefaultIgnore;
+def warn_c17_compat_missing_comma_before_ellipsis : Warning<
+ "'...' as the only parameter of a function is incompatible with C standards "
+ "before C2x">, DefaultIgnore, InGroup<CPre2xCompat>;
}
let CategoryName = "Nullability Issue" in {
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -612,6 +612,20 @@
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
-----------------------------
Index: clang/docs/LanguageExtensions.rst
===================================================================
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -3229,6 +3229,52 @@
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
---------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
