When we're declaring an object or function with linkage, teach name

lookup to skip over names without linkage. This finishes
<rdar://problem/6127293>.

llvm-svn: 65386

clang/include/clang/AST/Decl.h

@@ -109,6 +109,9 @@ public:
   /// overloaded function.
   bool declarationReplaces(NamedDecl *OldD) const;
 
+  /// \brief Determine whether this declaration has linkage.
+  bool hasLinkage() const;
+
   static bool classof(const Decl *D) {
     return D->getKind() >= NamedFirst && D->getKind() <= NamedLast;
   }

clang/lib/AST/Decl.cpp

@@ -205,6 +205,15 @@ bool NamedDecl::declarationReplaces(NamedDecl *OldD) const {
   return this->getKind() == OldD->getKind();
 }
 
+bool NamedDecl::hasLinkage() const {
+  if (const VarDecl *VD = dyn_cast<VarDecl>(this))
+    return VD->hasExternalStorage() || VD->isFileVarDecl();
+
+  if (isa<FunctionDecl>(this) && !isa<CXXMethodDecl>(this))
+    return true;
+
+  return false;
+}
 
 //===----------------------------------------------------------------------===//
 // VarDecl Implementation

clang/lib/Sema/Sema.h

@@ -567,7 +567,12 @@ public:
     /// Look up a namespace name within a C++ using directive or
     /// namespace alias definition, ignoring non-namespace names (C++
     /// [basic.lookup.udir]p1).
-    LookupNamespaceName
+    LookupNamespaceName,
+    // Look up an ordinary name that is going to be redeclared as a
+    // name with linkage. This lookup ignores any declarations that
+    // are outside of the current scope unless they have linkage. See 
+    // C99 6.2.2p4-5 and C++ [basic.link]p6.
+    LookupRedeclarationWithLinkage
   };
 
   /// @brief Represents the results of name lookup.
@@ -807,12 +812,13 @@ private:
 public:
   /// Determines whether D is a suitable lookup result according to the
   /// lookup criteria.
