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

dexonsmith added a comment. In D122895#3511855 , @aaron.ballman wrote: >> (It also seems unfortunate to regress the false positive rate of this >> diagnostic before `-fstrict-prototypes` is available.) > > `-fno-knr-functions` is available already

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

aaron.ballman added a comment. In D122895#3512314 , @aaron.ballman wrote: > In D122895#3511682 , @aaron.ballman > wrote: > >> In D122895#3511632 , @jyknight >> wrote:

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

aaron.ballman added a comment. In D122895#3512341 , @aaron.ballman wrote: > I think the current behavior today makes sense but we should see if we can > improve it to make *more* sense. With `-Wstrict-prototypes`, we should > complain about the block

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

aaron.ballman added a comment. In D122895#3511855 , @aaron.ballman wrote: > In D122895#3511646 , @dexonsmith > wrote: > >> void f1(void (^block)()); >> >> void f2(void) { >> f1(^(int x) { /* do

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

aaron.ballman added a comment. In D122895#3511682 , @aaron.ballman wrote: > In D122895#3511632 , @jyknight > wrote: > >> The warnings for this case aren't great: >> >> int foo(); >> >> int >> foo(int

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

aaron.ballman added a comment. In D122895#3511646 , @dexonsmith wrote: > In D122895#3511611 , @dexonsmith > wrote: > >> Previously, `-Wstrict-prototypes` was useful for preventing actual bugs in >> code. > >

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

dexonsmith added a comment. In D122895#3511649 , @aaron.ballman wrote: > In D122895#3511611 , @dexonsmith > wrote: > >> Sure, I'm all for adding a new warning for users that want a pedantic >> warning. Can it

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

aaron.ballman added a comment. In 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

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

aaron.ballman added a comment. In D122895#3511611 , @dexonsmith wrote: > 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

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

dexonsmith added a comment. In D122895#3511611 , @dexonsmith wrote: > Previously, `-Wstrict-prototypes` was useful for preventing actual bugs in > code. For example, it's important to have a warning that catches code like this: void f1(void

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

jyknight added a comment. The warnings for this case aren't great: int foo(); int foo(int arg) { return 5; } results in: /tmp/test.c:1:5: warning: a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x

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

dexonsmith added a subscriber: arphaman. dexonsmith added a comment. In D122895#3511376 , @aaron.ballman wrote: > In D122895#3511312 , @aaron.ballman > wrote: > >> However, I think the blocks behavior is a bug

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

aaron.ballman added a comment. In 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

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

aaron.ballman added a comment. In 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

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

dexonsmith added inline comments. Comment at: clang/test/Sema/warn-strict-prototypes.m:20 + // know it's a block when diagnosing. + void (^block2)(void) = ^void() { // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}} };

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

efriedma added inline comments. Comment at: clang/test/Sema/warn-strict-prototypes.m:20 + // know it's a block when diagnosing. + void (^block2)(void) = ^void() { // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}} };

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

dexonsmith added a comment. 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. Warning on block

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

dexonsmith added inline comments. Comment at: clang/test/Sema/warn-strict-prototypes.m:20 + // know it's a block when diagnosing. + void (^block2)(void) = ^void() { // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}} };

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

dexonsmith added subscribers: steven_wu, rjmccall, rnk, dexonsmith. dexonsmith added inline comments. Comment at: clang/test/Sema/warn-strict-prototypes.m:20 + // know it's a block when diagnosing. + void (^block2)(void) = ^void() { // expected-warning {{a function

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

aaron.ballman added a comment. In D122895#3484097 , @aaron.ballman wrote: > In D122895#3484076 , @manojgupta > wrote: > >> Tried locally but I still see the warning with -fno-knr-functions. It also >> says

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

aaron.ballman added a comment. In D122895#3484076 , @manojgupta wrote: > Tried locally but I still see the warning with -fno-knr-functions. It also > says that the argument is unused. > > bin/clang --version > clang version 15.0.0

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

manojgupta added a comment. Following behavior is also surprising: Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/ https://reviews.llvm.org/D122895 ___ cfe-commits mailing list

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

