Fix implementation of C11 6.2.7/4 and C++11 [dcl.array]p3:

When a local extern declaration redeclares some other entity, the type of that
entity is merged with the prior type if the prior declaration is visible (in C)
or is declared in the same scope (in C++).

 - Make LookupRedeclarationWithLinkage actually work in C++, use it in the right
   set of cases, and make it track whether it found a shadowed declaration.
 - Track whether we found a declaration in the same scope (for C++) including
   across serialization and template instantiation.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@188307 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/AST/Decl.h

@@ -706,8 +706,13 @@ private:
 
     /// \brief Whether this variable is (C++0x) constexpr.
     unsigned IsConstexpr : 1;
+
+    /// \brief Whether this local extern variable's previous declaration was
+    /// declared in the same block scope. This controls whether we should merge
+    /// the type of this declaration with its previous declaration.
+    unsigned PreviousDeclInSameBlockScope : 1;
   };
-  enum { NumVarDeclBits = 12 };
+  enum { NumVarDeclBits = 13 };
 
   friend class ASTDeclReader;
   friend class StmtIteratorBase;
@@ -1127,6 +1132,15 @@ public:
   bool isConstexpr() const { return VarDeclBits.IsConstexpr; }
   void setConstexpr(bool IC) { VarDeclBits.IsConstexpr = IC; }
 
+  /// Whether this local extern variable declaration's previous declaration
+  /// was declared in the same block scope. Only correct in C++.
+  bool isPreviousDeclInSameBlockScope() const {
+    return VarDeclBits.PreviousDeclInSameBlockScope;
+  }
+  void setPreviousDeclInSameBlockScope(bool Same) {
+    VarDeclBits.PreviousDeclInSameBlockScope = Same;
+  }
+
   /// \brief If this variable is an instantiated static data member of a
   /// class template specialization, returns the templated static data member
   /// from which it was instantiated.

include/clang/Sema/Lookup.h

@@ -139,7 +139,8 @@ public:
       Redecl(Redecl != Sema::NotForRedeclaration),
       HideTags(true),
       Diagnose(Redecl == Sema::NotForRedeclaration),
-      AllowHidden(Redecl == Sema::ForRedeclaration)
+      AllowHidden(Redecl == Sema::ForRedeclaration),
+      Shadowed(false)
   {
     configure();
   }
@@ -160,7 +161,8 @@ public:
       Redecl(Redecl != Sema::NotForRedeclaration),
       HideTags(true),
       Diagnose(Redecl == Sema::NotForRedeclaration),
-      AllowHidden(Redecl == Sema::ForRedeclaration)
+      AllowHidden(Redecl == Sema::ForRedeclaration),
+      Shadowed(false)
   {
     configure();
   }
@@ -179,7 +181,8 @@ public:
       Redecl(Other.Redecl),
       HideTags(Other.HideTags),
       Diagnose(false),
-      AllowHidden(Other.AllowHidden)
+      AllowHidden(Other.AllowHidden),
+      Shadowed(false)
   {}
 
   ~LookupResult() {
@@ -394,6 +397,14 @@ public:
     ResultKind = NotFoundInCurrentInstantiation;
   }
 
+  /// \brief Determine whether the lookup result was shadowed by some other
+  /// declaration that lookup ignored.
+  bool isShadowed() const { return Shadowed; }
+
+  /// \brief Note that we found and ignored a declaration while performing
+  /// lookup.
+  void setShadowed() { Shadowed = true; }
+
   /// \brief Resolves the result kind of the lookup, possibly hiding
   /// decls.
   ///
@@ -482,6 +493,7 @@ public:
     if (Paths) deletePaths(Paths);
     Paths = NULL;
     NamingClass = 0;
+    Shadowed = false;
   }
 
   /// \brief Clears out any current state and re-initializes for a
@@ -660,6 +672,11 @@ private:
 
   /// \brief True if we should allow hidden declarations to be 'visible'.
   bool AllowHidden;
+
+  /// \brief True if the found declarations were shadowed by some other
+  /// declaration that we skipped. This only happens when \c LookupKind
+  /// is \c LookupRedeclarationWithLinkage.
+  bool Shadowed;
 };
 
   /// \brief Consumes visible declarations found when searching for

include/clang/Sema/Sema.h

@@ -1808,7 +1808,7 @@ public:
   /// \param ExplicitInstantiationOrSpecialization When true, we are checking
   /// whether the declaration is in scope for the purposes of explicit template
   /// instantiation or specialization. The default is false.
-  bool isDeclInScope(NamedDecl *&D, DeclContext *Ctx, Scope *S = 0,
+  bool isDeclInScope(NamedDecl *D, DeclContext *Ctx, Scope *S = 0,
                      bool ExplicitInstantiationOrSpecialization = false);
 
   /// Finds the scope corresponding to the given decl context, if it
@@ -1862,13 +1862,15 @@ public:
   void mergeDeclAttributes(NamedDecl *New, Decl *Old,
                            AvailabilityMergeKind AMK = AMK_Redeclaration);
   void MergeTypedefNameDecl(TypedefNameDecl *New, LookupResult &OldDecls);
-  bool MergeFunctionDecl(FunctionDecl *New, Decl *Old, Scope *S);
+  bool MergeFunctionDecl(FunctionDecl *New, Decl *Old, Scope *S,
+                         bool MergeTypeWithOld);
   bool MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old,
-                                    Scope *S);
+                                    Scope *S, bool MergeTypeWithOld);
   void mergeObjCMethodDecls(ObjCMethodDecl *New, ObjCMethodDecl *Old);
-  void MergeVarDecl(VarDecl *New, LookupResult &OldDecls,
+  void MergeVarDecl(VarDecl *New, LookupResult &Previous,
-                    bool OldDeclsWereHidden);
+                    bool MergeTypeWithPrevious);
-  void MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool OldIsHidden);
+  void MergeVarDeclTypes(VarDecl *New, VarDecl *Old,
+                         bool MergeTypeWithOld);
   void MergeVarDeclExceptionSpecs(VarDecl *New, VarDecl *Old);
   bool MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old, Scope *S);
 

lib/Sema/SemaDecl.cpp

@@ -1105,7 +1105,7 @@ void Sema::pushExternalDeclIntoScope(NamedDecl *D, DeclarationName Name) {
     TUScope->AddDecl(D);
 }
 
-bool Sema::isDeclInScope(NamedDecl *&D, DeclContext *Ctx, Scope *S,
+bool Sema::isDeclInScope(NamedDecl *D, DeclContext *Ctx, Scope *S,
                          bool ExplicitInstantiationOrSpecialization) {
   return IdResolver.isDeclInScope(D, Ctx, S,
                                   ExplicitInstantiationOrSpecialization);
@@ -2277,7 +2277,8 @@ static bool haveIncompatibleLanguageLinkages(const T *Old, const T *New) {
 /// merged with.
 ///
 /// Returns true if there was an error, false otherwise.
-bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, Scope *S) {
+bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, Scope *S,
+                             bool MergeTypeWithOld) {
   // Verify the old decl was also a function.
   FunctionDecl *Old = 0;
   if (FunctionTemplateDecl *OldFunctionTemplate
@@ -2614,7 +2615,7 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, Scope *S) {
     }
 
     if (OldQTypeForComparison == NewQType)
-      return MergeCompatibleFunctionDecls(New, Old, S);
+      return MergeCompatibleFunctionDecls(New, Old, S, MergeTypeWithOld);
 
     // Fall through for conflicting redeclarations and redefinitions.
   }
@@ -2626,7 +2627,7 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, Scope *S) {
     const FunctionType *OldFuncType = OldQType->getAs<FunctionType>();
     const FunctionType *NewFuncType = NewQType->getAs<FunctionType>();
     const FunctionProtoType *OldProto = 0;
-    if (isa<FunctionNoProtoType>(NewFuncType) &&
+    if (MergeTypeWithOld && isa<FunctionNoProtoType>(NewFuncType) &&
         (OldProto = dyn_cast<FunctionProtoType>(OldFuncType))) {
       // The old declaration provided a function prototype, but the
       // new declaration does not. Merge in the prototype.
@@ -2659,7 +2660,7 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, Scope *S) {
       New->setParams(Params);
     }
 
-    return MergeCompatibleFunctionDecls(New, Old, S);
+    return MergeCompatibleFunctionDecls(New, Old, S, MergeTypeWithOld);
   }
 
   // GNU C permits a K&R definition to follow a prototype declaration
@@ -2717,9 +2718,10 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, Scope *S) {
                diag::note_previous_declaration);
       }
 
+      if (MergeTypeWithOld)
         New->setType(Context.getFunctionType(MergedReturn, ArgTypes,
                                              OldProto->getExtProtoInfo()));
-      return MergeCompatibleFunctionDecls(New, Old, S);
+      return MergeCompatibleFunctionDecls(New, Old, S, MergeTypeWithOld);
     }
 
     // Fall through to diagnose conflicting types.
@@ -2771,7 +2773,7 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, Scope *S) {
 ///
 /// \returns false
 bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old,
-                                        Scope *S) {
+                                        Scope *S, bool MergeTypeWithOld) {
   // Merge the attributes
   mergeDeclAttributes(New, Old);
 
@@ -2794,9 +2796,10 @@ bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old,
     return MergeCXXFunctionDecl(New, Old, S);
 
   // Merge the function types so the we get the composite types for the return
-  // and argument types.
+  // and argument types. Per C11 6.2.7/4, only update the type if the old decl
+  // was visible.
   QualType Merged = Context.mergeTypes(Old->getType(), New->getType());
-  if (!Merged.isNull())
+  if (!Merged.isNull() && MergeTypeWithOld)
     New->setType(Merged);
 
   return false;
@@ -2830,7 +2833,8 @@ void Sema::mergeObjCMethodDecls(ObjCMethodDecl *newMethod,
 /// Declarations using the auto type specifier (C++ [decl.spec.auto]) call back
 /// to here in AddInitializerToDecl. We can't check them before the initializer
 /// is attached.
-void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool OldWasHidden) {
+void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old,
+                             bool MergeTypeWithOld) {
   if (New->isInvalidDecl() || Old->isInvalidDecl())
     return;
 
@@ -2871,6 +2875,30 @@ void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool OldWasHidden) {
     MergedT = Context.mergeTypes(New->getType(), Old->getType());
   }
   if (MergedT.isNull()) {
+    // It's OK if we couldn't merge types if either type is dependent, for a
+    // block-scope variable. In other cases (static data members of class
+    // templates, variable templates, ...), we require the types to be
+    // equivalent.
+    // FIXME: The C++ standard doesn't say anything about this.
+    if ((New->getType()->isDependentType() ||
+         Old->getType()->isDependentType()) && New->isLocalVarDecl()) {
+      // If the old type was dependent, we can't merge with it, so the new type
+      // becomes dependent for now. We'll reproduce the original type when we
+      // instantiate the TypeSourceInfo for the variable.
+      if (!New->getType()->isDependentType() && MergeTypeWithOld)
+        New->setType(Context.DependentTy);
+      return;
+    }
+
+    // FIXME: Even if this merging succeeds, some other non-visible declaration
+    // of this variable might have an incompatible type. For instance:
+    //
+    //   extern int arr[];
+    //   void f() { extern int arr[2]; }
+    //   void g() { extern int arr[3]; }
+    //
+    // Neither C nor C++ requires a diagnostic for this, but we should still try
+    // to diagnose it.
     Diag(New->getLocation(), diag::err_redefinition_different_type)
       << New->getDeclName() << New->getType() << Old->getType();
     Diag(Old->getLocation(), diag::note_previous_definition);
@@ -2879,7 +2907,7 @@ void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool OldWasHidden) {
 
   // Don't actually update the type on the new declaration if the old
   // declaration was a extern declaration in a different scope.
-  if (!OldWasHidden)
+  if (MergeTypeWithOld)
     New->setType(MergedT);
 }
 
@@ -2892,7 +2920,7 @@ void Sema::MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool OldWasHidden) {
 /// definitions here, since the initializer hasn't been attached.
 ///
 void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous,
-                        bool PreviousWasHidden) {
+                        bool MergeTypeWithPrevious) {
   // If the new decl is already invalid, don't do any other checking.
   if (New->isInvalidDecl())
     return;
@@ -2935,7 +2963,7 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous,
   }
 
   // Merge the types.
-  MergeVarDeclTypes(New, Old, PreviousWasHidden);
+  MergeVarDeclTypes(New, Old, MergeTypeWithPrevious);
   if (New->isInvalidDecl())
     return;
 
@@ -4172,26 +4200,31 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
   // See if this is a redefinition of a variable in the same scope.
   if (!D.getCXXScopeSpec().isSet()) {
     bool IsLinkageLookup = false;
+    bool CreateBuiltins = false;
 
     // 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 the declaration we're planning to build will be declared with
+    // external linkage in the translation unit, create any builtin with
+    // the same name.
     if (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_typedef)
       /* Do nothing*/;
-    else if (R->isFunctionType()) {
+    else if (CurContext->isFunctionOrMethod() &&
-      if (CurContext->isFunctionOrMethod() ||
+             (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_extern ||
+              R->isFunctionType())) {
+      IsLinkageLookup = true;
+      CreateBuiltins =
+          CurContext->getEnclosingNamespaceContext()->isTranslationUnit();
+    } else if (CurContext->getRedeclContext()->isTranslationUnit() &&
                D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static)
-        IsLinkageLookup = true;
+      CreateBuiltins = true;
-    } else if (D.getDeclSpec().getStorageClassSpec() == DeclSpec::SCS_extern)
-      IsLinkageLookup = true;
-    else if (CurContext->getRedeclContext()->isTranslationUnit() &&
-             D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static)
-      IsLinkageLookup = true;
 
     if (IsLinkageLookup)
       Previous.clear(LookupRedeclarationWithLinkage);
 
-    LookupName(Previous, S, /* CreateBuiltins = */ IsLinkageLookup);
+    LookupName(Previous, S, CreateBuiltins);
   } else { // Something like "int foo::x;"
     LookupQualifiedName(Previous, DC);
 
@@ -5241,6 +5274,14 @@ Sema::ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
       Previous, DC, S, shouldConsiderLinkage(NewVD),
       IsExplicitSpecialization || IsVariableTemplateSpecialization);
 
+  // Check whether the previous declaration is in the same block scope. This
+  // affects whether we merge types with it, per C++11 [dcl.array]p3.
+  if (getLangOpts().CPlusPlus &&
+      NewVD->isLocalVarDecl() && NewVD->hasExternalStorage())
+    NewVD->setPreviousDeclInSameBlockScope(
+        Previous.isSingleResult() && !Previous.isShadowed() &&
+        isDeclInScope(Previous.getFoundDecl(), DC, S, false));
+
   if (!getLangOpts().CPlusPlus) {
     D.setRedeclaration(CheckVariableDeclaration(NewVD, Previous));
   } else {
@@ -5714,7 +5755,6 @@ bool Sema::CheckVariableDeclaration(VarDecl *NewVD,
   // If we did not find anything by this name, look for a non-visible
   // extern "C" declaration with the same name.
   //
-  // Clang has a lot of problems with extern local declarations.
   // The actual standards text here is:
   //
   // C++11 [basic.link]p6:
@@ -5726,6 +5766,11 @@ bool Sema::CheckVariableDeclaration(VarDecl *NewVD,
   //   block scope declaration declares that same entity and
   //   receives the linkage of the previous declaration.
   //
+  // C++11 [dcl.array]p3:
+  //   If there is a preceding declaration of the entity in the same
+  //   scope in which the bound was specified, an omitted array bound
+  //   is taken to be the same as in that earlier declaration.
+  //
   // C11 6.2.7p4:
   //   For an identifier with internal or external linkage declared
   //   in a scope in which a prior declaration of that identifier is
@@ -5735,16 +5780,22 @@ bool Sema::CheckVariableDeclaration(VarDecl *NewVD,
   //
   // The most important point here is that we're not allowed to
   // update our understanding of the type according to declarations
-  // not in scope.
+  // not in scope (in C++) or not visible (in C).
-  bool PreviousWasHidden =
+  bool MergeTypeWithPrevious;
-      Previous.empty() &&
+  if (Previous.empty() &&
-      checkForConflictWithNonVisibleExternC(*this, NewVD, Previous);
+      checkForConflictWithNonVisibleExternC(*this, NewVD, Previous))
+    MergeTypeWithPrevious = false;
+  else
+    MergeTypeWithPrevious =
+        !Previous.isShadowed() &&
+        (!getLangOpts().CPlusPlus || NewVD->isPreviousDeclInSameBlockScope() ||
+         !NewVD->getLexicalDeclContext()->isFunctionOrMethod());
 
   // Filter out any non-conflicting previous declarations.
   filterNonConflictingPreviousDecls(Context, NewVD, Previous);
 
   if (!Previous.empty()) {
-    MergeVarDecl(NewVD, Previous, PreviousWasHidden);
+    MergeVarDecl(NewVD, Previous, MergeTypeWithPrevious);
     return true;
   }
   return false;
@@ -6870,8 +6921,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     if (!NewFD->isInvalidDecl())
       D.setRedeclaration(CheckFunctionDeclaration(S, NewFD, Previous,
                                                   isExplicitSpecialization));
-    // Make graceful recovery from an invalid redeclaration.
     else if (!Previous.empty())
+      // Make graceful recovery from an invalid redeclaration.
       D.setRedeclaration(true);
     assert((NewFD->isInvalidDecl() || !D.isRedeclaration() ||
             Previous.getResultKind() != LookupResult::FoundOverloaded) &&
@@ -7230,6 +7281,12 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
   assert(!NewFD->getResultType()->isVariablyModifiedType() 
          && "Variably modified return types are not handled here");
 
+  // Determine whether the type of this function should be merged with
+  // a previous visible declaration. This never happens for functions in C++,
+  // and always happens in C if the previous declaration was visible.
+  bool MergeTypeWithPrevious = !getLangOpts().CPlusPlus &&
+                               !Previous.isShadowed();
+
   // Filter out any non-conflicting previous declarations.
   filterNonConflictingPreviousDecls(Context, NewFD, Previous);
 
@@ -7293,6 +7350,7 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
       // declaration, and thus redeclares that entity...
       Redeclaration = true;
       OldDecl = Previous.getFoundDecl();
+      MergeTypeWithPrevious = false;
 
       // ... except in the presence of __attribute__((overloadable)).
       if (OldDecl->hasAttr<OverloadableAttr>()) {
@@ -7354,7 +7412,7 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
   if (Redeclaration) {
     // NewFD and OldDecl represent declarations that need to be
     // merged.
-    if (MergeFunctionDecl(NewFD, OldDecl, S)) {
+    if (MergeFunctionDecl(NewFD, OldDecl, S, MergeTypeWithPrevious)) {
       NewFD->setInvalidDecl();
       return Redeclaration;
     }
@@ -7936,8 +7994,11 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init,
 
     // If this is a redeclaration, check that the type we just deduced matches
     // the previously declared type.
-    if (VarDecl *Old = VDecl->getPreviousDecl())
+    if (VarDecl *Old = VDecl->getPreviousDecl()) {
-      MergeVarDeclTypes(VDecl, Old, /*OldWasHidden*/ false);
+      // We never need to merge the type, because we cannot form an incomplete
+      // array of auto, nor deduce such a type.
+      MergeVarDeclTypes(VDecl, Old, /*MergeTypeWithPrevious*/false);
+    }
 
     // Check the deduced type is valid for a variable declaration.
     CheckVariableDeclarationType(VDecl);

lib/Sema/SemaLookup.cpp

@@ -852,6 +852,7 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
   assert(getLangOpts().CPlusPlus && "Can perform only C++ lookup");
 
   DeclarationName Name = R.getLookupName();
+  Sema::LookupNameKind NameKind = R.getLookupKind();
 
   // If this is the name of an implicitly-declared special member function,
   // go through the scope stack to implicitly declare
@@ -889,6 +890,7 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
   //
   UnqualUsingDirectiveSet UDirs;
   bool VisitedUsingDirectives = false;
+  bool LeftStartingScope = false;
   DeclContext *OutsideOfTemplateParamDC = 0;
   for (; S && !isNamespaceOrTranslationUnitScope(S); S = S->getParent()) {
     DeclContext *Ctx = static_cast<DeclContext*>(S->getEntity());
@@ -897,6 +899,20 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
     bool Found = false;
     for (; I != IEnd && S->isDeclScope(*I); ++I) {
       if (NamedDecl *ND = R.getAcceptableDecl(*I)) {
+        if (NameKind == LookupRedeclarationWithLinkage) {
+          // Determine whether this (or a previous) declaration is
+          // out-of-scope.
+          if (!LeftStartingScope && !Initial->isDeclScope(*I))
+            LeftStartingScope = true;
+
+          // If we found something outside of our starting scope that
+          // does not have linkage, skip it.
+          if (LeftStartingScope && !((*I)->hasLinkage())) {
+            R.setShadowed();
+            continue;
+          }
+        }
+
         Found = true;
         R.addDecl(ND);
       }
@@ -909,7 +925,7 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
       return true;
     }
 
-    if (R.getLookupKind() == LookupLocalFriendName && !S->isClassScope()) {
+    if (NameKind == LookupLocalFriendName && !S->isClassScope()) {
       // C++11 [class.friend]p11:
       //   If a friend declaration appears in a local class and the name
       //   specified is an unqualified name, a prior declaration is
@@ -1019,7 +1035,7 @@ bool Sema::CppLookupName(LookupResult &R, Scope *S) {
   if (!S) return false;
 
   // If we are looking for members, no need to look into global/namespace scope.
-  if (R.getLookupKind() == LookupMemberName)
+  if (NameKind == LookupMemberName)
     return false;
 
   // Collect UsingDirectiveDecls in all scopes, and recursively all
@@ -1291,9 +1307,11 @@ bool Sema::LookupName(LookupResult &R, Scope *S, bool AllowBuiltinCreation) {
 
           // If we found something outside of our starting scope that
           // does not have linkage, skip it.
-          if (LeftStartingScope && !((*I)->hasLinkage()))
+          if (LeftStartingScope && !((*I)->hasLinkage())) {
+            R.setShadowed();
             continue;
           }
+        }
         else if (NameKind == LookupObjCImplicitSelfParam &&
                  !isa<ImplicitParamDecl>(*I))
           continue;

lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -973,7 +973,8 @@ Decl *TemplateDeclInstantiator::VisitVarTemplateDecl(VarTemplateDecl *D) {
 
   // FIXME: This, and ForVarTemplate, is a hack that is probably unnecessary.
   // We should use a simplified version of VisitVarDecl.
-  VarDecl *VarInst = cast_or_null<VarDecl>(VisitVarDecl(Pattern, /*ForVarTemplate=*/true));
+  VarDecl *VarInst =
+      cast_or_null<VarDecl>(VisitVarDecl(Pattern, /*ForVarTemplate=*/ true));
 
   DeclContext *DC = Owner;
 
@@ -3336,6 +3337,8 @@ void Sema::BuildVariableInstantiation(
   NewVar->setInitStyle(OldVar->getInitStyle());
   NewVar->setCXXForRangeDecl(OldVar->isCXXForRangeDecl());
   NewVar->setConstexpr(OldVar->isConstexpr());
+  NewVar->setPreviousDeclInSameBlockScope(
+      OldVar->isPreviousDeclInSameBlockScope());
   NewVar->setAccess(OldVar->getAccess());
 
   if (!OldVar->isStaticDataMember()) {
@@ -3348,13 +3351,18 @@ void Sema::BuildVariableInstantiation(
   if (NewVar->hasAttrs())
     CheckAlignasUnderalignment(NewVar);
 
-  // FIXME: In theory, we could have a previous declaration for variables that
-  // are not static data members.
-  // FIXME: having to fake up a LookupResult is dumb.
   LookupResult Previous(*this, NewVar->getDeclName(), NewVar->getLocation(),
                         Sema::LookupOrdinaryName, Sema::ForRedeclaration);
 
-  if (!isa<VarTemplateSpecializationDecl>(NewVar))
+  if (NewVar->getLexicalDeclContext()->isFunctionOrMethod() &&
+      OldVar->getPreviousDecl()) {
+    // We have a previous declaration. Use that one, so we merge with the
+    // right type.
+    if (NamedDecl *NewPrev = FindInstantiatedDecl(
+            NewVar->getLocation(), OldVar->getPreviousDecl(), TemplateArgs))
+      Previous.addDecl(NewPrev);
+  } else if (!isa<VarTemplateSpecializationDecl>(NewVar) &&
+             OldVar->hasLinkage())
     LookupQualifiedName(Previous, NewVar->getDeclContext(), false);
 
   CheckVariableDeclaration(NewVar, Previous);

lib/Serialization/ASTReaderDecl.cpp

@@ -939,6 +939,7 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
   VD->VarDeclBits.CXXForRangeDecl = Record[Idx++];
   VD->VarDeclBits.ARCPseudoStrong = Record[Idx++];
   VD->VarDeclBits.IsConstexpr = Record[Idx++];
+  VD->VarDeclBits.PreviousDeclInSameBlockScope = Record[Idx++];
   VD->setCachedLinkage(Linkage(Record[Idx++]));
 
   // Only true variables (not parameters or implicit parameters) can be merged.

lib/Serialization/ASTWriterDecl.cpp

@@ -703,6 +703,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
   Record.push_back(D->isCXXForRangeDecl());
   Record.push_back(D->isARCPseudoStrong());
   Record.push_back(D->isConstexpr());
+  Record.push_back(D->isPreviousDeclInSameBlockScope());
   Record.push_back(D->getLinkageInternal());
 
   if (D->getInit()) {
@@ -737,6 +738,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
       !isa<ParmVarDecl>(D) &&
       !isa<VarTemplateSpecializationDecl>(D) &&
       !D->isConstexpr() &&
+      !D->isPreviousDeclInSameBlockScope() &&
       !SpecInfo)
     AbbrevToUse = Writer.getDeclVarAbbrev();
 
@@ -1622,6 +1624,7 @@ void ASTWriter::WriteDeclsBlockAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(0));                       // isCXXForRangeDecl
   Abv->Add(BitCodeAbbrevOp(0));                       // isARCPseudoStrong
   Abv->Add(BitCodeAbbrevOp(0));                       // isConstexpr
+  Abv->Add(BitCodeAbbrevOp(0));                       // isPrevDeclInSameScope
   Abv->Add(BitCodeAbbrevOp(0));                       // Linkage
   Abv->Add(BitCodeAbbrevOp(0));                       // HasInit
   Abv->Add(BitCodeAbbrevOp(0));                   // HasMemberSpecializationInfo
@@ -1701,6 +1704,7 @@ void ASTWriter::WriteDeclsBlockAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isCXXForRangeDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
   Abv->Add(BitCodeAbbrevOp(0));                         // isConstexpr
+  Abv->Add(BitCodeAbbrevOp(0));                         // isPrevDeclInSameScope
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // Linkage
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // HasInit
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // HasMemberSpecInfo

test/CXX/basic/basic.link/p6.cpp

@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++1y %s
+
+// expected-no-diagnostics
 
 // C++11 [basic.link]p6:
 //   The name of a function declared in block scope and the name
@@ -9,35 +11,34 @@
 //   block scope declaration declares that same entity and
 //   receives the linkage of the previous declaration.
 
-// rdar://13535367
+extern int same_entity;
-namespace test0 {
+constexpr int *get1() {
-  extern "C" int test0_array[];
+  int same_entity = 0; // not the same entity
-  void declare() { extern int test0_array[100]; }
+  {
-  extern "C" int test0_array[];
+    extern int same_entity;
-  int value = sizeof(test0_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+    return &same_entity;
-}
-
-namespace test1 {
-  extern "C" int test1_array[];
-  void test() {
-    { extern int test1_array[100]; }
-    extern int test1_array[];
-    int x = sizeof(test1_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
   }
 }
+static_assert(get1() == &same_entity, "failed to find previous decl");
 
-namespace test2 {
+static int same_entity_2[3];
-  void declare() { extern int test2_array[100]; }
+constexpr int *get2() {
-  extern int test2_array[];
+  // This is a redeclaration of the same entity, even though it doesn't
-  int value = sizeof(test2_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+  // inherit the type of the prior declaration.
+  extern int same_entity_2[];
+  return same_entity_2;
 }
+static_assert(get2() == same_entity_2, "failed to find previous decl");
 
-namespace test3 {
+static int different_entities;
-  void test() {
+constexpr int *get3() {
-    { extern int test3_array[100]; }
+  int different_entities = 0;
-    extern int test3_array[];
+  {
-    int x = sizeof(test3_array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+    // FIXME: This is not a redeclaration of the prior entity, because
+    // it is not visible here. Under DR426, this is ill-formed, and without
+    // it, the static_assert below should fail.
+    extern int different_entities;
+    return &different_entities;
   }
 }
-
+static_assert(get3() == &different_entities, "failed to find previous decl");
-

test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp

@@ -0,0 +1,152 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++1y -triple x86_64-linux-gnu %s
+
+// If there is a preceding declaration of the entity *in the same scope* in
+// which the bound was specified, an omitted array bound is taken to be the
+// same as in that earlier declaration
+
+// rdar://13535367
+namespace test0 {
+  extern "C" int array[];
+  void declare() { extern int array[100]; }
+  int value1 = sizeof(array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+  extern "C" int array[];
+  int value2 = sizeof(array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+}
+
+namespace test1 {
+  extern "C" int array[];
+  void test() {
+    { extern int array[100]; }
+    extern int array[];
+    int x = sizeof(array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+  }
+}
+
+namespace test2 {
+  void declare() { extern int array[100]; }
+  extern int array[];
+  int value = sizeof(array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+}
+
+namespace test3 {
+  void test() {
+    { extern int array[100]; }
+    extern int array[];
+    int x = sizeof(array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+  }
+}
+
+namespace test4 {
+  extern int array[];
+  void test() {
+    extern int array[100];
+    int x = sizeof(array);
+  }
+  int y = sizeof(array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+}
+
+namespace test5 {
+  void test() {
+    extern int array[100];
+    extern int array[];
+    int x = sizeof(array);
+  }
+}
+
+namespace test6 {
+  void test() {
+    extern int array[100];
+    {
+      extern int array[];
+      int x = sizeof(array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+    }
+    int y = sizeof(array);
+  }
+}
+
+namespace test7 {
+  extern int array[100];
+  void test() {
+    extern int array[];
+    int x = sizeof(array); // expected-error {{invalid application of 'sizeof' to an incomplete type 'int []'}}
+  }
+  int y = sizeof(array);
+}
+
+namespace dependent {
+  template<typename T> void f() {
+    extern int arr1[];
+    extern T arr1;
+    extern T arr2;
+    extern int arr2[];
+    static_assert(sizeof(arr1) == 12, "");
+    static_assert(sizeof(arr2) == 12, "");
+
+    // Use a failing test to ensure the type isn't considered dependent.
+    static_assert(sizeof(arr2) == 13, ""); // expected-error {{failed}}
+  }
+
+  void g() { f<int[3]>(); } // expected-note {{in instantiation of}}
+
+  template<typename T> void h1() {
+    extern T arr3;
+    {
+      int arr3;
+      {
+        extern int arr3[];
+        // Detected in template definition.
+        (void)sizeof(arr3); // expected-error {{incomplete}}
+      }
+    }
+  }
+
+  template<typename T> void h2() {
+    extern int arr4[3];
+    {
+      int arr4;
+      {
+        extern T arr4;
+        // Detected in template instantiation.
+        (void)sizeof(arr4); // expected-error {{incomplete}}
+      }
+    }
+  }
+
+  void i() {
+    h1<int[3]>();
+    h2<int[]>(); // expected-note {{in instantiation of}}
+  }
+
+  int arr5[3];
+  template<typename T> void j() {
+    extern T arr5;
+    extern T arr6;
+    (void)sizeof(arr5); // expected-error {{incomplete}}
+    (void)sizeof(arr6); // expected-error {{incomplete}}
+  }
+  int arr6[3];
+
+  void k() { j<int[]>(); } // expected-note {{in instantiation of}}
+
+  template<typename T, typename U> void l() {
+    extern T arrX; // expected-note {{previous}}
+    extern U arrX; // expected-error {{different type: 'int [4]' vs 'int [3]'}}
+    (void)sizeof(arrX); // expected-error {{incomplete}}
+  }
+
+  void m() {
+    l<int[], int[3]>(); // ok
+    l<int[3], int[]>(); // ok
+    l<int[3], int[3]>(); // ok
+    l<int[3], int[4]>(); // expected-note {{in instantiation of}}
+    l<int[], int[]>(); // expected-note {{in instantiation of}}
+  }
+
+  template<typename T> void n() {
+    extern T n_var;
+  }
+  template void n<int>();
+  // FIXME: Diagnose this!
+  float n_var;
+  template void n<double>();
+}

test/PCH/cxx-templates.cpp

@@ -1,23 +1,21 @@
 // Test this without pch.
 // RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -include %S/cxx-templates.h -verify %s -ast-dump -o -
-// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -include %S/cxx-templates.h %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -include %S/cxx-templates.h %s -emit-llvm -o - -DNO_ERRORS | FileCheck %s
 
 // Test with pch.
 // RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -x c++-header -emit-pch -o %t %S/cxx-templates.h
 // RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -include-pch %t -verify %s -ast-dump  -o -
-// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -include-pch %t %s -emit-llvm -o - -error-on-deserialized-decl doNotDeserialize | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -include-pch %t %s -emit-llvm -o - -error-on-deserialized-decl doNotDeserialize -DNO_ERRORS | FileCheck %s
 
 // Test with modules.
 // RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -fmodules -x c++-header -emit-pch -o %t %S/cxx-templates.h
 // RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -fmodules -include-pch %t -verify %s -ast-dump  -o -
-// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -fmodules -include-pch %t %s -emit-llvm -o - -error-on-deserialized-decl doNotDeserialize | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fexceptions -fmodules -include-pch %t %s -emit-llvm -o - -error-on-deserialized-decl doNotDeserialize -DNO_ERRORS | FileCheck %s
 
 // Test with pch and delayed template parsing.
 // RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fdelayed-template-parsing -fexceptions -x c++-header -emit-pch -o %t %S/cxx-templates.h
 // RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fdelayed-template-parsing -fexceptions -include-pch %t -verify %s -ast-dump  -o -
-// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fdelayed-template-parsing -fexceptions -include-pch %t %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -fdelayed-template-parsing -fexceptions -include-pch %t %s -emit-llvm -o - -DNO_ERRORS | FileCheck %s
-
-// expected-no-diagnostics
 
 // CHECK: define weak_odr void @_ZN2S4IiE1mEv
 // CHECK: define linkonce_odr void @_ZN2S3IiE1mEv
@@ -104,3 +102,9 @@ namespace cyclic_module_load {
   extern std::valarray<int> x;
   std::valarray<int> y(x);
 }
+
+#ifndef NO_ERRORS
+// expected-error@cxx-templates.h:305 {{incomplete}}
+template int local_extern::f<int[]>(); // expected-note {{in instantiation of}}
+#endif
+template int local_extern::g<int[]>();

test/PCH/cxx-templates.h

@@ -296,3 +296,18 @@ namespace cyclic_module_load {
     };
   }
 }
+
+namespace local_extern {
+  template<typename T> int f() {
+    extern int arr[3];
+    {
+      extern T arr;
+      return sizeof(arr);
+    }
+  }
+  template<typename T> int g() {
+    extern int arr[3];
+    extern T arr;
+    return sizeof(arr);
+  }
+}

test/Sema/extern-redecl.c

@@ -62,3 +62,27 @@ void test5c() {
   void *(*_malloc)() = &malloc;
   float *(*_calloc)() = &calloc;
 }
+
+void test6() {
+  extern int test6_array1[100];
+  extern int test6_array2[100];
+  void test6_fn1(int*);
+  void test6_fn2(int*);
+  {
+    // Types are only merged from visible declarations.
+    char test6_array2;
+    char test6_fn2;
+    {
+      extern int test6_array1[];
+      extern int test6_array2[];
+      (void)sizeof(test6_array1); // ok
+      (void)sizeof(test6_array2); // expected-error {{incomplete type}}
+
+      void test6_fn1();
+      void test6_fn2();
+      test6_fn1(1.2); // expected-error {{passing 'double' to parameter of incompatible type 'int *'}}
+      // FIXME: This is valid, but we should warn on it.
+      test6_fn2(1.2);
+    }
+  }
+}

test/SemaCXX/function-redecl.cpp

@@ -4,22 +4,27 @@ int foo(int);
 namespace N {
   void f1() {
     void foo(int); // okay
+    void bar(int);
   }
 
-  // FIXME: we shouldn't even need this declaration to detect errors
+  void foo(int); // expected-note 2{{previous declaration is here}}
-  // below.
-  void foo(int); // expected-note{{previous declaration is here}}
 
   void f2() {
-    int foo(int); // expected-error{{functions that differ only in their return type cannot be overloaded}}
+    int foo(int); // expected-error {{functions that differ only in their return type cannot be overloaded}}
+    // FIXME: We should be able to diagnose the conflict between this
+    // declaration of 'bar' and the previous one, even though they come
+    // from different lexical scopes.
+    int bar(int); // expected-note {{previous declaration is here}}
+    int baz(int); // expected-note {{previous declaration is here}}
 
     {
       int foo;
+      int bar;
+      int baz;
       {
-        // FIXME: should diagnose this because it's incompatible with
+        float foo(int); // expected-error {{functions that differ only in their return type cannot be overloaded}}
-        // N::foo. However, name lookup isn't properly "skipping" the
+        float bar(int); // expected-error {{functions that differ only in their return type cannot be overloaded}}
-        // "int foo" above.
+        float baz(int); // expected-error {{functions that differ only in their return type cannot be overloaded}}
-        float foo(int); 
       }
     }
   }