aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM! ================ Comment at: clang/test/Headers/stdarg.c:34 + __va_copy(g, v); + va_copy(g, v); // c89-error{{implicit}} c89-note{{va_copy}} c99-no-diagnostics +} ---------------- iana wrote: > aaron.ballman wrote: > > 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. > 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. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157793/new/ https://reviews.llvm.org/D157793 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits