[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

Fri Apr 15 13:24:33 PDT 2022

aaron.ballman added a comment.

In D122983#3454632 <https://reviews.llvm.org/D122983#3454632>, @rsmith wrote:

> In D122983#3454508 <https://reviews.llvm.org/D122983#3454508>, @aaron.ballman wrote:
>
>> In D122983#3454494 <https://reviews.llvm.org/D122983#3454494>, @jyknight wrote:
>>
>>> In D122983#3454406 <https://reviews.llvm.org/D122983#3454406>, @aaron.ballman wrote:
>>>
>>>> Out of curiosity -- do you think we should remove the `DefaultIgnore` in C89 mode so that we warn by default there (perhaps as a follow up)? Also, I presume you expect `diag::ext_implicit_lib_function_decl` to behave the same way (warn in C89, warn-as-err in C99 and up) as part of this patch?
>
> I think it would make sense to include the C89 warning in `-Wc99-compat`, but I agree with James that it doesn't seem worthwhile to enable it by default.

Ah, that's a good idea for a follow-up, thanks!

>> I feel a bit more strongly that implicit builtins should be handled the same as other implicit functions (in terms of `DefaultError` behavior), unless there's some scenarios I'm not thinking of.
>
> I'm not sure which case you mean by "implicit builtins". Two cases seem relevant:
>
> - `ext_implicit_lib_function_decl`, for things like using `strlen` without including its header; I think that should behave the same as using any other undeclared function name (error by default in C99 onwards, error with no warning flag in C2x onwards even though we would synthesize the correct prototype).

Great, this was the one I was talking about, and that's the approach I'd like to take with it. I'll have that in the next revision.

> - `warn_builtin_unknown`, for things like using `__builtin_no_such_builtin_exists()` (a function name starting `__builtin_` that is not a known builtin and hasn't been declared), which is currently a warning-as-error in all language modes; I think that should stay an error by default in all modes, and I'd be happy to see it turn into a harder error (with no warning flag), whether that happens only in C2x onwards, in C99 onwards, or in all modes.

Okay, I hadn't touched that one because it was already default error. I'm happy to try to turn it into a hard error in a followup patch.

Thanks for the feedback!

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983