manojgupta added a comment. Tried locally but I still see the warning with -fno-knr-functions. It also says that the argument is unused. bin/clang --version clang version 15.0.0 (https://github.com/llvm/llvm-project.git a9d68a5524dea113cace5983697786599cbdce9a

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

aaron.ballman added a comment. In D122895#3483175 , @manojgupta wrote: > Unless I probably mis-interpreted something, -fno-knr-functions does not > suppress the warning: https://godbolt.org/z/rbEfbbb33 Godbolt hasn't had the chance to catch up to

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

aaron.ballman added a comment. In D122895#3483163 , @manojgupta wrote: > Basically, I'm wondering if you'd be able to enable -fno-knr-function? > > Thanks. this looks promising. Any ideas when -fno-knr-function support was > added? Oops, I had a

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

manojgupta added a comment. Unless I probably mis-interpreted something, -fno-knr-functions does not suppress the warning: https://godbolt.org/z/rbEfbbb33 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/

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

manojgupta added a comment. Basically, I'm wondering if you'd be able to enable -fno-knr-function? Thanks. this looks promising. Any ideas when -fno-knr-function support was added? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/

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

aaron.ballman added a comment. In D122895#3482575 , @aaron.ballman wrote: > In D122895#3482555 , @tahonermann > wrote: > >>> I think it's debatable whether this is a bug or not >> >> For C99 through C17, I kind

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

aaron.ballman added a comment. In D122895#3482707 , @manojgupta wrote: > Is disabling the pedantic warning an option for your users? > > Disabling it wholesale is not an option since they actually want this warning > (the older version). But we

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

manojgupta added a comment. Is disabling the pedantic warning an option for your users? Disabling it wholesale is not an option since they actually want this warning (the older version). But we agreed to disable it specifically for the code where the warning was getting fired. One instance

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

aaron.ballman added a comment. In D122895#3482555 , @tahonermann wrote: >> I think it's debatable whether this is a bug or not > > For C99 through C17, I kind of agree, but for C2x (where the warning is still > issued with `-Wstrict-prototypes`), my

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

tahonermann added a comment. > I think it's debatable whether this is a bug or not For C99 through C17, I kind of agree, but for C2x (where the warning is still issued with `-Wstrict-prototypes`), my understanding is that `void foo(void)` and `void foo()` are equivalent; there is no

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

aaron.ballman added a comment. In 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

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

manojgupta added a comment. 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:

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

aaron.ballman added inline comments. Comment at: clang/test/CodeGen/2009-06-01-addrofknr.c:8 -static int func(f) +static int func(f) // expected-warning {{this function declaration without a prototype is deprecated in all versions of C and changes behavior in C2x}} void

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

aaron.ballman added inline comments. Comment at: clang/test/Sema/knr-def-call.c:15 +void f2(float); // expected-note{{previous declaration is here}} \ + expected-warning {{this function declaration with a prototype changes behavior in C2x because it is

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

jyknight added inline comments. Comment at: clang/test/Sema/knr-def-call.c:15 +void f2(float); // expected-note{{previous declaration is here}} \ + expected-warning {{this function declaration with a prototype changes behavior in C2x because it is followed by

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

aaron.ballman added inline comments. Comment at: clang/test/CodeGen/2009-06-01-addrofknr.c:8 -static int func(f) +static int func(f) // expected-warning {{this function declaration without a prototype is deprecated in all versions of C and changes behavior in C2x}} void

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

jyknight added inline comments. Comment at: clang/test/CodeGen/2009-06-01-addrofknr.c:8 -static int func(f) +static int func(f) // expected-warning {{this function declaration without a prototype is deprecated in all versions of C and changes behavior in C2x}} void *f;

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG11da1b53d8cd: [C89/C2x] Improve diagnostics around strict prototypes in C (authored by aaron.ballman). Changed prior to commit: https://reviews.llvm.org/D122895?vs=419853=421627#toc Repository: rG

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

aaron.ballman added a comment. Thanks @erichkeane! @jyknight, @hubert.reinterpretcast, @eli.friedman -- I'll likely land this sometime tomorrow unless I hear from you. But if I land it and you still have comments, I'll be happy to address them post commit. CHANGES SINCE LAST ACTION

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

aaron.ballman added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/ https://reviews.llvm.org/D122895 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

aaron.ballman updated this revision to Diff 419853. aaron.ballman added a comment. Fix failing test case caught by precommit CI. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/ https://reviews.llvm.org/D122895 Files: clang/docs/ReleaseNotes.rst

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

aaron.ballman added inline comments. Comment at: clang/test/CodeGen/2009-06-01-addrofknr.c:8 -static int func(f) +static int func(f) // expected-warning {{this function declaration without a prototype is deprecated in all versions of C and changes behavior in C2x}} void

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

aaron.ballman updated this revision to Diff 419844. aaron.ballman marked 5 inline comments as done. aaron.ballman added a comment. Updating based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/ https://reviews.llvm.org/D122895 Files:

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

aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: clang/test/CodeGen/2009-06-01-addrofknr.c:8 -static int func(f) +static int func(f) // expected-warning {{this function declaration without a prototype is deprecated in all

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

jyknight added a comment. Starting by looking at the test cases, I've got some suggestions on making the diagnostics a bit less confusing. Comment at: clang/test/CodeGen/2009-06-01-addrofknr.c:8 -static int func(f) +static int func(f) // expected-warning {{this function

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

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5566-5567 +def warn_non_prototype_changes_behavior : Warning< + "this function declaration without a prototype is deprecated in all

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

cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5566-5567 +def warn_non_prototype_changes_behavior : Warning< + "this function declaration without a prototype is deprecated in all versions " + "of C and changes behavior in C2x">,

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

aaron.ballman updated this revision to Diff 419734. aaron.ballman added a comment. Rebased, no changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122895/new/ https://reviews.llvm.org/D122895 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticGroups.td

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

aaron.ballman created this revision. aaron.ballman added reviewers: jyknight, eli.friedman, hubert.reinterpretcast, erichkeane, clang-language-wg. Herald added a subscriber: jdoerfert. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang.