[clang] 385e7df - Correctly diagnose prototype redeclaration errors in C
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 13 05:22:08 PDT 2022
Author: Aaron Ballman
Date: 2022-04-13T08:21:31-04:00
New Revision: 385e7df33046d7292612ee1e3ac00a59d8bc0441
URL: https://github.com/llvm/llvm-project/commit/385e7df33046d7292612ee1e3ac00a59d8bc0441
DIFF: https://github.com/llvm/llvm-project/commit/385e7df33046d7292612ee1e3ac00a59d8bc0441.diff
LOG: Correctly diagnose prototype redeclaration errors in C
We did not implement C99 6.7.5.3p15 fully in that we missed the rule
for compatible function types where a prior declaration has a prototype
and a subsequent definition (not just declaration) has an empty
identifier list or an identifier list with a mismatch in parameter
arity. This addresses that situation by issuing an error on code like:
void f(int);
void f() {} // type conflicts with previous declaration
(Note: we already diagnose the other type conflict situations
appropriately, this was the only situation we hadn't covered that I
could find.)
Added:
clang/test/Sema/prototype-redecls.c
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/CodeGen/functions.c
clang/test/Sema/predefined-function.c
clang/test/Sema/warn-deprecated-non-prototype.c
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a1ff6f8787bb1..91fc57dfac595 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -144,6 +144,10 @@ Improvements to Clang's diagnostics
cases where the deprecated declarations or definitions of a function without
a prototype will change behavior in C2x. This diagnostic is grouped under the
``-Wstrict-prototypes`` warning group, but is enabled by default.
+- Clang now appropriately issues an error in C when a definition of a function
+ without a prototype and with no arguments is an invalid redeclaration of a
+ function with a prototype. e.g., ``void f(int); void f() {}`` is now properly
+ diagnosed.
Non-comprehensive list of changes in this release
-------------------------------------------------
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e368726d11c66..fae0ca1fa1e09 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2803,7 +2803,7 @@ class Sema final {
// Returns true if the function declaration is a redeclaration
bool CheckFunctionDeclaration(Scope *S,
FunctionDecl *NewFD, LookupResult &Previous,
- bool IsMemberSpecialization);
+ bool IsMemberSpecialization, bool DeclIsDefn);
bool shouldLinkDependentDeclWithPrevious(Decl *D, Decl *OldDecl);
bool canFullyTypeCheckRedeclaration(ValueDecl *NewD, ValueDecl *OldD,
QualType NewT, QualType OldT);
@@ -3495,7 +3495,7 @@ class Sema final {
void MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New,
LookupResult &OldDecls);
bool MergeFunctionDecl(FunctionDecl *New, NamedDecl *&Old, Scope *S,
- bool MergeTypeWithOld);
+ bool MergeTypeWithOld, bool NewDeclIsDefn);
bool MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old,
Scope *S, bool MergeTypeWithOld);
void mergeObjCMethodDecls(ObjCMethodDecl *New, ObjCMethodDecl *Old);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index cd9e6d56a5aee..706b3daf918dc 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3397,8 +3397,8 @@ static void adjustDeclContextForDeclaratorDecl(DeclaratorDecl *NewD,
/// merged with.
///
/// Returns true if there was an error, false otherwise.
-bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD,
- Scope *S, bool MergeTypeWithOld) {
+bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
+ bool MergeTypeWithOld, bool NewDeclIsDefn) {
// Verify the old decl was also a function.
FunctionDecl *Old = OldD->getAsFunction();
if (!Old) {
@@ -3880,6 +3880,25 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD,
// C: Function types need to be compatible, not identical. This handles
// duplicate function decls like "void f(int); void f(enum X);" properly.
if (!getLangOpts().CPlusPlus) {
+ // C99 6.7.5.3p15: ...If one type has a parameter type list and the other
+ // type is specified by a function definition that contains a (possibly
+ // empty) identifier list, both shall agree in the number of parameters
+ // and the type of each parameter shall be compatible with the type that
+ // results from the application of default argument promotions to the
+ // type of the corresponding identifier. ...
+ // This cannot be handled by ASTContext::typesAreCompatible() because that
+ // doesn't know whether the function type is for a definition or not when
+ // eventually calling ASTContext::mergeFunctionTypes(). The only situation
+ // we need to cover here is that the number of arguments agree as the
+ // default argument promotion rules were already checked by
+ // ASTContext::typesAreCompatible().
+ if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
+ Old->getNumParams() != New->getNumParams()) {
+ Diag(New->getLocation(), diag::err_conflicting_types) << New;
+ Diag(Old->getLocation(), PrevDiag) << Old << Old->getType();
+ return true;
+ }
+
// If we are merging two functions where only one of them has a prototype,
// we may have enough information to decide to issue a diagnostic that the
// function without a protoype will change behavior in C2x. This handles
@@ -9802,7 +9821,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
if (!NewFD->isInvalidDecl())
D.setRedeclaration(CheckFunctionDeclaration(S, NewFD, Previous,
- isMemberSpecialization));
+ isMemberSpecialization,
+ D.isFunctionDefinition()));
else if (!Previous.empty())
// Recover gracefully from an invalid redeclaration.
D.setRedeclaration(true);
@@ -9952,7 +9972,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
if (!NewFD->isInvalidDecl())
D.setRedeclaration(CheckFunctionDeclaration(S, NewFD, Previous,
- isMemberSpecialization));
+ isMemberSpecialization,
+ D.isFunctionDefinition()));
else if (!Previous.empty())
// Recover gracefully from an invalid redeclaration.
D.setRedeclaration(true);
@@ -11065,7 +11086,8 @@ static bool CheckMultiVersionFunction(Sema &S, FunctionDecl *NewFD,
/// \returns true if the function declaration is a redeclaration.
bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
LookupResult &Previous,
- bool IsMemberSpecialization) {
+ bool IsMemberSpecialization,
+ bool DeclIsDefn) {
assert(!NewFD->getReturnType()->isVariablyModifiedType() &&
"Variably modified return types are not handled here");
@@ -11183,7 +11205,8 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
if (Redeclaration) {
// NewFD and OldDecl represent declarations that need to be
// merged.
- if (MergeFunctionDecl(NewFD, OldDecl, S, MergeTypeWithPrevious)) {
+ if (MergeFunctionDecl(NewFD, OldDecl, S, MergeTypeWithPrevious,
+ DeclIsDefn)) {
NewFD->setInvalidDecl();
return Redeclaration;
}
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 4d3a4ba019adf..0ce9ddd36d68c 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -13351,7 +13351,8 @@ void Sema::CheckImplicitSpecialMemberDeclaration(Scope *S, FunctionDecl *FD) {
R.resolveKind();
R.suppressDiagnostics();
- CheckFunctionDeclaration(S, FD, R, /*IsMemberSpecialization*/false);
+ CheckFunctionDeclaration(S, FD, R, /*IsMemberSpecialization*/ false,
+ FD->isThisDeclarationADefinition());
}
void Sema::setupImplicitSpecialMemberType(CXXMethodDecl *SpecialMem,
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 925d6fa04c2c2..9d0dc8cad46b8 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2244,7 +2244,8 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(
}
SemaRef.CheckFunctionDeclaration(/*Scope*/ nullptr, Function, Previous,
- IsExplicitSpecialization);
+ IsExplicitSpecialization,
+ Function->isThisDeclarationADefinition());
// Check the template parameter list against the previous declaration. The
// goal here is to pick up default arguments added since the friend was
@@ -2605,7 +2606,8 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
}
SemaRef.CheckFunctionDeclaration(nullptr, Method, Previous,
- IsExplicitSpecialization);
+ IsExplicitSpecialization,
+ Method->isThisDeclarationADefinition());
if (D->isPure())
SemaRef.CheckPureMethod(Method, SourceRange());
diff --git a/clang/test/CodeGen/functions.c b/clang/test/CodeGen/functions.c
index fdfd732251946..e6bd5b49c514d 100644
--- a/clang/test/CodeGen/functions.c
+++ b/clang/test/CodeGen/functions.c
@@ -16,9 +16,6 @@ void test3(T f) {
f();
}
-int a(int);
-int a() {return 1;}
-
void f0(void) {}
// CHECK-LABEL: define{{.*}} void @f0()
diff --git a/clang/test/Sema/predefined-function.c b/clang/test/Sema/predefined-function.c
index 166c4661c9045..21d1439de99c6 100644
--- a/clang/test/Sema/predefined-function.c
+++ b/clang/test/Sema/predefined-function.c
@@ -19,13 +19,13 @@ int bar(int i) // expected-note {{previous definition is here}}
{
return 0;
}
-int bar() // expected-error {{redefinition of 'bar'}}
+int bar() // expected-error {{conflicting types for 'bar'}}
{
return 0;
}
-int foobar(int); // note {{previous declaration is here}}
-int foobar() // error {{conflicting types for 'foobar'}}
+int foobar(int); // expected-note {{previous declaration is here}}
+int foobar() // expected-error {{conflicting types for 'foobar'}}
{
return 0;
}
diff --git a/clang/test/Sema/prototype-redecls.c b/clang/test/Sema/prototype-redecls.c
new file mode 100644
index 0000000000000..9b85753cbbb5b
--- /dev/null
+++ b/clang/test/Sema/prototype-redecls.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -Wno-strict-prototypes -verify %s
+
+void blapp(int); // expected-note {{previous}}
+void blapp() { } // expected-error {{conflicting types for 'blapp'}}
+
+void yarp(int, ...); // expected-note {{previous}}
+void yarp(); // expected-error {{conflicting types for 'yarp'}}
+
+void blarg(int, ...); // expected-note {{previous}}
+void blarg() {} // expected-error {{conflicting types for 'blarg'}}
+
+void blerp(short); // expected-note {{previous}}
+void blerp(x) int x; {} // expected-error {{conflicting types for 'blerp'}}
+
+void glerp(int);
+void glerp(x) short x; {} // Okay, promoted type is fine
+
+// All these cases are okay
+void derp(int);
+void derp(x) int x; {}
+
+void garp(int);
+void garp();
+void garp(x) int x; {}
diff --git a/clang/test/Sema/warn-deprecated-non-prototype.c b/clang/test/Sema/warn-deprecated-non-prototype.c
index 57e03968e9fd7..33fe734e94197 100644
--- a/clang/test/Sema/warn-deprecated-non-prototype.c
+++ b/clang/test/Sema/warn-deprecated-non-prototype.c
@@ -66,8 +66,8 @@ void test(fmt) // both-warning {{a function declaration without a prototy
// comes from merging the function declarations together. The second is the
// point at which we know the behavior has changed (because we can see the
// previous declaration at that point), but we've already issued the type
-// warning by that point. It's not ideal to be this chatty, but this situation
+// error by that point. It's not ideal to be this chatty, but this situation
// should be pretty rare.
-void blapp(int);
-void blapp() { } // both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
+void blapp(int); // both-note {{previous declaration is here}}
+void blapp() { } // both-error {{conflicting types for 'blapp'}} \
// strict-warning {{a function declaration without a prototype is deprecated in all versions of C}}
More information about the cfe-commits
mailing list