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

2022-12-08 Thread Aaron Ballman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe321c53f7bb8: [C2x] Relaxing requirements for va_start (authored by aaron.ballman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139436/new/ https://review

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

2022-12-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 480600. aaron.ballman added a comment. Adding some more test cases that demonstrate that you can still call these functions as you'd expect and that function pointer assignment works as expected. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

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

2022-12-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaChecking.cpp:7230 + Val && LangOpts.C2x && *Val == 0) +return false; aaron.ballman wrote: > erichkeane

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

2022-12-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 480533. aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. Update based on review feedback. - Removed code setting the return type for the builtin (it's already set elsewhere) - Added test coverage verifying the return type

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

2022-12-06 Thread Aaron Ballman via Phabricator via cfe-commits
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 wrot

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

2022-12-06 Thread Corentin Jabot via Phabricator via cfe-commits
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 th

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

2022-12-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. 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: > . Was going to complain about the error message here

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

2022-12-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Give others an extra chance to look at this, but other than the 1 concern, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139436/new/ https://reviews.llvm.org/D139436 ___ cfe-commits mailing list cfe-commits@l

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

2022-12-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:7230 + Val && LangOpts.C2x && *Val == 0) +return false; 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 i

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

2022-12-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 480522. aaron.ballman marked 4 inline comments as done. aaron.ballman added a comment. Updating based on review feedback. - Rolled back parsing changes as they were not needed - Moved the diagnostic from common kinds to sema kinds because it's now solel

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

2022-12-06 Thread Aaron Ballman via Phabricator via cfe-commits
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 wrot

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

2022-12-06 Thread Corentin Jabot via Phabricator via cfe-commits
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 Yeah, i really think there should be some king of diagnostic here.

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

2022-12-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:7356 +// 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) Is this rig

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

2022-12-06 Thread Aaron Ballman via Phabricator via cfe-commits
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_s