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.

llvm-svn: 65792

clang/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 {

clang/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<LinkageSpecDecl>(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<OverloadableAttr>();
+
+  for (const DeclContext *DC = getDeclContext(); !DC->isTranslationUnit(); 
+       DC = DC->getParent()) {
+    if (const LinkageSpecDecl *Linkage = dyn_cast<LinkageSpecDecl>(DC))  {
+      if (Linkage->getLanguage() == LinkageSpecDecl::lang_c)
+        return getStorageClass() != Static && !getAttr<OverloadableAttr>();
+
+      break;
+    }
+  }
+
+  return false;
+}
+
 /// \brief Returns a value indicating whether this function
 /// corresponds to a builtin function.
 ///

clang/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<void>();
+
+  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) {

clang/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();
 

clang/lib/Sema/Sema.h

@@ -149,6 +149,33 @@ public:
   /// FieldCollector - Collects CXXFieldDecls during parsing of C++ classes.
   llvm::OwningPtr<CXXFieldCollector> 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<DeclarationName, NamedDecl *> 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,

clang/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<TranslationUnitDecl>((*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<TagDecl>(*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<DeclarationName, NamedDecl *>::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<FieldDecl>(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<DeclarationName, NamedDecl *>::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<DeclarationName, NamedDecl *>::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;

clang/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<FunctionDecl>(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;
 }
 

clang/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

clang/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'}}