aaron.ballman created this revision.
aaron.ballman added reviewers: jyknight, eli.friedman, clang-language-wg.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.
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.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D123627
Files:
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/prototype-redecls.c
clang/test/Sema/warn-deprecated-non-prototype.c
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
@@ -68,6 +68,6 @@
// 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
// 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}}
Index: clang/test/Sema/prototype-redecls.c
===================================================================
--- /dev/null
+++ 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; {}
Index: clang/test/Sema/predefined-function.c
===================================================================
--- clang/test/Sema/predefined-function.c
+++ clang/test/Sema/predefined-function.c
@@ -19,13 +19,13 @@
{
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;
}
Index: clang/test/CodeGen/functions.c
===================================================================
--- clang/test/CodeGen/functions.c
+++ clang/test/CodeGen/functions.c
@@ -16,9 +16,6 @@
f();
}
-int a(int);
-int a() {return 1;}
-
void f0(void) {}
// CHECK-LABEL: define{{.*}} void @f0()
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2244,7 +2244,8 @@
}
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 @@
}
SemaRef.CheckFunctionDeclaration(nullptr, Method, Previous,
- IsExplicitSpecialization);
+ IsExplicitSpecialization,
+ Method->isThisDeclarationADefinition());
if (D->isPure())
SemaRef.CheckPureMethod(Method, SourceRange());
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13351,7 +13351,8 @@
R.resolveKind();
R.suppressDiagnostics();
- CheckFunctionDeclaration(S, FD, R, /*IsMemberSpecialization*/false);
+ CheckFunctionDeclaration(S, FD, R, /*IsMemberSpecialization*/ false,
+ FD->isThisDeclarationADefinition());
}
void Sema::setupImplicitSpecialMemberType(CXXMethodDecl *SpecialMem,
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -3397,8 +3397,8 @@
/// 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 @@
// 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 @@
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 @@
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 @@
/// \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 @@
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;
}
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2803,7 +2803,7 @@
// 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 @@
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);
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -144,6 +144,10 @@
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
-------------------------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
