[Modules] Fix an issue where the reconstructed redeclaration chain was incomplete, missing the definition from a module.

-Make sure that a deserialized external decl gets added to the TU scope.
-When associating an identifier with a set of decls, use the most recent local ones,
  if they exist, otherwise associating decls from modules (that came after a local one)
  will lead to an incomplete reconstructed re-declaration chain.

rdar://13712705

llvm-svn: 180634

clang/include/clang/Serialization/ASTReader.h

@@ -1106,6 +1106,8 @@ private:
 
   void finishPendingActions();
 
+  void pushExternalDeclIntoScope(NamedDecl *D, DeclarationName Name);
+
   void addPendingDeclContextInfo(Decl *D,
                                  serialization::GlobalDeclID SemaDC,
                                  serialization::GlobalDeclID LexicalDC) {

clang/lib/Serialization/ASTReader.cpp

@@ -6005,8 +6005,8 @@ void ASTReader::InitializeSema(Sema &S) {
   // Makes sure any declarations that were deserialized "too early"
   // still get added to the identifier's declaration chains.
   for (unsigned I = 0, N = PreloadedDecls.size(); I != N; ++I) {
-    NamedDecl *ND = cast<NamedDecl>(PreloadedDecls[I]->getMostRecentDecl());
-    SemaObj->pushExternalDeclIntoScope(ND, PreloadedDecls[I]->getDeclName());
+    pushExternalDeclIntoScope(PreloadedDecls[I],
+                              PreloadedDecls[I]->getDeclName());
   }
   PreloadedDecls.clear();
 
@@ -6420,8 +6420,7 @@ ASTReader::SetGloballyVisibleDecls(IdentifierInfo *II,
       // Introduce this declaration into the translation-unit scope
       // and add it to the declaration chain for this identifier, so
       // that (unqualified) name lookup will find it.
-      NamedDecl *ND = cast<NamedDecl>(D->getMostRecentDecl());
-      SemaObj->pushExternalDeclIntoScope(ND, II);
+      pushExternalDeclIntoScope(D, II);
     } else {
       // Queue this declaration so that it will be added to the
       // translation unit scope and identifier's declaration chain
@@ -7208,8 +7207,7 @@ void ASTReader::finishPendingActions() {
          TLD != TLDEnd; ++TLD) {
       IdentifierInfo *II = TLD->first;
       for (unsigned I = 0, N = TLD->second.size(); I != N; ++I) {
-        NamedDecl *ND = cast<NamedDecl>(TLD->second[I]->getMostRecentDecl());
-        SemaObj->pushExternalDeclIntoScope(ND, II);
+        pushExternalDeclIntoScope(cast<NamedDecl>(TLD->second[I]), II);
       }
     }
 
@@ -7349,6 +7347,21 @@ void ASTReader::FinishedDeserializing() {
   }
 }
 
+void ASTReader::pushExternalDeclIntoScope(NamedDecl *D, DeclarationName Name) {
+  D = cast<NamedDecl>(D->getMostRecentDecl());
+
+  if (SemaObj->IdResolver.tryAddTopLevelDecl(D, Name) && SemaObj->TUScope) {
+    SemaObj->TUScope->AddDecl(D);
+  } else if (SemaObj->TUScope) {
+    // Adding the decl to IdResolver may have failed because it was already in
+    // (even though it was not added in scope). If it is already in, make sure
+    // it gets in the scope as well.
+    if (std::find(SemaObj->IdResolver.begin(Name),
+                  SemaObj->IdResolver.end(), D) != SemaObj->IdResolver.end())
+      SemaObj->TUScope->AddDecl(D);
+  }
+}
+
 ASTReader::ASTReader(Preprocessor &PP, ASTContext &Context,
                      StringRef isysroot, bool DisableValidation,
                      bool AllowASTWithCompilerErrors, bool UseGlobalIndex)

clang/lib/Serialization/ASTWriter.cpp

@@ -3106,7 +3106,28 @@ public:
     for (SmallVector<Decl *, 16>::reverse_iterator D = Decls.rbegin(),
                                                 DEnd = Decls.rend();
          D != DEnd; ++D)
-      clang::io::Emit32(Out, Writer.getDeclID(*D));
+      clang::io::Emit32(Out, Writer.getDeclID(getMostRecentLocalDecl(*D)));
+  }
+
+  /// \brief Returns the most recent local decl or the given decl if there are
+  /// no local ones. The given decl is assumed to be the most recent one.
+  Decl *getMostRecentLocalDecl(Decl *Orig) {
+    // The only way a "from AST file" decl would be more recent from a local one
+    // is if it came from a module.
+    if (!PP.getLangOpts().Modules)
+      return Orig;
+
+    // Look for a local in the decl chain.
+    for (Decl *D = Orig; D; D = D->getPreviousDecl()) {
+      if (!D->isFromASTFile())
+        return D;
+      // If we come up a decl from a (chained-)PCH stop since we won't find a
+      // local one.
+      if (D->getOwningModuleID() == 0)
+        break;
+    }
+
+    return Orig;
   }
 };
 } // end anonymous namespace

clang/test/Modules/redecls/a.h

@@ -0,0 +1,3 @@
+@interface AA
+@end
+@class AA;

clang/test/Modules/redecls/b.h

@@ -0,0 +1 @@
+@class AA;

clang/test/Modules/redecls/main.m

@@ -0,0 +1,27 @@
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodule-name=a %S/module.map -fmodules-cache-path=%t.mcp
+// RUN: %clang_cc1 -fmodules -x objective-c -emit-module -fmodule-name=b %S/module.map -fmodules-cache-path=%t.mcp
+// RUN: %clang_cc1 -fmodules %s -emit-pch -o %t1.pch -fmodules-cache-path=%t.mcp
+// RUN: %clang_cc1 -fmodules %s -emit-pch -o %t2.pch -include-pch %t1.pch -fmodules-cache-path=%t.mcp
+// RUN: %clang_cc1 -fmodules %s -fsyntax-only -include-pch %t2.pch -fmodules-cache-path=%t.mcp -verify
+
+#ifndef HEADER1
+#define HEADER1
+
+@import a;
+
+#elif !defined(HEADER2)
+#define HEADER2
+
+@class AA;
+@import b;
+
+#else
+
+// rdar://13712705
+@interface SS : AA
+@end
+
+#warning parsed this
+#endif
+// expected-warning@-2{{parsed this}}

clang/test/Modules/redecls/module.map

@@ -0,0 +1,2 @@
+module a { header "a.h" }
+module b { header "b.h" }