jdoerfert added a comment. In D58091#1396437 <https://reviews.llvm.org/D58091#1396437>, @aaron.ballman wrote:
> In D58091#1396397 <https://reviews.llvm.org/D58091#1396397>, @jdoerfert wrote: > > > In D58091#1396382 <https://reviews.llvm.org/D58091#1396382>, @aaron.ballman > > wrote: > > > > > - but I wonder why those diagnostics are happening in the first place. It > > > seems like the warning is still useful when it triggers outside of that > > > situation, no? > > > > > > The underlying conceptual problem, which I didn't know when I added > > `GE_Missing_type`, is that this has _nothing_ to do with the location of > > the declaration. We say, include the header X.h, if we were not able to > > build a type for recognized built-in Y that should be declared in X.h. > > However, we should report _why_ we could not build the type instead. For > > built-ins we do not have a type on record (`GE_Missing_type`), this is > > always, so no warning for now. For the ones that we only fail to build a > > type because some requirement is missing, we should report that, at least > > when we are in the respective header. I don't have a perfect solution of > > what to do actually. > > > > I could check if the declaration is (probably) in the respective header so > > we can switch between warnings? > > > That's kind of what I was wondering, but I deal with builtins so infrequently > that my expectations may be wrong here. If a user declares a builtin with a > conflicting type outside of a header file, that seems like we'd want to warn > the user about right? But this seems to remove that warning, at least in the > case of test/Sema/implicit-builtin-decl.c:71. Or do I misunderstand the > situation causing the warning to trigger? After this, we should still warn for all builtins for which we have an expected type on record. I added the `clang/test/Sema/builtin-setjmp.c` test to check for this situation. Here, `setjmp` is declared outside of the header (but it actually doesn't matter as I mentioned in the above comment). If you declare it without defining `jmp_buf` first, that is what the kernel ppl did, you will get a warning that states `jmp_buf` is unknown and we require it for the declaration of `setjmp/longjmp/...` (line 5). If you define `jmp_buf` and then declare `setjmp` with a conflicting type, that is not `T setjmp(jmp_buf)`, you will see the incompatible redeclaration warning (line 8). Does that make sense? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58091/new/ https://reviews.llvm.org/D58091 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
