From: Douglas Gregor Date: Mon, 2 Mar 2009 00:19:53 +0000 (+0000) Subject: Rework the way we find locally-scoped external declarations when we X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6393519272ce727f4d26e71bbefb5de712274d0e;p=clang Rework the way we find locally-scoped external declarations when we need them to evaluate redeclarations or call a function that hasn't already been declared. We now keep a DenseMap of these locally-scoped declarations so that they are not visible but can be quickly found, e.g., when we're looking for previous declarations or before we go ahead and implicitly declare a function that's being called. Fixes PR3672. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@65792 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index 9536eca300..8cd9f7b251 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -338,6 +338,10 @@ public: return false; } + /// \brief Determines whether this variable is a variable with + /// external, C linkage. + bool isExternC(ASTContext &Context) const; + // Implement isa/cast/dyncast/etc. static bool classof(const Decl *D) { return D->getKind() >= VarFirst && D->getKind() <= VarLast; @@ -627,6 +631,10 @@ public: /// the entry point into an executable program. bool isMain() const; + /// \brief Determines whether this function is a function with + /// external, C linkage. + bool isExternC(ASTContext &Context) const; + /// getPreviousDeclaration - Return the previous declaration of this /// function. const FunctionDecl *getPreviousDeclaration() const { diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index b30b86980f..1ebfbf79da 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -63,6 +63,28 @@ QualType ParmVarDecl::getOriginalType() const { return getType(); } +bool VarDecl::isExternC(ASTContext &Context) const { + if (!Context.getLangOptions().CPlusPlus) + return (getDeclContext()->isTranslationUnit() && + getStorageClass() != Static) || + (getDeclContext()->isFunctionOrMethod() && hasExternalStorage()); + + for (const DeclContext *DC = getDeclContext(); !DC->isTranslationUnit(); + DC = DC->getParent()) { + if (const LinkageSpecDecl *Linkage = dyn_cast(DC)) { + if (Linkage->getLanguage() == LinkageSpecDecl::lang_c) + return getStorageClass() != Static; + + break; + } + + if (DC->isFunctionOrMethod()) + return false; + } + + return false; +} + OriginalParmVarDecl *OriginalParmVarDecl::Create( ASTContext &C, DeclContext *DC, SourceLocation L, IdentifierInfo *Id, @@ -273,6 +295,25 @@ bool FunctionDecl::isMain() const { getIdentifier() && getIdentifier()->isStr("main"); } +bool FunctionDecl::isExternC(ASTContext &Context) const { + // In C, any non-static, non-overloadable function has external + // linkage. + if (!Context.getLangOptions().CPlusPlus) + return getStorageClass() != Static && !getAttr(); + + for (const DeclContext *DC = getDeclContext(); !DC->isTranslationUnit(); + DC = DC->getParent()) { + if (const LinkageSpecDecl *Linkage = dyn_cast(DC)) { + if (Linkage->getLanguage() == LinkageSpecDecl::lang_c) + return getStorageClass() != Static && !getAttr(); + + break; + } + } + + return false; +} + /// \brief Returns a value indicating whether this function /// corresponds to a builtin function. /// diff --git a/lib/Sema/IdentifierResolver.cpp b/lib/Sema/IdentifierResolver.cpp index 6efedb4895..609c872395 100644 --- a/lib/Sema/IdentifierResolver.cpp +++ b/lib/Sema/IdentifierResolver.cpp @@ -75,6 +75,18 @@ void IdentifierResolver::IdDeclInfo::RemoveDecl(NamedDecl *D) { assert(0 && "Didn't find this decl on its identifier's chain!"); } +bool +IdentifierResolver::IdDeclInfo::ReplaceDecl(NamedDecl *Old, NamedDecl *New) { + for (DeclsTy::iterator I = Decls.end(); I != Decls.begin(); --I) { + if (Old == *(I-1)) { + *(I - 1) = New; + return true; + } + } + + return false; +} + //===----------------------------------------------------------------------===// // IdentifierResolver Implementation @@ -192,6 +204,27 @@ void IdentifierResolver::RemoveDecl(NamedDecl *D) { return toIdDeclInfo(Ptr)->RemoveDecl(D); } +bool IdentifierResolver::ReplaceDecl(NamedDecl *Old, NamedDecl *New) { + assert(Old->getDeclName() == New->getDeclName() && + "Cannot replace a decl with another decl of a different name"); + + DeclarationName Name = Old->getDeclName(); + void *Ptr = Name.getFETokenInfo(); + + if (!Ptr) + return false; + + if (isDeclPtr(Ptr)) { + if (Ptr == Old) { + Name.setFETokenInfo(New); + return true; + } + return false; + } + + return toIdDeclInfo(Ptr)->ReplaceDecl(Old, New); +} + /// begin - Returns an iterator for decls with name 'Name'. IdentifierResolver::iterator IdentifierResolver::begin(DeclarationName Name) { diff --git a/lib/Sema/IdentifierResolver.h b/lib/Sema/IdentifierResolver.h index 067900eac1..1843f4ebca 100644 --- a/lib/Sema/IdentifierResolver.h +++ b/lib/Sema/IdentifierResolver.h @@ -51,6 +51,11 @@ class IdentifierResolver { /// The decl must already be part of the decl chain. void RemoveDecl(NamedDecl *D); + /// Replaces the Old declaration with the New declaration. If the + /// replacement is successful, returns true. If the old + /// declaration was not found, returns false. + bool ReplaceDecl(NamedDecl *Old, NamedDecl *New); + private: DeclsTy Decls; }; @@ -167,6 +172,11 @@ public: /// The decl must already be part of the decl chain. void RemoveDecl(NamedDecl *D); + /// Replace the decl Old with the new declaration New on its + /// identifier chain. Returns true if the old declaration was found + /// (and, therefore, replaced). + bool ReplaceDecl(NamedDecl *Old, NamedDecl *New); + explicit IdentifierResolver(const LangOptions &LangOpt); ~IdentifierResolver(); diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index 6e0b935b88..30550c423d 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -149,6 +149,33 @@ public: /// FieldCollector - Collects CXXFieldDecls during parsing of C++ classes. llvm::OwningPtr FieldCollector; + /// \brief A mapping from external names to the most recent + /// locally-scoped external declaration with that name. + /// + /// This map contains external declarations introduced in local + /// scoped, e.g., + /// + /// \code + /// void f() { + /// void foo(int, int); + /// } + /// \endcode + /// + /// Here, the name "foo" will be associated with the declaration on + /// "foo" within f. This name is not visible outside of + /// "f". However, we still find it in two cases: + /// + /// - If we are declaring another external with the name "foo", we + /// can find "foo" as a previous declaration, so that the types + /// of this external declaration can be checked for + /// compatibility. + /// + /// - If we would implicitly declare "foo" (e.g., due to a call to + /// "foo" in C when no prototype or definition is visible), then + /// we find this declaration of "foo" and complain that it is + /// not visible. + llvm::DenseMap LocallyScopedExternalDecls; + IdentifierResolver IdResolver; // Enum values used by KnownFunctionIDs (see below). @@ -267,11 +294,12 @@ public: } DeclTy *ActOnDeclarator(Scope *S, Declarator &D, DeclTy *LastInGroup, bool IsFunctionDefinition); + void RegisterLocallyScopedExternCDecl(NamedDecl *ND, NamedDecl *PrevDecl, + Scope *S); NamedDecl* ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC, QualType R, Decl* LastDeclarator, Decl* PrevDecl, bool& InvalidDecl, bool &Redeclaration); - void InjectLocallyScopedExternalDeclaration(ValueDecl *VD); NamedDecl* ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC, QualType R, Decl* LastDeclarator, NamedDecl* PrevDecl, bool& InvalidDecl, diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 8764cd9d77..33e3921688 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -1355,6 +1355,33 @@ static QualType TryToFixInvalidVariablyModifiedType(QualType T, return QualType(); } +/// \brief Register the given locally-scoped external C declaration so +/// that it can be found later for redeclarations +void +Sema::RegisterLocallyScopedExternCDecl(NamedDecl *ND, NamedDecl *PrevDecl, + Scope *S) { + assert(ND->getLexicalDeclContext()->isFunctionOrMethod() && + "Decl is not a locally-scoped decl!"); + // Note that we have a locally-scoped external with this name. + LocallyScopedExternalDecls[ND->getDeclName()] = ND; + + if (!PrevDecl) + return; + + // If there was a previous declaration of this variable, it may be + // in our identifier chain. Update the identifier chain with the new + // declaration. + if (IdResolver.ReplaceDecl(PrevDecl, ND)) { + // The previous declaration was found on the identifer resolver + // chain, so remove it from its scope. + while (S && !S->isDeclScope(PrevDecl)) + S = S->getParent(); + + if (S) + S->RemoveDecl(PrevDecl); + } +} + NamedDecl* Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC, QualType R, Decl* LastDeclarator, @@ -1476,63 +1503,6 @@ isOutOfScopePreviousDeclaration(NamedDecl *PrevDecl, DeclContext *DC, return true; } -/// \brief Inject a locally-scoped declaration with external linkage -/// into the appropriate namespace scope. -/// -/// Given a declaration of an entity with linkage that occurs within a -/// local scope, this routine inject that declaration into top-level -/// scope so that it will be visible for later uses and declarations -/// of the same entity. -void Sema::InjectLocallyScopedExternalDeclaration(ValueDecl *VD) { - // FIXME: We don't do this in C++ because, although we would like - // to get the extra checking that this operation implies, - // the declaration itself is not visible according to C++'s rules. - assert(!getLangOptions().CPlusPlus && - "Can't inject locally-scoped declarations in C++"); - IdentifierResolver::iterator I = IdResolver.begin(VD->getDeclName()), - IEnd = IdResolver.end(); - NamedDecl *PrevDecl = 0; - while (I != IEnd && !isa((*I)->getDeclContext())) { - PrevDecl = *I; - ++I; - } - - if (I == IEnd) { - // No name with this identifier has been declared at translation - // unit scope. Add this name into the appropriate scope. - if (PrevDecl) - IdResolver.AddShadowedDecl(VD, PrevDecl); - else - IdResolver.AddDecl(VD); - TUScope->AddDecl(VD); - return; - } - - if (isa(*I)) { - // The first thing we found was a tag declaration, so insert - // this function so that it will be found before the tag - // declaration. - if (PrevDecl) - IdResolver.AddShadowedDecl(VD, PrevDecl); - else - IdResolver.AddDecl(VD); - TUScope->AddDecl(VD); - return; - } - - if (VD->declarationReplaces(*I)) { - // We found a previous declaration of the same entity. Replace - // that declaration with this one. - TUScope->RemoveDecl(*I); - TUScope->AddDecl(VD); - IdResolver.RemoveDecl(*I); - if (PrevDecl) - IdResolver.AddShadowedDecl(VD, PrevDecl); - else - IdResolver.AddDecl(VD); - } -} - NamedDecl* Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC, QualType R, Decl* LastDeclarator, @@ -1666,6 +1636,16 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC, isOutOfScopePreviousDeclaration(PrevDecl, DC, Context))) PrevDecl = 0; + if (!PrevDecl && NewVD->isExternC(Context)) { + // Since we did not find anything by this name and we're declaring + // an extern "C" variable, look for a non-visible extern "C" + // declaration with the same name. + llvm::DenseMap::iterator Pos + = LocallyScopedExternalDecls.find(Name); + if (Pos != LocallyScopedExternalDecls.end()) + PrevDecl = Pos->second; + } + // Merge the decl with the existing one if appropriate. if (PrevDecl) { if (isa(PrevDecl) && D.getCXXScopeSpec().isSet()) { @@ -1689,12 +1669,11 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC, } } - // If this is a locally-scoped extern variable in C, inject a - // declaration into translation unit scope so that all external - // declarations are visible. - if (!getLangOptions().CPlusPlus && CurContext->isFunctionOrMethod() && - NewVD->hasLinkage()) - InjectLocallyScopedExternalDeclaration(NewVD); + // If this is a locally-scoped extern C variable, update the map of + // such variables. + if (CurContext->isFunctionOrMethod() && NewVD->isExternC(Context) && + !InvalidDecl) + RegisterLocallyScopedExternCDecl(NewVD, PrevDecl, S); return NewVD; } @@ -1920,6 +1899,16 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC, isOutOfScopePreviousDeclaration(PrevDecl, DC, Context))) PrevDecl = 0; + if (!PrevDecl && NewFD->isExternC(Context)) { + // Since we did not find anything by this name and we're declaring + // an extern "C" function, look for a non-visible extern "C" + // declaration with the same name. + llvm::DenseMap::iterator Pos + = LocallyScopedExternalDecls.find(Name); + if (Pos != LocallyScopedExternalDecls.end()) + PrevDecl = Pos->second; + } + // Merge or overload the declaration with an existing declaration of // the same name, if appropriate. bool OverloadableAttrRequired = false; @@ -2046,11 +2035,11 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC, } } - // If this is a locally-scoped function in C, inject a declaration - // into translation unit scope so that all external declarations are - // visible. - if (!getLangOptions().CPlusPlus && CurContext->isFunctionOrMethod()) - InjectLocallyScopedExternalDeclaration(NewFD); + // If this is a locally-scoped extern C function, update the + // map of such names. + if (CurContext->isFunctionOrMethod() && NewFD->isExternC(Context) + && !InvalidDecl) + RegisterLocallyScopedExternCDecl(NewFD, PrevDecl, S); return NewFD; } @@ -2653,6 +2642,18 @@ Sema::DeclTy *Sema::ActOnFinishFunctionBody(DeclTy *D, StmtArg BodyArg) { /// call, forming a call to an implicitly defined function (per C99 6.5.1p2). NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc, IdentifierInfo &II, Scope *S) { + // Before we produce a declaration for an implicitly defined + // function, see whether there was a locally-scoped declaration of + // this name as a function or variable. If so, use that + // (non-visible) declaration, and complain about it. + llvm::DenseMap::iterator Pos + = LocallyScopedExternalDecls.find(&II); + if (Pos != LocallyScopedExternalDecls.end()) { + Diag(Loc, diag::warn_use_out_of_scope_declaration) << Pos->second; + Diag(Pos->second->getLocation(), diag::note_previous_declaration); + return Pos->second; + } + // Extension in C99. Legal in C90, but warn about it. if (getLangOptions().C99) Diag(Loc, diag::ext_implicit_function_decl) << &II; diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 1a93039b03..aca19cdca6 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -84,29 +84,6 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, SourceLocation Loc) { Diag(D->getLocation(), diag::note_unavailable_here) << 0; } - if (D->getDeclContext()->isFunctionOrMethod() && - !D->getDeclContext()->Encloses(CurContext)) { - // We've found the name of a function or variable that was - // declared with external linkage within another function (and, - // therefore, a scope where we wouldn't normally see the - // declaration). Once we've made sure that no previous declaration - // was properly made visible, produce a warning. - bool HasGlobalScopedDeclaration = false; - for (const FunctionDecl *FD = dyn_cast(D); FD; - FD = FD->getPreviousDeclaration()) { - if (FD->getDeclContext()->isFileContext()) { - HasGlobalScopedDeclaration = true; - break; - } - } - // FIXME: do the same thing for variable declarations - - if (!HasGlobalScopedDeclaration) { - Diag(Loc, diag::warn_use_out_of_scope_declaration) << D; - Diag(D->getLocation(), diag::note_previous_declaration); - } - } - return false; } diff --git a/test/Sema/function-redecl.c b/test/Sema/function-redecl.c index e8252db343..4ba97c2a8f 100644 --- a/test/Sema/function-redecl.c +++ b/test/Sema/function-redecl.c @@ -74,6 +74,7 @@ void outer_test() { int outer5(int); // expected-error{{redefinition of 'outer5' as different kind of symbol}} int* outer6(int); // expected-note{{previous declaration is here}} int *outer7(int); + int outer8(int); int *ip7 = outer7(6); } @@ -86,3 +87,9 @@ void outer_test2(int x) { int* ip = outer6(x); // expected-warning{{use of out-of-scope declaration of 'outer6'}} int *ip2 = outer7(x); } + +void outer_test3() { + int *(*fp)(int) = outer8; // expected-error{{use of undeclared identifier 'outer8'}} +} + +static float outer8(float); // okay diff --git a/test/Sema/var-redecl.c b/test/Sema/var-redecl.c index 317a0f578f..82ce58bcb9 100644 --- a/test/Sema/var-redecl.c +++ b/test/Sema/var-redecl.c @@ -49,3 +49,8 @@ void outer_shadowing_test() { } } } + +void g18(void) { + extern int g19; +} +int *p=&g19; // expected-error{{use of undeclared identifier 'g19'}}