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 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}} \
----------------
jyknight wrote:
> 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.
Thank you for confirming (and poking me about this). As always, I really
appreciate your input!
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
