[PATCH] D139436: [C2x] Relaxing requirements for va_start

Tue Dec 6 09:42:31 PST 2022

cor3ntin added inline comments.

================
Comment at: clang/lib/Headers/stdarg.h:30
+/* C2x does not require the second parameter for va_start. */
+#define va_start(ap, ...) __builtin_va_start(ap, 0)
+#else
----------------
aaron.ballman wrote:
> cor3ntin wrote:
> > Yeah, i really think there should be some king of diagnostic here.
> > Maybe `__builtin_va_start` could take an arbitrary number of arguments and diagnose for more that 2 args or if the second arg is not valid?
> > is it a matter of being compatible with gcc? do we need to?
> We try pretty hard to keep our builtins compatible with GCC; that makes everyone's lives easier (most especially standard library authors). If we did anything here, it would be to add a new builtin specific for the new variant. However, that won't save us because of the standard's requirement to not *expand* the variadic arguments. We'd need that builtin to be named `va_start` and we'd have to get rid of this macro (otherwise, the arguments passed to the builtin will be expanded as they're lexed). But `va_start` is defined by the standard as being a macro specifically, so we'd still need to make `#if defined(va_start)` work. Ultimately, I don't think it's worth that level of heroics -- instead, I am hoping WG14 will relax the requirement that the arguments not be expanded or evaluated; that gives us options.
We probably can reuse the same macro name (we'd still have to handle the 0 case though), but if you'd rather be conforming now and revisit the issue when WG14 remove the limitation that parameters are not expanded, I'm fine with it!

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139436/new/

https://reviews.llvm.org/D139436