aaron.ballman marked 4 inline comments as done.
aaron.ballman 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
----------------
cor3ntin wrote:
> 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!
For the moment, I'd rather match what GCC is doing (deficiencies and all) until
it's clear what actually gets into the final standard. We can revisit once WG14
has said their piece.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:7230
+ Val && LangOpts.C2x && *Val == 0)
+ return false;
----------------
erichkeane wrote:
> I see below that we still set the return type of hte call to Void. Do we
> wnat to keep that line, or at least, move it to the top of this function?
I added test coverage and it seems that setting the return type is not needed
(by my understanding, `CreateBuiltin` requires the return type to be passed to
it explicitly, and `LazilyCreateBuiltin` uses the builtin type string to
specify *at least* the return type information (even if custom type checking
happens for the rest of the signature).
================
Comment at: clang/test/C/C2x/n2975.c:49
+
+void foo(int a...); // expected-error {{C requires a comma prior to the
ellipsis in a variadic function type}}
----------------
erichkeane wrote:
> erichkeane wrote:
> > .
> Was going to complain about the error message here, but I'm on the fence
> whether we should say THIS (the required comma), or something more
> descriptive, but at least this is better than GCC just failing in normal
> parsing.
FWIW, this is existing diagnostic wording and behavior. I think it's
reasonable, but if we want to do something better, we can fix it in a follow-up.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139436/new/
https://reviews.llvm.org/D139436
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
