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

Reply via email to