[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#3511682 <https://reviews.llvm.org/D122895#3511682>, @aaron.ballman wrote:

> In D122895#3511632 <https://reviews.llvm.org/D122895#3511632>, @jyknight wrote:
>
>> The warnings for this case aren't great:
>>
>>   int foo();
>>   
>>   int
>>   foo(int arg) {
>>     return 5;
>>   }
>
> Yeah, that's not ideal, I'm looking into it to see if I can improve that scenario or not.

There's not much to be done about the strict prototype warning for the declaration; the issue is that we need to give the strict prototypes warning when forming the function *type* for the declaration, which is long before we have any idea there's a subsequent definition (also, because this is for forming the type, we don't know what declarations, if any, may be related, so there's no real way to improve the fix-it behavior). However, I think the second warning is just bad wording -- we know we have a function definition with a prototype for this particular diagnostic. However, there's the redeclaration case to consider at the same time. So here's the full test and the best behavior that I can come up with so far:

  void foo();
  void foo(int arg);
  
  void bar();
  void bar(int arg) {}

With `-Wstrict-prototypes` enabled:

  F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only -Wstrict-prototypes "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c"
  C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:1:9: warning: a function declaration without a prototype
        is deprecated in all versions of C [-Wstrict-prototypes]
  void foo();
          ^
           void
  C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:6: warning: this function declaration with a prototype
        will change behavior in C2x because of a previous declaration with no prototype [-Wdeprecated-non-prototype]
  void foo(int arg);
       ^
  C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:4:9: warning: a function declaration without a prototype
        is deprecated in all versions of C [-Wstrict-prototypes]
  void bar();
          ^
           void
  C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:5:6: warning: this function definition with a prototype
        will change behavior in C2x because of a previous declaration with no prototype [-Wdeprecated-non-prototype]
  void bar(int arg) {}
       ^
  4 warnings generated.

With `-Wstrict-prototypes` disabled:

  F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c"
  C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:1:6: warning: a function declaration without a prototype
        is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
  void foo();
       ^
           void
  C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:6: warning: this function declaration with a prototype
        will change behavior in C2x because of a previous declaration with no prototype [-Wdeprecated-non-prototype]
  void foo(int arg);
       ^
  C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:4:6: warning: a function declaration without a prototype
        is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
  void bar();
       ^
           void
  C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:5:6: warning: this function definition with a prototype
        will change behavior in C2x because of a previous declaration with no prototype [-Wdeprecated-non-prototype]
  void bar(int arg) {}
       ^
  4 warnings generated.

It's not ideal, but it's about the most workable solution I can come up with that doesn't regress far more important cases. It'd be nice if we had a note instead of leaning on the previous warning like we're doing, but I still claim this is defensible given how rare the situation is that you'd declare without a prototype and later redeclare (perhaps through a definition) with a non-`void` prototype. Note (it's easy to miss with the wall of text), the difference between the two runs is that when strict prototypes are enabled, we issue the strict prototype warning on the first declaration and when strict prototypes are disabled, we issue the "is not supported in C2x" diagnostic on the first declaration -- but in either case the intention is to alert the user to which previous declaration has no prototype for the subsequent declaration/definition that caused the issue.

`this function declaration|definition with a prototype will change behavior in C2x because of a previous declaration with no prototype` is the new diagnostic wording I've got so far, and I'm not strongly tied to it, if you have suggestions to improve it. I would also be happy with `this function declaration|definition with a prototype is not supported in C2x because of a previous declaration with no prototype` or something along those lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122895