[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122895#3481564 <https://reviews.llvm.org/D122895#3481564>, @manojgupta wrote:

> Some of our users are not very happy with the churn probably caused by this change where the declaration has the "void" argument but the later definition does not have explicit "void".
>
>   void foo(void);
>   
>   void foo() 
>   {
>   }
>
> GCC  does not warn about this usage: https://godbolt.org/z/zPP8qjc98
>
> Any opinions?

Thank you for the feedback! I think it's debatable whether this is a bug or not, but it's an interesting case that I may think about a bit further before making any final decisions. My current thinking is...

We turned `-Wstrict-prototypes` into a pedantic warning specifically telling the user about K&R C function usage everywhere it occurs, and from that perspective `void f() {}` is correct to diagnose pedantically as the signature is specifically a K&R C one. However, the composite type between `void f(void)` (with a parameter list) and `void f() {}` (with an empty identifier list) is a function with a prototype of `void(void)` (per C99 6.2.7p3). Thus, a call to the function through the declaration can pass no arguments, and a call to the function through the definition can pass no arguments, and so it also seems reasonable to not diagnose.

(We added `-Wdeprecated-non-prototype` to cover the case where there's a behavioral change to the code in C2x, which diagnoses `void f(int);` and `void f(i) int i; {}` (this code will break in C2x), so despite the similarity with your case, it's a different diagnostic entirely.)

FWIW, GCC's behavior isn't particularly interesting because we're knowingly diverging from their strategy with the warning, so it's not too surprising that our behavior is different.

I think what sells it for me not being a bug is that we issue that warning when determining the function *type* when forming the declaration which happens before doing function merging. At the point where we issue this warning, we don't have enough information to know whether the function will eventually have a composite type with a prototype or not (we don't even have enough information to know whether it's for a function declaration or not; it could be for a typedef, etc), so suppressing the diagnostic would be rather difficult. And because it's a pedantic warning specifically, I think it's defensible that we're issuing the diagnostic.

Is disabling the pedantic warning an option for your users?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122895