Skip to content

Commit

Permalink
Improve the strict prototype diagnostic behavior
Browse files Browse the repository at this point in the history 
Post-commit feedback on https://reviews.llvm.org/D122895 pointed out
that the diagnostic wording for some code was using "declaration" in a
confusing way, such as:

int foo(); // warning: a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x

int foo(int arg) { // warning: a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x
  return 5;
}

And that we had other minor issues with the diagnostics being somewhat
confusing.

This patch addresses the confusion by reworking the implementation to
be a bit more simple and a bit less chatty. Specifically, it changes
the warning and note diagnostics to be able to specify "declaration" or
"definition" as appropriate, and it changes the function merging logic
so that the function without a prototype is always what gets warned on,
and the function with a prototype is sometimes what gets noted.
Additionally, when diagnosing a K&R C definition that is preceded by a
function without a prototype, we don't note the prior declaration, we
warn on it because it will also be changing behavior in C2x.

Differential Revision: https://reviews.llvm.org/D125814
  • Loading branch information
@AaronBallman
AaronBallman committed May 26, 2022
1 parent cd5783d commit 681c50c
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 123 deletions.
10 changes: 6 additions & 4 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Expand Up @@ -5589,10 +5589,12 @@ def warn_strict_prototypes : Extension<
"a %select{function|block}0 declaration without a prototype is deprecated "
"%select{in all versions of C|}0">, InGroup<StrictPrototypes>;
def warn_non_prototype_changes_behavior : Warning<
"a function declaration without a prototype is deprecated in all versions of "
"C and is not supported in C2x">, InGroup<DeprecatedNonPrototype>;
def note_func_decl_changes_behavior : Note<
"a function declaration without a prototype is not supported in C2x">;
"a function %select{declaration|definition}0 without a prototype is "
"deprecated in all versions of C %select{and is not supported in C2x|and is "
"treated as a zero-parameter prototype in C2x, conflicting with a "
"%select{previous|subsequent}2 %select{declaration|definition}3}1">,
InGroup<DeprecatedNonPrototype>;
def note_conflicting_prototype : Note<"conflicting prototype is here">;
def warn_missing_variable_declarations : Warning<
"no previous extern declaration for non-static variable %0">,
InGroup<DiagGroup<"missing-variable-declarations">>, DefaultIgnore;
Expand Down
109 changes: 36 additions & 73 deletions clang/lib/Sema/SemaDecl.cpp
View file
Expand Up @@ -3940,59 +3940,29 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
}

if (WithProto->getNumParams() != 0) {
// The function definition has parameters, so this will change
// behavior in C2x.
//
// If we already warned about about the function without a prototype
// being deprecated, add a note that it also changes behavior. If we
// didn't warn about it being deprecated (because the diagnostic is
// not enabled), warn now that it is deprecated and changes behavior.
bool AddNote = false;
if (Diags.isIgnored(diag::warn_strict_prototypes,
WithoutProto->getLocation())) {
if (WithoutProto->getBuiltinID() == 0 &&
!WithoutProto->isImplicit() &&
SourceMgr.isBeforeInTranslationUnit(WithoutProto->getLocation(),
WithProto->getLocation())) {
PartialDiagnostic PD =
PDiag(diag::warn_non_prototype_changes_behavior);
if (TypeSourceInfo *TSI = WithoutProto->getTypeSourceInfo()) {
if (auto FTL = TSI->getTypeLoc().getAs<FunctionNoProtoTypeLoc>())
PD << FixItHint::CreateInsertion(FTL.getRParenLoc(), "void");
}
Diag(WithoutProto->getLocation(), PD);
}
} else {
AddNote = true;
}

// Because the function with a prototype has parameters but a previous
// declaration had none, the function with the prototype will also
// change behavior in C2x.
if (WithProto->getBuiltinID() == 0 && !WithProto->isImplicit()) {
if (SourceMgr.isBeforeInTranslationUnit(
WithProto->getLocation(), WithoutProto->getLocation())) {
// If the function with the prototype comes before the function
// without the prototype, we only want to diagnose the one without
// the prototype.
Diag(WithoutProto->getLocation(),
diag::warn_non_prototype_changes_behavior);
} else {
// Otherwise, diagnose the one with the prototype, and potentially
// attach a note to the one without a prototype if needed.
Diag(WithProto->getLocation(),
diag::warn_non_prototype_changes_behavior);
if (AddNote && WithoutProto->getBuiltinID() == 0)
Diag(WithoutProto->getLocation(),
diag::note_func_decl_changes_behavior);
}
} else if (AddNote && WithoutProto->getBuiltinID() == 0 &&
!WithoutProto->isImplicit()) {
// If we were supposed to add a note but the function with a
// prototype is a builtin or was implicitly declared, which means we
// have nothing to attach the note to, so we issue a warning instead.
if (WithoutProto->getBuiltinID() == 0 && !WithoutProto->isImplicit()) {
// The one without the prototype will be changing behavior in C2x, so
// warn about that one so long as it's a user-visible declaration.
bool IsWithoutProtoADef = false, IsWithProtoADef = false;
if (WithoutProto == New)
IsWithoutProtoADef = NewDeclIsDefn;
else
IsWithProtoADef = NewDeclIsDefn;
Diag(WithoutProto->getLocation(),
diag::warn_non_prototype_changes_behavior);
diag::warn_non_prototype_changes_behavior)
<< IsWithoutProtoADef << (WithoutProto->getNumParams() ? 0 : 1)
<< (WithoutProto == Old) << IsWithProtoADef;

// The reason the one without the prototype will be changing behavior
// is because of the one with the prototype, so note that so long as
// it's a user-visible declaration. There is one exception to this:
// when the new declaration is a definition without a prototype, the
// old declaration with a prototype is not the cause of the issue,
// and that does not need to be noted because the one with a
// prototype will not change behavior in C2x.
if (WithProto->getBuiltinID() == 0 && !WithProto->isImplicit() &&
!IsWithoutProtoADef)
Diag(WithProto->getLocation(), diag::note_conflicting_prototype);
}
}
}
Expand Down Expand Up @@ -15053,29 +15023,22 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
// deprecated, add a note that it also changes behavior. If we didn't
// warn about it being deprecated (because the diagnostic is not
// enabled), warn now that it is deprecated and changes behavior.
bool AddNote = false;
if (PossiblePrototype) {
if (Diags.isIgnored(diag::warn_strict_prototypes,
PossiblePrototype->getLocation())) {

PartialDiagnostic PD =
PDiag(diag::warn_non_prototype_changes_behavior);
if (TypeSourceInfo *TSI = PossiblePrototype->getTypeSourceInfo()) {
if (auto FTL = TSI->getTypeLoc().getAs<FunctionNoProtoTypeLoc>())
PD << FixItHint::CreateInsertion(FTL.getRParenLoc(), "void");
}
Diag(PossiblePrototype->getLocation(), PD);
} else {
AddNote = true;
}
}

// Because this function definition has no prototype and it has
// parameters, it will definitely change behavior in C2x.
Diag(FD->getLocation(), diag::warn_non_prototype_changes_behavior);
if (AddNote)
// This K&R C function definition definitely changes behavior in C2x,
// so diagnose it.
Diag(FD->getLocation(), diag::warn_non_prototype_changes_behavior)
<< /*definition*/ 1 << /* not supported in C2x */ 0;

// If we have a possible prototype for the function which is a user-
// visible declaration, we already tested that it has no prototype.
// This will change behavior in C2x. This gets a warning rather than a
// note because it's the same behavior-changing problem as with the
// definition.
if (PossiblePrototype)
Diag(PossiblePrototype->getLocation(),
diag::note_func_decl_changes_behavior);
diag::warn_non_prototype_changes_behavior)
<< /*declaration*/ 0 << /* conflicting */ 1 << /*subsequent*/ 1
<< /*definition*/ 1;
}

// Warn on CPUDispatch with an actual body.
Expand Down
6 changes: 3 additions & 3 deletions clang/test/C/drs/dr0xx.c
View file
Expand Up @@ -231,13 +231,13 @@ int dr032 = (1, 2); /* expected-warning {{left operand of comma operator has no
/* WG14 DR035: partial
* Questions about definition of functions without a prototype
*/
void dr035_1(a, b) /* expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} */
void dr035_1(a, b) /* expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}} */
int a(enum b {x, y}); /* expected-warning {{declaration of 'enum b' will not be visible outside of this function}} */
int b; {
int test = x; /* expected-error {{use of undeclared identifier 'x'}} */
}

