[Headers] Add missing __need_ macros to stdarg.h

[iana]

Summary

Apple needs a __need_ macro for va_list. Add one, and also ones for va_start/va_arg/va_end, __va_copy, va_copy.

[MaskRay]

Apple needs a need_ macro for va_list. Add one, and also ones for va_start/va_arg/va_end, va_copy, va_copy.

Do you have a link to the specification or the source that this is needed?

[iana]

In D157793#4583663, @MaskRay wrote:

Apple needs a need_ macro for va_list. Add one, and also ones for va_start/va_arg/va_end, va_copy, va_copy.

Do you have a link to the specification or the source that this is needed?

We need this so that our <sys/_types/_va_list.h> doesn't have to redeclare va_list and doesn't pull in all of stdarg.h either. As far as I know there's no specification for this (or stddef.h) being include-able in pieces.

[aaron.ballman]

In D157793#4583698, @iana wrote:

In D157793#4583663, @MaskRay wrote:

Apple needs a need_ macro for va_list. Add one, and also ones for va_start/va_arg/va_end, va_copy, va_copy.

Do you have a link to the specification or the source that this is needed?

We need is so that our <sys/_types/_va_list.h> doesn't have to redeclare va_list and doesn't pull in all of stdarg.h either. As far as I know there's no specification for this (or stddef.h) being include-able in pieces.

I'm not opposed, but this adds significant complexity to support a questionable use case -- the C standard library headers are not intended to be partially included, so if the plan is to use these __need macros in all of the headers we provide, I think that should go through an RFC process to be sure we want to maintain the added complexity. (Also, who is "we" in this case?)

[iana]

In D157793#4585268, @aaron.ballman wrote:

In D157793#4583698, @iana wrote:

In D157793#4583663, @MaskRay wrote:

Apple needs a need_ macro for va_list. Add one, and also ones for va_start/va_arg/va_end, va_copy, va_copy.

Do you have a link to the specification or the source that this is needed?

We need is so that our <sys/_types/_va_list.h> doesn't have to redeclare va_list and doesn't pull in all of stdarg.h either. As far as I know there's no specification for this (or stddef.h) being include-able in pieces.

I'm not opposed, but this adds significant complexity to support a questionable use case -- the C standard library headers are not intended to be partially included, so if the plan is to use these __need macros in all of the headers we provide, I think that should go through an RFC process to be sure we want to maintain the added complexity. (Also, who is "we" in this case?)

We is Apple. We have a <sys/_types/_va_list.h> header that is generating redeclaration warnings in modules mode. I can't just include <stdarg.h> instead since it would change the semantics of <sys/_types/_va_list.h>. stdarg.h and stddef.h appear to be the only clang headers that Apple needs to use in pieces though, I don't think we should add this complexity to any other headers for consistency or "just because".

[aaron.ballman]

In D157793#4585685, @iana wrote:

In D157793#4585268, @aaron.ballman wrote:

In D157793#4583698, @iana wrote:

In D157793#4583663, @MaskRay wrote:

Apple needs a need_ macro for va_list. Add one, and also ones for va_start/va_arg/va_end, va_copy, va_copy.

Do you have a link to the specification or the source that this is needed?

We need is so that our <sys/_types/_va_list.h> doesn't have to redeclare va_list and doesn't pull in all of stdarg.h either. As far as I know there's no specification for this (or stddef.h) being include-able in pieces.

I'm not opposed, but this adds significant complexity to support a questionable use case -- the C standard library headers are not intended to be partially included, so if the plan is to use these __need macros in all of the headers we provide, I think that should go through an RFC process to be sure we want to maintain the added complexity. (Also, who is "we" in this case?)

We is Apple.

Thank you!

We have a <sys/_types/_va_list.h> header that is generating redeclaration warnings in modules mode. I can't just include <stdarg.h> instead since it would change the semantics of <sys/_types/_va_list.h>. stdarg.h and stddef.h appear to be the only clang headers that Apple needs to use in pieces though, I don't think we should add this complexity to any other headers for consistency or "just because".

Thank you for the explanation; if it's just these two files, I think that's pretty reasonable. I was worried it was also going to spill into limits.h, stdint.h, and other freestanding headers.

clang/test/Headers/stdarg.c

3

I think the triples can be removed.

12

You should spell out the first instance of the diagnostic

35

You should spell out these diagnostics, and I think c99-no-diagnostics should be placed up by the RUN lines so it's more obvious that we expect no diagnostics in C99 mode.

