[PATCH] D58091: Customize warnings for missing built-in type

[Johannes Doerfert via Phabricator via cfe-commits]

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