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 a function without a prototype}} +void f2(x) float x; { } // expected-warning{{promoted type 'double' of K&R function parameter is not compatible with the parameter type 'float' declared in a previous prototype}} \ ---------------- aaron.ballman wrote: > jyknight wrote: > > aaron.ballman wrote: > > > aaron.ballman wrote: > > > > jyknight wrote: > > > > > I think we don't want to emit a warning here. If a declaration with > > > > > params doesn't match a K&R definition, we already emit a > > > > > conflicting-types error. > > > > > > > > > > [[Well...except for one case, I've noticed -- we don't current emit > > > > > any warning when a definition with no parameters fails to match a > > > > > preceding declaration with params, e.g. `void f(int); void f() { }`. > > > > > I'm quite surprised -- I'm willing to say that such code is 100% just > > > > > a bug, not intentional. I note that GCC unconditionally rejects it. I > > > > > think we should also be emitting an unconditional error in that > > > > > case.]]] > > > > > > > > > > Anyhow, when you have a K&R style definition with parameters, that -- > > > > > all by itself -- is definitely invalid in C2x, so we don't need to > > > > > emit any warning on the declaration. > > > > > I'm quite surprised -- I'm willing to say that such code is 100% just > > > > > a bug, not intentional. I note that GCC unconditionally rejects it. I > > > > > think we should also be emitting an unconditional error in that case. > > > > > > > > I'd rather we be consistent here -- either every mixture of > > > > prototype/unprototyped is an error, or they're all a warning. I've > > > > added your example as a test case and we warn on it as being a change > > > > of behavior in C2x, which I think is defensible. > > > > > > > > > Anyhow, when you have a K&R style definition with parameters, that -- > > > > > all by itself -- is definitely invalid in C2x, so we don't need to > > > > > emit any warning on the declaration. > > > > > > > > I tend to agree, let me see what I can do. > > > I addressed this so we no longer diagnose the function with a prototype > > > in the case where it precedes the function without a prototype. > > > I'd rather we be consistent here -- either every mixture of > > > prototype/unprototyped is an error, or they're all a warning. I've added > > > your example as a test case and we warn on it as being a change of > > > behavior in C2x, which I think is defensible. > > > > Even before, we are NOT consistent. We emit an error on `void f(int); void > > f(x) float x; {}`, but not for `void f(int); void f() {}`. In both cases, > > we have a prototyped declaration, followed by an old-style "prototypeless" > > definition. I think it would be sensible to diagnose with an unconditional > > error in both cases, not only the former. > > Even before, we are NOT consistent. We emit an error on void f(int); void > > f(x) float x; {}, but not for void f(int); void f() {}. In both cases, we > > have a prototyped declaration, followed by an old-style "prototypeless" > > definition. I think it would be sensible to diagnose with an unconditional > > error in both cases, not only the former. > > I don't think `void f(int); void f() {}` should be diagnosed solely as an > issue with strict prototypes; I think it should be diagnosed the same as > `void f(int); void f(x) float x; {}`, which is *not* about strict prototypes > but *is* about the fact that the function types cannot merge because they > conflict. Once there's a declaration with a prototype, the function always > has a prototype (see C17 6.2.7p3 ) and redeclarations (including for > definition) need to have a compatible signature (see C17 6.7.6.3p15). So I'd > expect a `conflicting types` error diagnostic. I think it's defensible to > *additionally* issue the strict prototypes warning (though if we can silence > the warning because we're already issuing an error without introducing > significant extra complexity, that would be ideal). I'll look into adding the > error diagnostic. Sorry about being unclear -- what you propose is precisely what I meant to say. 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