Actually, this file should perhaps be split into two files as they're testing different things. (I was tripped up seeing no-diagnostics but we have c99-error entries above, that's when I realized the split file was being used differently in the RUN lines which is a bit novel.) But I'm not certain I fully understand what your comment means about why we're using split file in the first place, so I might be missing something.

clang/test/Headers/stdargneeds.c

3

Same suggestions in this file regarding triples and spelling out diagnostics, but I think a single file makes sense as this is testing each of the __needs_whatever macros the same way.

[iana]

Review feedback

clang/test/Headers/stdarg.c

35

It's only trying to test what including <stdarg.h> gets you by default. The first chunk is to prove that nothing is provided via built-ins or anything like if you don't include anything. The second chunk shows that you get the expected declarations in each standard mode if you include <stdarg.h> with no __need_ macros.

The problem is this.

va_copy(g, v); // The first time you get: implicitly declaring library function 'va_copy'

va_copy(g, v); // But now the compiler has decided that va_copy has a declaration, so you don't get any diagnostics even though va_copy doesn't have its real declaration, I think the compiler assumes 'int va_copy(int, int)' or something like that.

Maybe we don't need to test the include-nothing case both here and stdargneeds.c?

D157757 has the same problem for offsetof but since it uses C23 also, the diagnostics get repeated.

[aaron.ballman]

LGTM!

clang/test/Headers/stdarg.c

35

Ah, I see now, thank you for the explanation.

The compiler adds an implicit function declaration for va_copy in C (so the signature is int func() as a K&R C function without a prototype), hence the need to split the file.

This seems reasonable to me.

[jyknight]

I'm a bit confused by this change vs its description.

It looks like stdarg already supported __need___va_list, which is what you said Apple needs. Does Apple also require the other __need_va_copy/etc, too? If not, why add support for them? (I think we should prefer not to add support for pieces that are not actually needed)

[iana]

In D157793#4588909, @jyknight wrote:

I'm a bit confused by this change vs its description.

It looks like stdarg already supported __need___va_list, which is what you said Apple needs. Does Apple also require the other __need_va_copy/etc, too? If not, why add support for them? (I think we should prefer not to add support for pieces that are not actually needed)

That name is misleading, __need___va_list gives you __gnuc_va_list and not va_list.

[iana]

Rebase

clang/lib/Headers/stdarg.h

 /*===---- stdarg.h - Variable argument handling ----------------------------===
  *
  * 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 __STDARG_H
+#if !defined(__STDARG_H) || defined(__need___va_list) ||                       \
+    defined(__need_va_list) || defined(__need_va_arg) ||                       \
+    defined(__need___va_copy) || defined(__need_va_copy)
+
+#if !defined(__need___va_list) && !defined(__need_va_list) &&                  \
+    !defined(__need_va_arg) && !defined(__need___va_copy) &&                   \
+    !defined(__need_va_copy)
+#define __STDARG_H
+#define __need___va_list
+#define __need_va_list
+#define __need_va_arg
+#define __need___va_copy
+/* GCC always defines __va_copy, but does not define va_copy unless in c99 mode
+ * or -ansi is not specified, since it was not part of C90.
+ */
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L) ||              \
+    (defined(__cplusplus) && __cplusplus >= 201103L) ||                        \
+    !defined(__STRICT_ANSI__)
+#define __need_va_copy
+#endif
+#endif
 
+#ifdef __need___va_list
 #ifndef __GNUC_VA_LIST
 #define __GNUC_VA_LIST
 typedef __builtin_va_list __gnuc_va_list;
 #endif
-
-#ifdef __need___va_list
 #undef __need___va_list
-#else
-#define __STDARG_H
+#endif /* defined(__need___va_list) */
+
+#ifdef __need_va_list
 #ifndef _VA_LIST
 typedef __builtin_va_list va_list;
 #define _VA_LIST
 #endif
+#undef __need_va_list
+#endif /* defined(__need_va_list) */
 
+#ifdef __need_va_arg
 #if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L
 /* C23 does not require the second parameter for va_start. */
 #define va_start(ap, ...) __builtin_va_start(ap, 0)
 #else
 /* Versions before C23 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)
+#undef __need_va_arg
+#endif /* defined(__need_va_arg) */
 
-/* GCC always defines __va_copy, but does not define va_copy unless in c99 mode
- * or -ansi is not specified, since it was not part of C90.
- */
+#ifdef __need___va_copy
 #define __va_copy(d,s) __builtin_va_copy(d,s)
+#undef __need___va_copy
+#endif /* defined(__need___va_copy) */
 
-#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L) ||              \
-    (defined(__cplusplus) && __cplusplus >= 201103L) ||                        \
-    !defined(__STRICT_ANSI__)
+#ifdef __need_va_copy
 #define va_copy(dest, src)  __builtin_va_copy(dest, src)
-#endif
-
-#endif /* __STDARG_H */
+#undef __need_va_copy
+#endif /* defined(__need_va_copy) */
 
-#endif /* not __STDARG_H */
+#endif

clang/test/Headers/stdarg.c

+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -verify=c89 -Werror=implicit-function-declaration -std=c89 %t/stdarg0.c
+// RUN: %clang_cc1 -fsyntax-only -verify=c99 -Werror=implicit-function-declaration -std=c99 %t/stdarg0.c
+// RUN: %clang_cc1 -fsyntax-only -verify=c89 -Werror=implicit-function-declaration -std=c89 %t/stdarg1.c
+// RUN: %clang_cc1 -fsyntax-only -verify=c99 -Werror=implicit-function-declaration -std=c99 %t/stdarg1.c
+
+// Split the file so that the "implicitly declaring library function" errors get repeated.
+
+//--- stdarg0.c
+static void f(int p, ...) {
+    __gnuc_va_list g; // c89-error{{undeclared identifier '__gnuc_va_list'}} c99-error{{undeclared identifier}}
+    va_list v; // c89-error{{undeclared identifier 'va_list'}} c99-error{{undeclared identifier}}
+    va_start(v, p); // c89-error{{implicitly declaring library function 'va_start'}} c89-note{{include the header <stdarg.h> or explicitly provide a declaration for 'va_start'}} c89-error{{undeclared identifier 'v'}} \
+                       c99-error{{call to undeclared library function 'va_start'}} c99-note{{provide a declaration}} c99-error{{undeclared identifier}}
+    int i = va_arg(v, int); // c89-error{{implicit declaration of function 'va_arg'}} c89-error{{expected expression}} c89-error{{use of undeclared identifier 'v'}} \
+                               c99-error{{call to undeclared function 'va_arg'}} c99-error{{expected expression}} c99-error{{undeclared identifier}}
+    va_end(v); // c89-error{{implicitly declaring library function 'va_end'}} c89-note{{include the header <stdarg.h> or explicitly provide a declaration for 'va_end'}} c89-error{{undeclared identifier 'v'}} \
+                  c99-error{{call to undeclared library function 'va_end'}} c99-note{{provide a declaration}} c99-error{{undeclared identifier}}
+    __va_copy(g, v); // c89-error{{implicit declaration of function '__va_copy'}} c89-error{{use of undeclared identifier 'g'}} c89-error{{use of undeclared identifier 'v'}} \
+                        c99-error{{call to undeclared function '__va_copy'}} c99-error{{undeclared identifier}} c99-error{{undeclared identifier}}
+    va_copy(g, v); // c89-error{{implicitly declaring library function 'va_copy'}} c89-note{{include the header <stdarg.h> or explicitly provide a declaration for 'va_copy'}} c89-error{{use of undeclared identifier 'g'}} c89-error{{use of undeclared identifier 'v'}} \
+                      c99-error{{call to undeclared library function 'va_copy'}} c99-note{{provide a declaration}} c99-error{{undeclared identifier}} c99-error{{undeclared identifier}}
+}
+
+//--- stdarg1.c
+// c99-no-diagnostics
+
+#include <stdarg.h>
+static void f(int p, ...) {
+    __gnuc_va_list g;
+    va_list v;
+    va_start(v, p);
+    int i = va_arg(v, int);
+    va_end(v);
+    __va_copy(g, v);
+    va_copy(g, v); // c89-error{{implicitly declaring library function}} c89-note{{provide a declaration}}
+}

clang/test/Headers/stdargneeds.c

