From 275a369f003f25bd22c00c1c0fc0251c7208caf4 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Tue, 10 Mar 2009 23:43:53 +0000 Subject: [PATCH] Add type checking for tentative definitions at the end of the translation unit. Thread the various declarations of variables via VarDecl::getPreviousDeclaration. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@66601 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Decl.h | 26 +++++++- include/clang/Basic/DiagnosticSemaKinds.def | 5 ++ lib/AST/Decl.cpp | 16 +++++ lib/Sema/Sema.cpp | 45 ++++++++++++++ lib/Sema/Sema.h | 1 - lib/Sema/SemaDecl.cpp | 67 +++++---------------- test/Sema/enum.c | 4 +- test/Sema/incomplete-decl.c | 4 +- test/Sema/init.c | 3 +- test/Sema/tentative-decls.c | 9 ++- 10 files changed, 117 insertions(+), 63 deletions(-) diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index b7d9fd0263..495471ef51 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -233,6 +233,9 @@ private: /// condition, e.g., if (int x = foo()) { ... }. bool DeclaredInCondition : 1; + /// \brief The previous declaration of this variable. + VarDecl *PreviousDeclaration; + // Move to DeclGroup when it is implemented. SourceLocation TypeSpecStartLoc; friend class StmtIteratorBase; @@ -240,8 +243,9 @@ protected: VarDecl(Kind DK, DeclContext *DC, SourceLocation L, IdentifierInfo *Id, QualType T, StorageClass SC, SourceLocation TSSL = SourceLocation()) : ValueDecl(DK, DC, L, Id, T), Init(0), - ThreadSpecified(false), HasCXXDirectInit(false), - DeclaredInCondition(false), TypeSpecStartLoc(TSSL) { + ThreadSpecified(false), HasCXXDirectInit(false), + DeclaredInCondition(false), PreviousDeclaration(0), + TypeSpecStartLoc(TSSL) { SClass = SC; } public: @@ -261,6 +265,12 @@ public: Expr *getInit() { return (Expr*) Init; } void setInit(Expr *I) { Init = (Stmt*) I; } + /// \brief Retrieve the definition of this variable, which may come + /// from a previous declaration. Def will be set to the VarDecl that + /// contains the initializer, and the result will be that + /// initializer. + const Expr *getDefinition(const VarDecl *&Def); + void setThreadSpecified(bool T) { ThreadSpecified = T; } bool isThreadSpecified() const { return ThreadSpecified; @@ -290,6 +300,14 @@ public: DeclaredInCondition = InCondition; } + /// getPreviousDeclaration - Return the previous declaration of this + /// variable. + const VarDecl *getPreviousDeclaration() const { return PreviousDeclaration; } + + void setPreviousDeclaration(VarDecl * PrevDecl) { + PreviousDeclaration = PrevDecl; + } + /// hasLocalStorage - Returns true if a variable with function scope /// is a non-static local variable. bool hasLocalStorage() const { @@ -337,6 +355,10 @@ public: } return false; } + + /// \brief Determine whether this is a tentative definition of a + /// variable in C. + bool isTentativeDefinition(ASTContext &Context) const; /// \brief Determines whether this variable is a variable with /// external, C linkage. diff --git a/include/clang/Basic/DiagnosticSemaKinds.def b/include/clang/Basic/DiagnosticSemaKinds.def index 271e14d8a1..74e5b58e96 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.def +++ b/include/clang/Basic/DiagnosticSemaKinds.def @@ -891,6 +891,11 @@ DIAG(err_typecheck_pointer_arith_void_type, ERROR, "arithmetic on pointer to void type") DIAG(err_typecheck_decl_incomplete_type, ERROR, "variable has incomplete type %0") +DIAG(err_tentative_def_incomplete_type, ERROR, + "tentative definition has type %0 that is never completed") +DIAG(err_tentative_def_incomplete_type_arr, ERROR, + "tentative definition has array of type %0 that is never completed") + DIAG(err_realimag_invalid_type, ERROR, "invalid type %0 to %1 operator") DIAG(err_typecheck_sclass_fscope, ERROR, diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index eaf69f0197..9bc07d5ce3 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -273,6 +273,22 @@ void VarDecl::Destroy(ASTContext& C) { VarDecl::~VarDecl() { } +bool VarDecl::isTentativeDefinition(ASTContext &Context) const { + if (!isFileVarDecl() || Context.getLangOptions().CPlusPlus) + return false; + + return (!getInit() && + (getStorageClass() == None || getStorageClass() == Static)); +} + +const Expr *VarDecl::getDefinition(const VarDecl *&Def) { + Def = this; + while (Def && !Def->getInit()) + Def = Def->getPreviousDeclaration(); + + return Def? Def->getInit() : 0; +} + //===----------------------------------------------------------------------===// // FunctionDecl Implementation //===----------------------------------------------------------------------===// diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index 57ed9884ba..d8610133f2 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -218,7 +218,52 @@ void Sema::DeleteStmt(StmtTy *S) { /// translation unit when EOF is reached and all but the top-level scope is /// popped. void Sema::ActOnEndOfTranslationUnit() { + // C99 6.9.2p2: + // A declaration of an identifier for an object that has file + // scope without an initializer, and without a storage-class + // specifier or with the storage-class specifier static, + // constitutes a tentative definition. If a translation unit + // contains one or more tentative definitions for an identifier, + // and the translation unit contains no external definition for + // that identifier, then the behavior is exactly as if the + // translation unit contains a file scope declaration of that + // identifier, with the composite type as of the end of the + // translation unit, with an initializer equal to 0. + if (!getLangOptions().CPlusPlus) { + // Note: we traverse the scope's list of declarations rather than + // the DeclContext's list, because we only want to see the most + // recent declaration of each identifier. + for (Scope::decl_iterator I = TUScope->decl_begin(), + IEnd = TUScope->decl_end(); + I != IEnd; ++I) { + Decl *D = static_cast(*I); + if (D->isInvalidDecl()) + continue; + if (VarDecl *VD = dyn_cast(D)) { + if (VD->isTentativeDefinition(Context)) { + if (const IncompleteArrayType *ArrayT + = Context.getAsIncompleteArrayType(VD->getType())) { + if (RequireCompleteType(VD->getLocation(), + ArrayT->getElementType(), + diag::err_tentative_def_incomplete_type_arr)) + VD->setInvalidDecl(); + else { + // Set the length of the array to 1 (C99 6.9.2p5). + llvm::APSInt One(Context.getTypeSize(Context.getSizeType()), + true); + QualType T + = Context.getConstantArrayType(ArrayT->getElementType(), + One, ArrayType::Normal, 0); + VD->setType(T); + } + } else if (RequireCompleteType(VD->getLocation(), VD->getType(), + diag::err_tentative_def_incomplete_type)) + VD->setInvalidDecl(); + } + } + } + } } diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index c4964bf69b..b3669bc74d 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -451,7 +451,6 @@ public: bool MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old); bool MergeVarDecl(VarDecl *New, Decl *Old); bool MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old); - void CheckForFileScopedRedefinitions(Scope *S, VarDecl *VD); /// C++ Overloading. bool IsOverload(FunctionDecl *New, Decl* OldD, diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index c5768fd73a..35573bb33d 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -776,56 +776,6 @@ bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old) { return false; } -/// Predicate for C "tentative" external object definitions (C99 6.9.2). -static bool isTentativeDefinition(VarDecl *VD) { - if (VD->isFileVarDecl()) - return (!VD->getInit() && - (VD->getStorageClass() == VarDecl::None || - VD->getStorageClass() == VarDecl::Static)); - return false; -} - -/// CheckForFileScopedRedefinitions - Make sure we forgo redefinition errors -/// when dealing with C "tentative" external object definitions (C99 6.9.2). -void Sema::CheckForFileScopedRedefinitions(Scope *S, VarDecl *VD) { - bool VDIsTentative = isTentativeDefinition(VD); - bool VDIsIncompleteArray = VD->getType()->isIncompleteArrayType(); - - // FIXME: I don't think this will actually see all of the - // redefinitions. Can't we check this property on-the-fly? - for (IdentifierResolver::iterator I = IdResolver.begin(VD->getIdentifier()), - E = IdResolver.end(); - I != E; ++I) { - if (*I != VD && isDeclInScope(*I, VD->getDeclContext(), S)) { - VarDecl *OldDecl = dyn_cast(*I); - - // Handle the following case: - // int a[10]; - // int a[]; - the code below makes sure we set the correct type. - // int a[11]; - this is an error, size isn't 10. - if (OldDecl && VDIsTentative && VDIsIncompleteArray && - OldDecl->getType()->isConstantArrayType()) - VD->setType(OldDecl->getType()); - - // Check for "tentative" definitions. We can't accomplish this in - // MergeVarDecl since the initializer hasn't been attached. - if (!OldDecl || isTentativeDefinition(OldDecl) || VDIsTentative) - continue; - - // Handle __private_extern__ just like extern. - if (OldDecl->getStorageClass() != VarDecl::Extern && - OldDecl->getStorageClass() != VarDecl::PrivateExtern && - VD->getStorageClass() != VarDecl::Extern && - VD->getStorageClass() != VarDecl::PrivateExtern) { - Diag(VD->getLocation(), diag::err_redefinition) << VD->getDeclName(); - Diag(OldDecl->getLocation(), diag::note_previous_definition); - // One redefinition error is enough. - break; - } - } - } -} - /// MergeVarDecl - We just parsed a variable 'New' which has the same name /// and scope as a previous declaration 'Old'. Figure out how to resolve this /// situation, merging decls or emitting diagnostics as appropriate. @@ -876,6 +826,10 @@ bool Sema::MergeVarDecl(VarDecl *New, Decl *OldD) { Diag(Old->getLocation(), diag::note_previous_definition); return true; } + + // Keep a chain of previous declarations. + New->setPreviousDeclaration(Old); + return false; } @@ -2168,6 +2122,15 @@ void Sema::AddInitializerToDecl(DeclTy *dcl, ExprArg init, bool DirectInit) { return; } + const VarDecl *Def = 0; + if (VDecl->getDefinition(Def)) { + Diag(VDecl->getLocation(), diag::err_redefinition) + << VDecl->getDeclName(); + Diag(Def->getLocation(), diag::note_previous_definition); + VDecl->setInvalidDecl(); + return; + } + // Take ownership of the expression, now that we're sure we have somewhere // to put it. Expr *Init = static_cast(init.release()); @@ -2349,7 +2312,7 @@ Sema::DeclTy *Sema::FinalizeDeclaratorGroup(Scope *S, DeclTy *group) { // storage-class specifier or with the storage-class specifier "static", // constitutes a tentative definition. Note: A tentative definition with // external linkage is valid (C99 6.2.2p5). - if (!getLangOptions().CPlusPlus && isTentativeDefinition(IDecl)) { + if (IDecl->isTentativeDefinition(Context)) { QualType CheckType = T; unsigned DiagID = diag::err_typecheck_decl_incomplete_type; @@ -2369,8 +2332,6 @@ Sema::DeclTy *Sema::FinalizeDeclaratorGroup(Scope *S, DeclTy *group) { IDecl->setInvalidDecl(); } } - if (IDecl->isFileVarDecl()) - CheckForFileScopedRedefinitions(S, IDecl); } return NewGroup; } diff --git a/test/Sema/enum.c b/test/Sema/enum.c index 37c9a1e7cf..dbc250abc3 100644 --- a/test/Sema/enum.c +++ b/test/Sema/enum.c @@ -21,7 +21,9 @@ int test() { return sizeof(enum e) ; } -enum gccForwardEnumExtension ve; // expected-warning{{ISO C forbids forward references to 'enum' types}} +enum gccForwardEnumExtension ve; // expected-warning{{ISO C forbids forward references to 'enum' types}} \ +// expected-error{{tentative definition has type 'enum gccForwardEnumExtension' that is never completed}} \ +// expected-note{{forward declaration of 'enum gccForwardEnumExtension'}} int test2(int i) { diff --git a/test/Sema/incomplete-decl.c b/test/Sema/incomplete-decl.c index 7ec436acce..de957403a3 100644 --- a/test/Sema/incomplete-decl.c +++ b/test/Sema/incomplete-decl.c @@ -1,9 +1,9 @@ // RUN: clang -fsyntax-only -verify %s -struct foo; // expected-note 3 {{forward declaration of 'struct foo'}} +struct foo; // expected-note 4 {{forward declaration of 'struct foo'}} void b; // expected-error {{variable has incomplete type 'void'}} -struct foo f; // // FIXME: error because 'struct foo' is never defined +struct foo f; // expected-error{{tentative definition has type 'struct foo' that is never completed}} static void c; // expected-error {{variable has incomplete type 'void'}} static struct foo g; // expected-error {{variable has incomplete type 'struct foo'}} diff --git a/test/Sema/init.c b/test/Sema/init.c index 1f84e41eab..6f592b80e6 100644 --- a/test/Sema/init.c +++ b/test/Sema/init.c @@ -74,7 +74,8 @@ int sym_fw1a_scr[] = { }; // PR3001 -struct s1 s2 = { +struct s1 s2 = { // expected-error{{tentative definition has type 'struct s1' that is never completed}} \ + // expected-note{{forward declaration of 'struct s1'}} .a = sizeof(struct s3), // expected-error {{invalid application of 'sizeof'}} \ // expected-note{{forward declaration of 'struct s3'}} .b = bogus // expected-error {{use of undeclared identifier 'bogus'}} diff --git a/test/Sema/tentative-decls.c b/test/Sema/tentative-decls.c index 3c1ab0e658..23297f3a22 100644 --- a/test/Sema/tentative-decls.c +++ b/test/Sema/tentative-decls.c @@ -5,7 +5,10 @@ struct a x1; // expected-note 2{{forward declaration of 'struct a'}} static struct a x2; // expected-error{{variable has incomplete type 'struct a'}} struct a x3[10]; // expected-error{{array has incomplete element type 'struct a'}} struct a {int x;}; -struct b x4; // FIXME: error because 'struct b' is never defined +static struct a x2_okay; +struct a x3_okay[10]; +struct b x4; // expected-error{{tentative definition has type 'struct b' that is never completed}} \ + // expected-note{{forward declaration of 'struct b'}} const int a [1] = {1}; extern const int a[]; @@ -23,8 +26,8 @@ int i1; extern int i1; // expected-note {{previous definition is here}} static int i1; // expected-error{{static declaration of 'i1' follows non-static declaration}} -static int i2 = 5; // expected-note 2 {{previous definition is here}} -int i2 = 3; // expected-error{{redefinition of 'i2'}} expected-error{{non-static declaration of 'i2' follows static declaration}} +static int i2 = 5; // expected-note 1 {{previous definition is here}} +int i2 = 3; // expected-error{{non-static declaration of 'i2' follows static declaration}} __private_extern__ int pExtern; int pExtern = 0; -- 2.40.0