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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to