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 *f;
----------------
aaron.ballman wrote:
> jyknight wrote:
> > This message seems vaguer than necessary, since we know _for sure_ this is
> > invalid.
> >
> > Can we say something like: "K&R-style function definitions are deprecated
> > in all versions of C and not supported in C2x"?
> I'd like to avoid using K&R in the diagnostic text (we're inconsistent about
> K&R vs prototype, etc already, but I'd like to see us excise `K&R` from
> diagnostics because it's not particularly descriptive to anyone younger than
> about 40. :-D
>
> How about: `function declarations without a prototype are deprecated in all
> versions of C and not supported in C2x` ?
I went with `a function declaration without a prototype is deprecated in all
versions of C and is not supported in C2x`, let me know if you think it needs
further adjusting. I also reworded the other diagnostics to sound similar.
================
Comment at: clang/test/Parser/declarators.c:5
-void f0();
+void f0(); /* expected-warning {{a function declaration without a prototype is
deprecated in all versions of C}} */
void f1(int [*]);
----------------
aaron.ballman wrote:
> jyknight wrote:
> > Perhaps we should add an explanation to the message like
> > `(specify '(void)' if you intended to accept no arguments)`
> > to make it clearer to folks who aren't familiar with this weirdness yet?
> They're already offered a fixit for this situation, so I don't know that we
> need the extra explanation? The user currently gets something like this:
> ```
> C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:16: warning:
> declaration of a function without a prototype is deprecated in all versions
> of C [-Wstrict-prototypes]
> void other_func();
> ^
> void
> ```
> Do you still think the diagnostic needs to be made longer?
I elected to leave this alone unless you feel strongly.
================
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:
> > 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.
================
Comment at: clang/test/Sema/warn-deprecated-non-prototype.c:46
+ strict-note {{this function declaration without a
prototype changes behavior in C2x}}
+void order1(int i); // both-warning {{this function declaration with a
prototype changes behavior in C2x because it is preceded by a function without
a prototype}}
+
----------------
aaron.ballman wrote:
> jyknight wrote:
> > Maybe something like "this function declaration will conflict with the
> > preceding declaration in C2x, because the preceding declaration does not
> > specify arguments."
> I can see the appeal, but that makes the diagnostic rather long and I'd still
> like to be consistent in the terminology we use. Given that the old
> diagnostic was `-Wstrict-prototypes` and the new one is
> `-Wdeprecated-non-prototype`, I think we don't need to tie ourselves into
> knots to avoid using the terminology. Do you feel strongly about rewording
> this?
I've elected to leave this one alone unless you feel strongly.
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