-  static bool isAcceptableLookupResult(Decl *D, LookupNameKind NameKind,
+  static bool isAcceptableLookupResult(NamedDecl *D, LookupNameKind NameKind,
                                        unsigned IDNS) {
     switch (NameKind) {
     case Sema::LookupOrdinaryName:
     case Sema::LookupTagName:
     case Sema::LookupMemberName:
+    case Sema::LookupRedeclarationWithLinkage: // FIXME: check linkage, scoping
       return D->isInIdentifierNamespace(IDNS);
       
     case Sema::LookupOperatorName:

clang/lib/Sema/SemaDecl.cpp

@@ -1334,10 +1334,29 @@ Sema::ActOnDeclarator(Scope *S, Declarator &D, DeclTy *lastDecl,
   NamedDecl *New;
   bool InvalidDecl = false;
 
+  QualType R = GetTypeForDeclarator(D, S);
+  if (R.isNull()) {
+    InvalidDecl = true;
+    R = Context.IntTy;
+  }
+
   // See if this is a redefinition of a variable in the same scope.
   if (!D.getCXXScopeSpec().isSet() && !D.getCXXScopeSpec().isInvalid()) {
+    LookupNameKind NameKind = LookupOrdinaryName;
+
+    // If the declaration we're planning to build will be a function
+    // or object with linkage, then look for another declaration with
+    // linkage (C99 6.2.2p4-5 and C++ [basic.link]p6).
+    if (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_typedef)
+      /* Do nothing*/;
+    else if (R->isFunctionType()) {
+      if (CurContext->isFunctionOrMethod())
+        NameKind = LookupRedeclarationWithLinkage;
+    } else if (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_extern)
+      NameKind = LookupRedeclarationWithLinkage;
+
     DC = CurContext;
-    PrevDecl = LookupName(S, Name, LookupOrdinaryName, true, 
+    PrevDecl = LookupName(S, Name, NameKind, true, 
                           D.getDeclSpec().getStorageClassSpec() != 
                             DeclSpec::SCS_static,
                           D.getIdentifierLoc());
@@ -1402,17 +1421,11 @@ Sema::ActOnDeclarator(Scope *S, Declarator &D, DeclTy *lastDecl,
       D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_typedef)
     PrevDecl = 0;
 
-  QualType R = GetTypeForDeclarator(D, S);
-  if (R.isNull()) {
-    InvalidDecl = true;
-    R = Context.IntTy;
-  }
-
   bool Redeclaration = false;
   if (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_typedef) {
     New = ActOnTypedefDeclarator(S, D, DC, R, LastDeclarator, PrevDecl,
                                  InvalidDecl, Redeclaration);
-  } else if (R.getTypePtr()->isFunctionType()) {
+  } else if (R->isFunctionType()) {
     New = ActOnFunctionDeclarator(S, D, DC, R, LastDeclarator, PrevDecl, 
                                   IsFunctionDefinition, InvalidDecl,
                                   Redeclaration);
@@ -1561,6 +1574,8 @@ isOutOfScopePreviousDeclaration(NamedDecl *PrevDecl, DeclContext *DC,
 
   // FIXME: PrevDecl could be an OverloadedFunctionDecl, in which
   // case we need to check each of the overloaded functions.
+  if (!PrevDecl->hasLinkage())
+    return false;
 
   if (Context.getLangOptions().CPlusPlus) {
     // C++ [basic.link]p6:
@@ -1595,13 +1610,6 @@ isOutOfScopePreviousDeclaration(NamedDecl *PrevDecl, DeclContext *DC,
     }
   }
 
-  // If the declaration we've found has no linkage, ignore it. 
-  if (VarDecl *VD = dyn_cast<VarDecl>(PrevDecl)) {
-    if (!VD->hasGlobalStorage())
-      return false;
-  } else if (!isa<FunctionDecl>(PrevDecl))
-    return false;
-
   return true;
 }
 
@@ -1755,9 +1763,7 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   // same scope as the new declaration, this may still be an
   // acceptable redeclaration.
   if (PrevDecl && !isDeclInScope(PrevDecl, DC, S) &&
-      !((NewVD->hasExternalStorage() ||
-         (NewVD->isFileVarDecl() && 
-          NewVD->getStorageClass() != VarDecl::Static)) &&
+      !(NewVD->hasLinkage() &&
         isOutOfScopePreviousDeclaration(PrevDecl, DC, Context)))
     PrevDecl = 0;     
 
@@ -1788,7 +1794,7 @@ Sema::ActOnVariableDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   // declaration into translation unit scope so that all external
   // declarations are visible.
   if (!getLangOptions().CPlusPlus && CurContext->isFunctionOrMethod() &&
-      NewVD->hasExternalStorage())
+      NewVD->hasLinkage())
     InjectLocallyScopedExternalDeclaration(NewVD);
 
   return NewVD;
@@ -2006,9 +2012,8 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   // same scope as the new declaration, this may still be an
   // acceptable redeclaration.
   if (PrevDecl && !isDeclInScope(PrevDecl, DC, S) &&
-      (isa<CXXMethodDecl>(NewFD) ||
-       NewFD->getStorageClass() == FunctionDecl::Static ||
-       !isOutOfScopePreviousDeclaration(PrevDecl, DC, Context)))
+      !(NewFD->hasLinkage() &&
+        isOutOfScopePreviousDeclaration(PrevDecl, DC, Context)))
     PrevDecl = 0;
 
   // Merge or overload the declaration with an existing declaration of

clang/lib/Sema/SemaLookup.cpp

@@ -271,6 +271,7 @@ getIdentifierNamespacesFromLookupNameKind(Sema::LookupNameKind NameKind,
   switch (NameKind) {
   case Sema::LookupOrdinaryName:
   case Sema::LookupOperatorName:
+  case Sema::LookupRedeclarationWithLinkage:
     IDNS = Decl::IDNS_Ordinary;
     if (CPlusPlus)
       IDNS |= Decl::IDNS_Tag | Decl::IDNS_Member;
@@ -763,16 +764,40 @@ Sema::LookupName(Scope *S, DeclarationName Name, LookupNameKind NameKind,
     case Sema::LookupNamespaceName:
       assert(false && "C does not perform these kinds of name lookup");
       break;
+
+    case Sema::LookupRedeclarationWithLinkage:
+      // Find the nearest non-transparent declaration scope.
+      while (!(S->getFlags() & Scope::DeclScope) ||
+             (S->getEntity() && 
+              static_cast<DeclContext *>(S->getEntity())
+                ->isTransparentContext()))
+        S = S->getParent();
+      IDNS = Decl::IDNS_Ordinary;
+      break;
     }
 
     // Scan up the scope chain looking for a decl that matches this
     // identifier that is in the appropriate namespace.  This search
     // should not take long, as shadowing of names is uncommon, and
     // deep shadowing is extremely uncommon.
+    bool LeftStartingScope = false;
+
     for (IdentifierResolver::iterator I = IdResolver.begin(Name),
                                    IEnd = IdResolver.end(); 
          I != IEnd; ++I)
       if ((*I)->isInIdentifierNamespace(IDNS)) {
+        if (NameKind == LookupRedeclarationWithLinkage) {
+          // Determine whether this (or a previous) declaration is
+          // out-of-scope.
+          if (!LeftStartingScope && !S->isDeclScope(*I))
+            LeftStartingScope = true;
+
+          // If we found something outside of our starting scope that
+          // does not have linkage, skip it.
+          if (LeftStartingScope && !((*I)->hasLinkage()))
+            continue;
+        }
+
         if ((*I)->getAttr<OverloadableAttr>()) {
           // If this declaration has the "overloadable" attribute, we
           // might have a set of overloaded functions.
@@ -806,7 +831,8 @@ Sema::LookupName(Scope *S, DeclarationName Name, LookupNameKind NameKind,
   // If we didn't find a use of this identifier, and if the identifier
   // corresponds to a compiler builtin, create the decl object for the builtin
   // now, injecting it into translation unit scope, and return it.
-  if (NameKind == LookupOrdinaryName) {
+  if (NameKind == LookupOrdinaryName || 
+      NameKind == LookupRedeclarationWithLinkage) {
     IdentifierInfo *II = Name.getAsIdentifierInfo();
     if (II && AllowBuiltinCreation) {
       // If this is a builtin on this (or all) targets, create the decl.

clang/test/Sema/function-redecl.c

@@ -46,7 +46,7 @@ extern void g3(int); // expected-note{{previous declaration is here}}
 static void g3(int x) { } // expected-error{{static declaration of 'g3' follows non-static declaration}}
 
 void test2() {
-  extern int f2; // expected-note{{previous definition is here}}
+  extern int f2; // expected-note 2 {{previous definition is here}}
   {
     void f2(int); // expected-error{{redefinition of 'f2' as different kind of symbol}}
   }
@@ -54,7 +54,7 @@ void test2() {
   {
     int f2;
     {
-      void f2(int); // okay
+      void f2(int); // expected-error{{redefinition of 'f2' as different kind of symbol}}
     }
   }
 }

clang/test/Sema/var-redecl.c

@@ -28,3 +28,24 @@ float outer4; // expected-error{{redefinition of 'outer4' with a different type}
 float outer5;  // expected-error{{redefinition of 'outer5' with a different type}}
 int outer8(int); // expected-error{{redefinition of 'outer8' as different kind of symbol}}
 float outer9; // expected-error{{redefinition of 'outer9' with a different type}}
+
+extern int outer13; // expected-note{{previous definition is here}}
+void outer_shadowing_test() {
+  extern int outer10;
+  extern int outer11; // expected-note{{previous definition is here}}
+  extern int outer12; // expected-note{{previous definition is here}}
+  {
+    float outer10;
+    float outer11;
+    float outer12;
+    {
+      extern int outer10; // okay
+      extern float outer11; // expected-error{{redefinition of 'outer11' with a different type}}
+      static double outer12;
+      {
+        extern float outer12; // expected-error{{redefinition of 'outer12' with a different type}}
+        extern float outer13; // expected-error{{redefinition of 'outer13' with a different type}}
+      }
+    }
+  }
+}