void dr035_2(c) /* expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} */
void dr035_2(c) /* expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}} */
enum m{q, r} c; { /* expected-warning {{declaration of 'enum m' will not be visible outside of this function}} */
/* FIXME: This should be accepted because the scope of m, q, and r ends at
* the closing brace of the function per C89 6.1.2.1.
Expand Down Expand Up @@ -391,7 +391,7 @@ void dr068(void) {
* a prototype causes implicit conversions rather than relying on default
* argument promotion and warm thoughts.
*/
void dr070_1(c) /* expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} */
void dr070_1(c) /* expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}} */
int c; {
}

Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGen/2009-06-01-addrofknr.c
View file
Expand Up @@ -5,7 +5,7 @@ struct funcptr {
int (*func)();
};

static int func(f) // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
static int func(f) // expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
void *f;
{
return 0;
Expand Down
10 changes: 5 additions & 5 deletions clang/test/Parser/declarators.c
View file
Expand Up @@ -27,12 +27,12 @@ void test2(int *P, int A) {
}

typedef int atype;
void test3(x, /* expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} */
void test3(x, /* expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}} */
atype /* expected-error {{unexpected type name 'atype': expected identifier}} */
) int x, atype; {}

void test4(x, x) int x; {} // expected-error {{redefinition of parameter 'x'}} \
// expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
// expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}


// PR3031
Expand Down Expand Up @@ -103,11 +103,11 @@ void *test14b = (void*)test14a; // Make sure test14a didn't get skipped.
long struct X { int x; } test15(void); // expected-error {{'long struct' is invalid}}

void test16(i) int i j; { } // expected-error {{expected ';' at end of declaration}} \
// expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
// expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
void test17(i, j) int i, j k; { } // expected-error {{expected ';' at end of declaration}} \
// expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
// expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
void knrNoSemi(i) int i { } // expected-error {{expected ';' at end of declaration}} \
// expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
// expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}

// PR12595
void test18(void) {
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Sema/arg-duplicate.c
View file
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c99

int f3(y, x, // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
int f3(y, x, // expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
x) // expected-error {{redefinition of parameter}}
int y,
x, // expected-note {{previous declaration is here}}
Expand Down
10 changes: 5 additions & 5 deletions clang/test/Sema/knr-def-call.c
View file
@@ -1,19 +1,19 @@
// RUN: %clang_cc1 -triple i386-pc-unknown -Wconversion -Wliteral-conversion -fsyntax-only -verify %s

// C DR #316, PR 3626.
void f0(a, b, c, d) int a,b,c,d; {} // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
void f0(a, b, c, d) int a,b,c,d; {} // expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
void t0(void) {
f0(1); // expected-warning{{too few arguments}}
}

void f1(a, b) int a, b; {} // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
void f1(a, b) int a, b; {} // expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
void t1(void) {
f1(1, 2, 3); // expected-warning{{too many arguments}}
}

void f2(float); // expected-note{{previous declaration is here}}
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}} \
expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}

typedef void (*f3)(void);
f3 t3(int b) { return b? f0 : f1; } // okay
Expand All @@ -33,7 +33,7 @@ char *rindex(s, c)

// PR8314
void proto(int);
void proto(x) // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
void proto(x) // expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
int x;
{
}
Expand All @@ -45,6 +45,6 @@ void use_proto() {

// PR31020
void func(short d) __attribute__((cdecl)); // expected-note{{previous declaration is here}}
void __attribute__((cdecl)) func(d) // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
void __attribute__((cdecl)) func(d) // expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
short d; // expected-warning{{promoted type 'int' of K&R function parameter is not compatible with the parameter type 'short' declared in a previous prototype}}
{}
4 changes: 2 additions & 2 deletions clang/test/Sema/knr-variadic-def.c
View file
Expand Up @@ -5,7 +5,7 @@
char *foo = "test";
int test(char*,...);

int test(fmt) // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
int test(fmt) // expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
char*fmt;
{
va_list ap;
Expand All @@ -21,7 +21,7 @@ int test(fmt) // expected-warning {{a function declaration without a prototype i

void exit(); // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}}

int main(argc,argv) // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
int main(argc,argv) // expected-warning {{a function definition without a prototype is deprecated in all versions of C and is not supported in C2x}}
int argc;char**argv;
{
exit(test("",foo));
Expand Down

0 comments on commit 681c50c

Please sign in to comment.