[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. Just FYI, I had to fix some tests after this in rG705306526b5ca7eed2fa28ebf832873cbb5085ec . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

This revision was automatically updated to reflect the committed changes. aaronpuchert marked an inline comment as done. Closed by commit rG2f26bc554270: Warn about zero-parameter KR definitions in -Wstrict-prototypes (authored by aaronpuchert). Changed prior to commit:

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert marked 5 inline comments as done. aaronpuchert added a comment. Thanks! Comment at: clang/test/Sema/warn-strict-prototypes.c:11 +// function definition with 0 params, no prototype. +void foo1() {} // expected-warning {{this old-style function definition is not

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaron.ballman added inline comments. Comment at: clang/test/Sema/warn-strict-prototypes.c:11 +// function definition with 0 params, no prototype. +void foo1() {} // expected-warning {{this old-style function definition is not preceded by a prototype}} +// function definition

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added inline comments. Comment at: clang/test/Sema/warn-strict-prototypes.c:11 +// function definition with 0 params, no prototype. +void foo1() {} // expected-warning {{this old-style function definition is not preceded by a prototype}} +// function definition

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added inline comments. Comment at: clang/test/Sema/warn-strict-prototypes.c:11 +// function definition with 0 params, no prototype. +void foo1() {} // expected-warning {{this old-style function definition is not preceded by a prototype}} +// function definition

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thanks for your patience while I thought about this more. I think the direction makes sense, so this LGTM with some extra test cases. Comment at:

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. Ping @aaron.ballman. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66919/new/ https://reviews.llvm.org/D66919 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. @aaron.ballman We could make the warning even stricter, but that should be a separate discussion. Is this change Ok for now? Comment at: clang/test/Sema/warn-strict-prototypes.c:60-62 // K function definition with previous prototype declared is

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaron.ballman added a comment. In D66919#1658483 , @aaronpuchert wrote: > In D66919#1658355 , @aaron.ballman > wrote: > > > We do have numerous warnings that are default errors, you can look for > >

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. In D66919#1658355 , @aaron.ballman wrote: > We do have numerous warnings that are default errors, you can look for > `DefaultError` in the diagnostic .td files to see the uses. Thanks, I hadn't seen that before. It seems

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaron.ballman added a comment. In D66919#1655052 , @aaronpuchert wrote: > In D66919#1655048 , @dexonsmith > wrote: > > > I went back to read notes from when we deployed `-Wstrict-prototypes` > > (which we have

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. In D66919#1655048 , @dexonsmith wrote: > I went back to read notes from when we deployed `-Wstrict-prototypes` (which > we have had on-by-default for our users for a couple of years, since it > catches lots of

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

dexonsmith added a comment. I went back to read notes from when we deployed `-Wstrict-prototypes` (which we have had on-by-default for our users for a couple of years, since it catches lots of `-fblocks`-related bugs). I was mainly objecting because I had (mis)remembered trying and backing

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. > IIRC, when we rolled out `-Wstrict-prototypes` we explicitly excluded this > case since it hit a lot of code without finding bugs. I'm not a historian, but I believe this was suggested by @rsmith in https://bugs.llvm.org/show_bug.cgi?id=20796#c8, and I think we

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. We had a discussion on IRC yesterday and @aaron.ballman pointed out that the K syntax has been considered "obsolescent" (= deprecated) since C89, 30 years ago. He added that there are thoughts within the standards committee about removing the syntax entirely from

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

dexonsmith added a comment. > In D66919#1650174 , @dexonsmith > wrote: > >> We could carve out a `-W` flag (if it doesn't already exist) that warns if >> you incorrectly pass parameters to a function whose definition has no >> prototype > > > The

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. By the way, I'm open to adding a fix-it hint for zero-parameter K definitions, since the fix is pretty straightforward. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66919/new/ https://reviews.llvm.org/D66919

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. In D66919#1651108 , @dexonsmith wrote: > we just don't warn on non-prototype defining declarations, where the meaning > is unambiguous: > > void foo() {} > "Meaning" is a difficult term. What we know is that this is a K

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaron.ballman added a comment. In D66919#1651108 , @dexonsmith wrote: > In D66919#1650775 , @aaron.ballman > wrote: > > > In D66919#1650174 , @dexonsmith > > wrote: > > >

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

dexonsmith added a comment. In D66919#1650775 , @aaron.ballman wrote: > In D66919#1650174 , @dexonsmith > wrote: > > > This could cause a lot of churn in existing projects (especially with > > `static void

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaron.ballman added a comment. In D66919#1650174 , @dexonsmith wrote: > This could cause a lot of churn in existing projects (especially with `static > void foo()`), without giving Clang any new information. I'm wary of this. Those projects likely

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert added a comment. Note that I'm just copying GCC, which seems the be the original intent behind the warning (https://bugs.llvm.org/show_bug.cgi?id=20796). So people who use both compilers will have seen that warning already. Note also that there is no warning if any declaration

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

dexonsmith added a reviewer: erik.pilkington. dexonsmith added a comment. This could cause a lot of churn in existing projects (especially with `static void foo()`), without giving Clang any new information. I'm wary of this. > Zero-parameter K definitions specify that the function has no >

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

aaronpuchert created this revision. aaronpuchert added reviewers: arphaman, bruno, rsmith. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. Zero-parameter K definitions specify that the function has no parameters, but they are still not prototypes, so calling the