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}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
