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
