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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a subscriber: arphaman.
dexonsmith added a comment.

In D122895#3511376 <https://reviews.llvm.org/D122895#3511376>, @aaron.ballman wrote:

> In D122895#3511312 <https://reviews.llvm.org/D122895#3511312>, @aaron.ballman wrote:
>
>> However, I think the blocks behavior is a bug based on this: https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaType.cpp#L728 and I'd be more than happy to fix that, as I'm not checking that condition here: https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaType.cpp#L5579 which seems like a pretty obvious thing to be checking for before warning about not having a strict prototype.
>
> I fixed this false positive bug in 4be105c98a9c7e083cd878ee1751e11160b97b4a <https://reviews.llvm.org/rG4be105c98a9c7e083cd878ee1751e11160b97b4a>, so blocks (and OpenCL) behavior should now be improved.

Thanks! I think that's the bigger issue of the two. @steven_wu or @arphaman will have more context though.

In D122895#3511312 <https://reviews.llvm.org/D122895#3511312>, @aaron.ballman wrote:

> In D122895#3510165 <https://reviews.llvm.org/D122895#3510165>, @dexonsmith wrote:
>
>> For additional context to my questions above, even though open source clang hasn't been using `-Wstrict-prototypes`, Xcode has had it on-by-default in new projects since sometime in 2017, with project modernizations to turn it on for old projects.
>
> Thanks, that's very good to know! And also, thank you for raising the questions here, I appreciate the discussion.
>
>> Warning on block and function definitions such as `^(){}` and `void f1() {}`, which are pedantically lacking a prototype but the compiler knows there are exactly zero parameters, would be super noisy for users.
>>
>> Unless I read the RFC too quickly, it doesn't look like it's adding any value, since these aren't going to change meaning. Is it possible to revert this part of the change?
>
> `-Wstrict-prototypes` is now a pedantic deprecation warning that fires any time you form a function type which has no prototype, which was discussed in the RFC (https://discourse.llvm.org/t/rfc-enabling-wstrict-prototypes-by-default-in-c/60521/38?u=aaronballman):
>
>   Change -Wstrict-prototypes to diagnose functions without a prototype that don’t change behavior in C2x, it remains off-by-default but is automatically enabled by -pedantic as it’s still warning the user about a deprecation.
>
> However, I think the blocks behavior is a bug based on this: https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaType.cpp#L728 and I'd be more than happy to fix that, as I'm not checking that condition here: https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaType.cpp#L5579 which seems like a pretty obvious thing to be checking for before warning about not having a strict prototype.
>
> But I'm pretty insistent on warning about the other case as it does use functions without a prototype and we need *some* blanket warning for use of a deprecated feature for folks who want to be strictly conforming. It sounds like Apple may want to no longer enable `-Wstrict-prototypes` by default as it's shifted to be a more pedantic warning than it was before -- would that be a viable option for you (can you use project modernizations to turn it back off for old projects)?

Sure, I'm all for adding a new warning for users that want a pedantic warning. Can it be put behind a separate flag, such as `-Wstrict-prototypes-pedantic`, which isn't triggered by `-Wstrict-prototypes`?

Previously, `-Wstrict-prototypes` was useful for preventing actual bugs in code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122895