aaron.ballman created this revision.
aaron.ballman added reviewers: jyknight, dexonsmith.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.
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;
}
This patch addresses the confusion in two ways. First, it updates the warning
text to be specific about a function *with* a prototype. But that pointed out
that we had a note which was perhaps not carrying its weight in terms of
conveying useful information to the user. In all of the situations we'd issue
the note, we already issue a warning with basically the same text as the note,
so the note comes across as overly chatty. The note has been removed and we're
leaning a bit more heavily on the prior diagnostic pointing out the previous
problematic declaration in that case.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D125814
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/test/Sema/warn-deprecated-non-prototype.c
clang/test/Sema/warn-strict-prototypes.c
Index: clang/test/Sema/warn-strict-prototypes.c
===================================================================
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -55,9 +55,8 @@
}
// Function declaration with no types
-void foo10(); // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}} \
- expected-note {{a function declaration without a prototype is not supported in C2x}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"void"
+void foo10(); // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:12}:"void"
// K&R function definition with incomplete param list declared
void foo10(p, p2) void *p; {} // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
expected-warning {{parameter 'p2' was not declared, defaults to 'int'; ISO C99 and later do not support implicit int}}
Index: clang/test/Sema/warn-deprecated-non-prototype.c
===================================================================
--- clang/test/Sema/warn-deprecated-non-prototype.c
+++ clang/test/Sema/warn-deprecated-non-prototype.c
@@ -24,8 +24,7 @@
// On by default warnings
void func(); // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
- strict-warning {{a function declaration without a prototype is deprecated in all versions of C}} \
- strict-note {{a function declaration without a prototype is not supported in C2x}}
+ strict-warning {{a function declaration without a prototype is deprecated in all versions of C}}
void func(a, b) int a, b; {} // both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
void one_more(a, b) int a, b; {} // both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
@@ -40,18 +39,16 @@
}
void order1(); // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
- strict-warning {{a function declaration without a prototype is deprecated in all versions of C}} \
- strict-note {{a function declaration without a prototype is not supported in C2x}}
-void order1(int i); // both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
+ strict-warning {{a function declaration without a prototype is deprecated in all versions of C}}
+void order1(int i); // both-warning {{this function declaration with a prototype will change behavior in C2x because of a previous declaration with no prototype}}
void order2(int i);
void order2(); // both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
strict-warning {{a function declaration without a prototype is deprecated in all versions of C}}
void order3(); // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
- strict-warning {{a function declaration without a prototype is deprecated in all versions of C}} \
- strict-note {{a function declaration without a prototype is not supported in C2x}}
-void order3(int i) {} // both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
+ strict-warning {{a function declaration without a prototype is deprecated in all versions of C}}
+void order3(int i) {} // both-warning {{this function definition with a prototype will change behavior in C2x because of a previous declaration with no prototype}}
// Just because the prototype is variadic doesn't mean we shouldn't warn on the
// K&R C function definition; this still changes behavior in C2x.
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -3941,11 +3941,8 @@
// 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 we didn't warn about it being deprecated (because the diagnostic
+ // is not enabled), warn now that it is deprecated and changes behavior.
if (Diags.isIgnored(diag::warn_strict_prototypes,
WithoutProto->getLocation())) {
if (WithoutProto->getBuiltinID() == 0 &&
@@ -3960,8 +3957,6 @@
}
Diag(WithoutProto->getLocation(), PD);
}
- } else {
- AddNote = true;
}
// Because the function with a prototype has parameters but a previous
@@ -3977,20 +3972,18 @@
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.
+ // attach a note to the one without a prototype if needed. If the
+ // declaration with the prototype is going to be a definition, warn
+ // about the definition changing behavior rather than the
+ // declaration.
+ bool WarnAboutDefn = false;
+ if (New == WithProto)
+ WarnAboutDefn = NewDeclIsDefn;
+
Diag(WithProto->getLocation(),
- diag::warn_non_prototype_changes_behavior);
- if (AddNote && WithoutProto->getBuiltinID() == 0)
- Diag(WithoutProto->getLocation(),
- diag::note_func_decl_changes_behavior);
+ diag::warn_prototype_changes_behavior)
+ << WarnAboutDefn;
}
- } 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.
- Diag(WithoutProto->getLocation(),
- diag::warn_non_prototype_changes_behavior);
}
}
}
@@ -15039,11 +15032,9 @@
// prototype, we don't want to diagnose it; if we have a possible
// prototype and it has no prototype, it may have already been
// diagnosed in SemaType.cpp as deprecated depending on whether
- // -Wstrict-prototypes is enabled. If we already warned about it 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;
+ // -Wstrict-prototypes is enabled. If we didn't warn about it being
+ // deprecated (because the diagnostic is not enabled), warn now that it
+ // is deprecated and changes behavior.
if (PossiblePrototype) {
if (Diags.isIgnored(diag::warn_strict_prototypes,
PossiblePrototype->getLocation())) {
@@ -15055,17 +15046,12 @@
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)
- Diag(PossiblePrototype->getLocation(),
- diag::note_func_decl_changes_behavior);
}
// Warn on CPUDispatch with an actual body.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5589,8 +5589,10 @@
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">;
+def warn_prototype_changes_behavior : Warning<
+ "this function %select{declaration|definition}0 with a prototype will change "
+ "behavior in C2x because of a previous declaration with no prototype">,
+ InGroup<DeprecatedNonPrototype>;
def warn_missing_variable_declarations : Warning<
"no previous extern declaration for non-static variable %0">,
InGroup<DiagGroup<"missing-variable-declarations">>, DefaultIgnore;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