+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds0.c
+// RUN: %clang_cc1 -fsyntax-only -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds1.c
+// RUN: %clang_cc1 -fsyntax-only -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds2.c
+// RUN: %clang_cc1 -fsyntax-only -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds3.c
+// RUN: %clang_cc1 -fsyntax-only -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds4.c
+// RUN: %clang_cc1 -fsyntax-only -verify -Werror=implicit-function-declaration -std=c89 %t/stdargneeds5.c
+
+// Split the file so that the "implicitly declaring library function" errors get repeated.
+// Use C89 to verify that __need_ can be used to get types that wouldn't normally be available.
+
+//--- stdargneeds0.c
+static void f(int p, ...) {
+    __gnuc_va_list g; // expected-error{{undeclared identifier '__gnuc_va_list'}}
+    va_list v; // expected-error{{undeclared identifier 'va_list'}}
+    va_start(v, p); // expected-error{{implicitly declaring library function 'va_start'}} expected-note{{include the header <stdarg.h> or explicitly provide a declaration for 'va_start'}} expected-error{{undeclared identifier 'v'}}
+    int i = va_arg(v, int); // expected-error{{implicit declaration of function 'va_arg'}} expected-error{{expected expression}} expected-error{{use of undeclared identifier 'v'}}
+    va_end(v); // expected-error{{implicitly declaring library function 'va_end'}} expected-note{{include the header <stdarg.h> or explicitly provide a declaration for 'va_end'}} expected-error{{undeclared identifier 'v'}}
+    __va_copy(g, v); // expected-error{{implicit declaration of function '__va_copy'}} expected-error{{use of undeclared identifier 'g'}} expected-error{{use of undeclared identifier 'v'}}
+    va_copy(g, v); // expected-error{{implicitly declaring library function 'va_copy'}} expected-note{{include the header <stdarg.h> or explicitly provide a declaration for 'va_copy'}} expected-error{{use of undeclared identifier 'g'}} expected-error{{use of undeclared identifier 'v'}}
+}
+
+//--- stdargneeds1.c
+#define __need___va_list
+#include <stdarg.h>
+static void f(int p, ...) {
+    __gnuc_va_list g;
+    va_list v; // expected-error{{undeclared identifier}}
+    va_start(v, p); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}} expected-error{{undeclared identifier}}
+    int i = va_arg(v, int); // expected-error{{implicit declaration of function}} expected-error{{expected expression}} expected-error{{undeclared identifier}}
+    va_end(v); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}} expected-error{{undeclared identifier}}
+    __va_copy(g, v); // expected-error{{implicit declaration of function}} expected-error{{undeclared identifier}}
+    va_copy(g, v); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}} expected-error{{undeclared identifier}}
+}
+
+//--- stdargneeds2.c
+#define __need_va_list
+#include <stdarg.h>
+static void f(int p, ...) {
+    __gnuc_va_list g; // expected-error{{undeclared identifier}}
+    va_list v;
+    va_start(v, p); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}}
+    int i = va_arg(v, int); // expected-error{{implicit declaration of function}} expected-error{{expected expression}}
+    va_end(v); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}}
+    __va_copy(g, v); // expected-error{{implicit declaration of function}} expected-error{{undeclared identifier}}
+    va_copy(g, v); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}} expected-error{{undeclared identifier}}
+}
+
+//--- stdargneeds3.c
+#define __need_va_list
+#define __need_va_arg
+#include <stdarg.h>
+static void f(int p, ...) {
+    __gnuc_va_list g; // expected-error{{undeclared identifier}}
+    va_list v;
+    va_start(v, p);
+    int i = va_arg(v, int);
+    va_end(v);
+    __va_copy(g, v); // expected-error{{implicit declaration of function}} expected-error{{undeclared identifier}}
+    va_copy(g, v); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}} expected-error{{undeclared identifier}}
+}
+
+//--- stdargneeds4.c
+#define __need___va_list
+#define __need_va_list
+#define __need___va_copy
+#include <stdarg.h>
+static void f(int p, ...) {
+    __gnuc_va_list g;
+    va_list v;
+    va_start(v, p); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}}
+    int i = va_arg(v, int); // expected-error{{implicit declaration of function}} expected-error{{expected expression}}
+    va_end(v); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}}
+    __va_copy(g, v);
+    va_copy(g, v); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}}
+}
+
+//--- stdargneeds5.c
+#define __need___va_list
+#define __need_va_list
+#define __need_va_copy
+#include <stdarg.h>
+static void f(int p, ...) {
+    __gnuc_va_list g;
+    va_list v;
+    va_start(v, p); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}}
+    int i = va_arg(v, int); // expected-error{{implicit declaration of function}} expected-error{{expected expression}}
+    va_end(v); // expected-error{{implicitly declaring library function}} expected-note{{provide a declaration}}
+    __va_copy(g, v); // expected-error{{implicit declaration of function}}
+    va_copy(g, v);
+}