When a module completes the definition of a class template specialization imported from another module, emit an update record, rather than using the broken decl rewriting mechanism. If multiple modules do this, merge the definitions together, much as we would if they were separate declarations.

llvm-svn: 206680

8 files changed, 477 insertions, 141 deletions

diff --git a/clang/lib/CodeGen/CGRecordLayout.h b/clang/lib/CodeGen/CGRecordLayout.h
index 0fc7b8acbd4..b45fee56ea0 100644
--- a/clang/lib/CodeGen/CGRecordLayout.h
+++ b/clang/lib/CodeGen/CGRecordLayout.h
@@ -183,6 +183,7 @@ public:
   /// \brief Return llvm::StructType element number that corresponds to the
   /// field FD.
   unsigned getLLVMFieldNo(const FieldDecl *FD) const {
+    FD = FD->getCanonicalDecl();
     assert(FieldInfo.count(FD) && "Invalid field for record!");
     return FieldInfo.lookup(FD);
   }
@@ -201,6 +202,7 @@ public:
 
   /// \brief Return the BitFieldInfo that corresponds to the field FD.
   const CGBitFieldInfo &getBitFieldInfo(const FieldDecl *FD) const {
+    FD = FD->getCanonicalDecl();
     assert(FD->isBitField() && "Invalid call for non-bit-field decl!");
     llvm::DenseMap<const FieldDecl *, CGBitFieldInfo>::const_iterator
       it = BitFields.find(FD);
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 75b4504ca62..b7ab169a64f 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -212,7 +212,7 @@ CGRecordLowering::CGRecordLowering(CodeGenTypes &Types, const RecordDecl *D)
 
 void CGRecordLowering::setBitFieldInfo(
     const FieldDecl *FD, CharUnits StartOffset, llvm::Type *StorageType) {
-  CGBitFieldInfo &Info = BitFields[FD];
+  CGBitFieldInfo &Info = BitFields[FD->getCanonicalDecl()];
   Info.IsSigned = FD->getType()->isSignedIntegerOrEnumerationType();
   Info.Offset = (unsigned)(getFieldBitOffset(FD) - Context.toBits(StartOffset));
   Info.Size = FD->getBitWidthValue(Context);
@@ -297,7 +297,7 @@ void CGRecordLowering::lowerUnion() {
         FieldType = getByteArrayType(LayoutSize);
       setBitFieldInfo(Field, CharUnits::Zero(), FieldType);
     }
-    Fields[Field] = 0;
+    Fields[Field->getCanonicalDecl()] = 0;
     llvm::Type *FieldType = getStorageType(Field);
     // Conditionally update our storage type if we've got a new "better" one.
     if (!StorageType ||
@@ -570,7 +570,7 @@ void CGRecordLowering::fillOutputFields() {
       FieldTypes.push_back(Member->Data);
     if (Member->Kind == MemberInfo::Field) {
       if (Member->FD)
-        Fields[Member->FD] = FieldTypes.size() - 1;
+        Fields[Member->FD->getCanonicalDecl()] = FieldTypes.size() - 1;
       // A field without storage must be a bitfield.
       if (!Member->Data)
         setBitFieldInfo(Member->FD, Member->Offset, FieldTypes.back());
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8f00f922110..96c63601148 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -3440,7 +3440,8 @@ static bool CollectFieldInitializer(Sema &SemaRef, BaseAndFieldInfo &Info,
     return false;
 
   // Overwhelmingly common case: we have a direct initializer for this field.
-  if (CXXCtorInitializer *Init = Info.AllBaseFields.lookup(Field))
+  if (CXXCtorInitializer *Init =
+          Info.AllBaseFields.lookup(Field->getCanonicalDecl()))
     return Info.addFieldInitializer(Init);
 
   // C++11 [class.base.init]p8:
@@ -3553,7 +3554,7 @@ bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, bool AnyErrors,
     if (Member->isBaseInitializer())
       Info.AllBaseFields[Member->getBaseClass()->getAs<RecordType>()] = Member;
     else {
-      Info.AllBaseFields[Member->getAnyMember()] = Member;
+      Info.AllBaseFields[Member->getAnyMember()->getCanonicalDecl()] = Member;
 
       if (IndirectFieldDecl *F = Member->getIndirectMember()) {
         for (auto *C : F->chain()) {
@@ -3701,7 +3702,7 @@ static void PopulateKeysForFields(FieldDecl *Field, SmallVectorImpl<const void*>
       return;
     }
   }
-  IdealInits.push_back(Field);
+  IdealInits.push_back(Field->getCanonicalDecl());
 }
 
 static const void *GetKeyForBase(ASTContext &Context, QualType BaseType) {
@@ -3713,7 +3714,7 @@ static const void *GetKeyForMember(ASTContext &Context,
   if (!Member->isAnyMemberInitializer())
     return GetKeyForBase(Context, QualType(Member->getBaseClass(), 0));
     
-  return Member->getAnyMember();
+  return Member->getAnyMember()->getCanonicalDecl();
 }
 
 static void DiagnoseBaseOrMemInitializerOrder(
@@ -3908,13 +3909,12 @@ void Sema::ActOnMemInitializers(Decl *ConstructorDecl,
     Init->setSourceOrder(i);
 
     if (Init->isAnyMemberInitializer()) {
-      FieldDecl *Field = Init->getAnyMember();
-      if (CheckRedundantInit(*this, Init, Members[Field]) ||
+      const void *Key = GetKeyForMember(Context, Init);
+      if (CheckRedundantInit(*this, Init, Members[Key]) ||
           CheckRedundantUnionInit(*this, Init, MemberUnions))
         HadError = true;
     } else if (Init->isBaseInitializer()) {
-      const void *Key =
-          GetKeyForBase(Context, QualType(Init->getBaseClass(), 0));
+      const void *Key = GetKeyForMember(Context, Init);
       if (CheckRedundantInit(*this, Init, Members[Key]))
         HadError = true;
     } else {
diff --git a/clang/lib/Serialization/ASTCommon.h b/clang/lib/Serialization/ASTCommon.h
index 524a9c2fdcd..c7669749260 100644
--- a/clang/lib/Serialization/ASTCommon.h
+++ b/clang/lib/Serialization/ASTCommon.h
@@ -27,6 +27,7 @@ enum DeclUpdateKind {
   UPD_CXX_ADDED_ANONYMOUS_NAMESPACE,
   UPD_CXX_INSTANTIATED_STATIC_DATA_MEMBER,
   UPD_CXX_INSTANTIATED_FUNCTION_DEFINITION,
+  UPD_CXX_INSTANTIATED_CLASS_DEFINITION,
   UPD_CXX_RESOLVED_EXCEPTION_SPEC,
   UPD_CXX_DEDUCED_RETURN_TYPE,
   UPD_DECL_MARKED_USED,
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 78bf151c63f..14075462bec 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2562,14 +2562,11 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
               (const unsigned char *)Blob.data() + sizeof(uint32_t),
               (const unsigned char *)Blob.data(),
               ASTDeclContextNameLookupTrait(*this, F));
-      if (ID == PREDEF_DECL_TRANSLATION_UNIT_ID) { // Is it the TU?
-        DeclContext *TU = Context.getTranslationUnitDecl();
-        F.DeclContextInfos[TU].NameLookupTableData = Table;
-        TU->setHasExternalVisibleStorage(true);
-      } else if (Decl *D = DeclsLoaded[ID - NUM_PREDEF_DECL_IDS]) {
+      if (Decl *D = GetExistingDecl(ID)) {
         auto *DC = cast<DeclContext>(D);
         DC->getPrimaryContext()->setHasExternalVisibleStorage(true);
         auto *&LookupTable = F.DeclContextInfos[DC].NameLookupTableData;
+        // FIXME: There should never be an existing lookup table.
         delete LookupTable;
         LookupTable = Table;
       } else
@@ -2965,10 +2962,15 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) {
         Error("invalid DECL_UPDATE_OFFSETS block in AST file");
         return Failure;
       }
-      // FIXME: If we've already loaded the decl, perform the updates now.
-      for (unsigned I = 0, N = Record.size(); I != N; I += 2)
-        DeclUpdateOffsets[getGlobalDeclID(F, Record[I])]
-          .push_back(std::make_pair(&F, Record[I+1]));
+      for (unsigned I = 0, N = Record.size(); I != N; I += 2) {
+        GlobalDeclID ID = getGlobalDeclID(F, Record[I]);
+        DeclUpdateOffsets[ID].push_back(std::make_pair(&F, Record[I + 1]));
+
+        // If we've already loaded the decl, perform the updates when we finish
+        // loading this block.
+        if (Decl *D = GetExistingDecl(ID))
+          PendingUpdateRecords.push_back(std::make_pair(ID, D));
+      }
       break;
     }
 
@@ -3626,11 +3628,24 @@ void ASTReader::InitializeContext() {
     DeserializationListener->DeclRead(PREDEF_DECL_TRANSLATION_UNIT_ID, 
                                       Context.getTranslationUnitDecl());
 
-  // Make sure we load the declaration update records for the translation unit,
-  // if there are any.
-  loadDeclUpdateRecords(PREDEF_DECL_TRANSLATION_UNIT_ID, 
-                        Context.getTranslationUnitDecl());
-  
+  // For any declarations we have already loaded, load any update records.
+  {
+    // We're not back to a consistent state until all our pending update
+    // records have been loaded. There can be interdependencies between them.
+    Deserializing SomeUpdateRecords(this);
+    ReadingKindTracker ReadingKind(Read_Decl, *this);
+
+    // Make sure we load the declaration update records for the translation
+    // unit, if there are any.
+    // FIXME: Is this necessary any more?
+    loadDeclUpdateRecords(PREDEF_DECL_TRANSLATION_UNIT_ID,
+                          Context.getTranslationUnitDecl());
+
+    for (auto &Update : PendingUpdateRecords)
+      loadDeclUpdateRecords(Update.first, Update.second);
+    PendingUpdateRecords.clear();
+  }
+
   // FIXME: Find a better way to deal with collisions between these
   // built-in types. Right now, we just ignore the problem.
   
@@ -5844,11 +5859,14 @@ Decl *ASTReader::GetExternalDecl(uint32_t ID) {
   return GetDecl(ID);
 }
 
-uint64_t ASTReader::readCXXBaseSpecifiers(ModuleFile &M, const RecordData &Record, 
-                                          unsigned &Idx){
-  if (Idx >= Record.size())
+uint64_t ASTReader::readCXXBaseSpecifiers(ModuleFile &M,
+                                          const RecordData &Record,
+                                          unsigned &Idx) {
+  if (Idx >= Record.size() || Record[Idx] > M.LocalNumCXXBaseSpecifiers) {
+    Error("malformed AST file: missing C++ base specifier");
     return 0;
-  
+  }
+
   unsigned LocalID = Record[Idx++];
   return getGlobalBitOffset(M, M.CXXBaseSpecifiersOffsets[LocalID - 1]);
 }
@@ -5863,7 +5881,7 @@ CXXBaseSpecifier *ASTReader::GetExternalCXXBaseSpecifiers(uint64_t Offset) {
   unsigned Code = Cursor.ReadCode();
   unsigned RecCode = Cursor.readRecord(Code, Record);
   if (RecCode != DECL_CXX_BASE_SPECIFIERS) {
-    Error("Malformed AST file: missing C++ base specifiers");
+    Error("malformed AST file: missing C++ base specifiers");
     return 0;
   }
 
@@ -5906,14 +5924,14 @@ ModuleFile *ASTReader::getOwningModuleFile(const Decl *D) {
 SourceLocation ASTReader::getSourceLocationForDeclID(GlobalDeclID ID) {
   if (ID < NUM_PREDEF_DECL_IDS)
     return SourceLocation();
-  
+
   unsigned Index = ID - NUM_PREDEF_DECL_IDS;
 
   if (Index > DeclsLoaded.size()) {
     Error("declaration ID out-of-range for AST file");
     return SourceLocation();
   }
-  
+
   if (Decl *D = DeclsLoaded[Index])
     return D->getLocation();
 
@@ -5922,15 +5940,15 @@ SourceLocation ASTReader::getSourceLocationForDeclID(GlobalDeclID ID) {
   return ReadSourceLocation(*Rec.F, RawLocation);
 }
 
-Decl *ASTReader::GetDecl(DeclID ID) {
-  if (ID < NUM_PREDEF_DECL_IDS) {    
+Decl *ASTReader::GetExistingDecl(DeclID ID) {
+  if (ID < NUM_PREDEF_DECL_IDS) {
     switch ((PredefinedDeclIDs)ID) {
     case PREDEF_DECL_NULL_ID:
       return 0;
-        
+
     case PREDEF_DECL_TRANSLATION_UNIT_ID:
       return Context.getTranslationUnitDecl();
-        
+
     case PREDEF_DECL_OBJC_ID_ID:
       return Context.getObjCIdDecl();
 
@@ -5939,16 +5957,16 @@ Decl *ASTReader::GetDecl(DeclID ID) {
 
     case PREDEF_DECL_OBJC_CLASS_ID:
       return Context.getObjCClassDecl();
-        
+
     case PREDEF_DECL_OBJC_PROTOCOL_ID:
       return Context.getObjCProtocolDecl();
-        
+
     case PREDEF_DECL_INT_128_ID:
       return Context.getInt128Decl();
 
     case PREDEF_DECL_UNSIGNED_INT_128_ID:
       return Context.getUInt128Decl();
-        
+
     case PREDEF_DECL_OBJC_INSTANCETYPE_ID:
       return Context.getObjCInstanceTypeDecl();
 
@@ -5956,7 +5974,7 @@ Decl *ASTReader::GetDecl(DeclID ID) {
       return Context.getBuiltinVaListDecl();
     }
   }
-  
+
   unsigned Index = ID - NUM_PREDEF_DECL_IDS;
 
   if (Index >= DeclsLoaded.size()) {
@@ -5964,7 +5982,22 @@ Decl *ASTReader::GetDecl(DeclID ID) {
     Error("declaration ID out-of-range for AST file");
     return 0;
   }
-  
+
+  return DeclsLoaded[Index];
+}
+
+Decl *ASTReader::GetDecl(DeclID ID) {
+  if (ID < NUM_PREDEF_DECL_IDS)
+    return GetExistingDecl(ID);
+
+  unsigned Index = ID - NUM_PREDEF_DECL_IDS;
+
+  if (Index >= DeclsLoaded.size()) {
+    assert(0 && "declaration ID out-of-range for AST file");
+    Error("declaration ID out-of-range for AST file");
+    return 0;
+  }
+
   if (!DeclsLoaded[Index]) {
     ReadDeclRecord(ID);
     if (DeserializationListener)
@@ -6270,14 +6303,19 @@ ASTReader::FindExternalVisibleDeclsByName(const DeclContext *DC,
   Contexts.push_back(DC);
   
   if (DC->isNamespace()) {
-    MergedDeclsMap::iterator Merged
-      = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
+    auto Merged = MergedDecls.find(const_cast<Decl *>(cast<Decl>(DC)));
     if (Merged != MergedDecls.end()) {
       for (unsigned I = 0, N = Merged->second.size(); I != N; ++I)
         Contexts.push_back(cast<DeclContext>(GetDecl(Merged->second[I])));
     }
   }
-  
+  if (isa<CXXRecordDecl>(DC)) {
+    auto Merged = MergedLookups.find(DC);
+    if (Merged != MergedLookups.end())
+      Contexts.insert(Contexts.end(), Merged->second.begin(),
+                      Merged->second.end());
+  }
+
   DeclContextNameLookupVisitor Visitor(*this, Contexts, Name, Decls);
 
   // If we can definitively determine which module file to look into,
@@ -7820,6 +7858,19 @@ void ASTReader::ReadComments() {
   }
 }
 
+std::string ASTReader::getOwningModuleNameForDiagnostic(const Decl *D) {
+  // If we know the owning module, use it.
+  if (Module *M = D->getOwningModule())
+    return M->getFullModuleName();
+
+  // Otherwise, use the name of the top-level module the decl is within.
+  if (ModuleFile *M = getOwningModuleFile(D))
+    return M->ModuleName;
+
+  // Not from a module.
+  return "";
+}
+
 void ASTReader::finishPendingActions() {
   while (!PendingIdentifierInfos.empty() || !PendingDeclChains.empty() ||
          !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
@@ -7887,6 +7938,20 @@ void ASTReader::finishPendingActions() {
       Info.D->setDeclContextsImpl(SemaDC, LexicalDC, getContext());
     }
 
+    // Trigger the import of the full definition of each class that had any
+    // odr-merging problems, so we can produce better diagnostics for them.
+    for (auto &Merge : PendingOdrMergeFailures) {
+      Merge.first->buildLookup();
+      Merge.first->decls_begin();
+      Merge.first->bases_begin();
+      Merge.first->vbases_begin();
+      for (auto *RD : Merge.second) {
+        RD->decls_begin();
+        RD->bases_begin();
+        RD->vbases_begin();
+      }
+    }
+
     // For each declaration from a merged context, check that the canonical
     // definition of that context also contains a declaration of the same
     // entity.
@@ -7925,11 +7990,11 @@ void ASTReader::finishPendingActions() {
       if (!Found) {
         D->setInvalidDecl();
 
-        Module *CanonDefModule = cast<Decl>(CanonDef)->getOwningModule();
+        std::string CanonDefModule =
+            getOwningModuleNameForDiagnostic(cast<Decl>(CanonDef));
         Diag(D->getLocation(), diag::err_module_odr_violation_missing_decl)
-          << D << D->getOwningModule()->getFullModuleName()
-          << CanonDef << !CanonDefModule
-          << (CanonDefModule ? CanonDefModule->getFullModuleName() : "");
+          << D << getOwningModuleNameForDiagnostic(D)
+          << CanonDef << CanonDefModule.empty() << CanonDefModule;
 
         if (Candidates.empty())
           Diag(cast<Decl>(CanonDef)->getLocation(),
@@ -7940,6 +8005,8 @@ void ASTReader::finishPendingActions() {
                  diag::note_module_odr_violation_possible_decl)
               << Candidates[I];
         }
+
+        DiagnosedOdrMergeFailures.insert(CanonDef);
       }
     }
   }
@@ -8009,6 +8076,46 @@ void ASTReader::finishPendingActions() {
       MD->setLazyBody(PB->second);
   }
   PendingBodies.clear();
+
+  // Issue any pending ODR-failure diagnostics.
+  for (auto &Merge : PendingOdrMergeFailures) {
+    if (!DiagnosedOdrMergeFailures.insert(Merge.first))
+      continue;
+
+    bool Diagnosed = false;
+    for (auto *RD : Merge.second) {
+      // Multiple different declarations got merged together; tell the user
+      // where they came from.
+      if (Merge.first != RD) {
+        // FIXME: Walk the definition, figure out what's different,
+        // and diagnose that.
+        if (!Diagnosed) {
+          std::string Module = getOwningModuleNameForDiagnostic(Merge.first);
+          Diag(Merge.first->getLocation(),
+               diag::err_module_odr_violation_different_definitions)
+            << Merge.first << Module.empty() << Module;
+          Diagnosed = true;
+        }
+
+        Diag(RD->getLocation(),
+             diag::note_module_odr_violation_different_definitions)
+          << getOwningModuleNameForDiagnostic(RD);
+      }
+    }
+
+    if (!Diagnosed) {
+      // All definitions are updates to the same declaration. This happens if a
+      // module instantiates the declaration of a class template specialization
+      // and two or more other modules instantiate its definition.
+      //
+      // FIXME: Indicate which modules had instantiations of this definition.
+      // FIXME: How can this even happen?
+      Diag(Merge.first->getLocation(),
+           diag::err_module_odr_violation_different_instantiations)
+        << Merge.first;
+    }
+  }
+  PendingOdrMergeFailures.clear();
 }
 
 void ASTReader::FinishedDeserializing() {
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 4d68c0db177..4823cbcdfd9 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -99,9 +99,12 @@ namespace clang {
     Module *readModule(const RecordData &R, unsigned &I) {
       return Reader.getSubmodule(readSubmoduleID(R, I));
     }
-    
+
+    void ReadCXXRecordDefinition(CXXRecordDecl *D);
     void ReadCXXDefinitionData(struct CXXRecordDecl::DefinitionData &Data,
                                const RecordData &R, unsigned &I);
+    void MergeDefinitionData(CXXRecordDecl *D,
+                             struct CXXRecordDecl::DefinitionData &NewDD);
 
     /// \brief RAII class used to capture the first ID within a redeclaration
     /// chain and to introduce it into the list of pending redeclaration chains
@@ -474,7 +477,8 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitTagDecl(TagDecl *TD) {
   } else
     TD->NamedDeclOrQualifier = ReadDeclAs<NamedDecl>(Record, Idx);
 
-  mergeRedeclarable(TD, Redecl);
+  if (!isa<CXXRecordDecl>(TD))
+    mergeRedeclarable(TD, Redecl);
   return Redecl;
 }
 
@@ -576,9 +580,10 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
 
   switch ((FunctionDecl::TemplatedKind)Record[Idx++]) {
   case FunctionDecl::TK_NonTemplate:
-    mergeRedeclarable(FD, Redecl);      
+    mergeRedeclarable(FD, Redecl);
     break;
   case FunctionDecl::TK_FunctionTemplate:
+    // Merged when we merge the template.
     FD->setDescribedFunctionTemplate(ReadDeclAs<FunctionTemplateDecl>(Record, 
                                                                       Idx));
     break;
@@ -588,6 +593,7 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
     SourceLocation POI = ReadSourceLocation(Record, Idx);
     FD->setInstantiationOfMemberFunction(Reader.getContext(), InstFD, TSK);
     FD->getMemberSpecializationInfo()->setPointOfInstantiation(POI);
+    mergeRedeclarable(FD, Redecl);
     break;
   }
   case FunctionDecl::TK_FunctionTemplateSpecialization: {
@@ -673,6 +679,8 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
     
     FD->setDependentTemplateSpecialization(Reader.getContext(),
                                            TemplDecls, TemplArgs);
+
+    // FIXME: Merging.
     break;
   }
   }
@@ -1259,57 +1267,152 @@ void ASTDeclReader::ReadCXXDefinitionData(
   }
 }
 
+void ASTDeclReader::MergeDefinitionData(
+    CXXRecordDecl *D, struct CXXRecordDecl::DefinitionData &MergeDD) {
+  assert(D->DefinitionData && "merging class definition into non-definition");
+  auto &DD = *D->DefinitionData;
+
+  // If the new definition has new special members, let the name lookup
+  // code know that it needs to look in the new definition too.
+  if ((MergeDD.DeclaredSpecialMembers & ~DD.DeclaredSpecialMembers) &&
+      DD.Definition != MergeDD.Definition) {
+    Reader.MergedLookups[DD.Definition].push_back(MergeDD.Definition);
+    DD.Definition->setHasExternalVisibleStorage();
+  }
+
+  // FIXME: Move this out into a .def file?
+  // FIXME: Issue a diagnostic on a mismatched MATCH_FIELD, rather than
+  // asserting; this can happen in the case of an ODR violation.
+  bool DetectedOdrViolation = false;
+#define OR_FIELD(Field) DD.Field |= MergeDD.Field;
+#define MATCH_FIELD(Field) \
+    DetectedOdrViolation |= DD.Field != MergeDD.Field; \
+    OR_FIELD(Field)
+  MATCH_FIELD(UserDeclaredConstructor)
+  MATCH_FIELD(UserDeclaredSpecialMembers)
+  MATCH_FIELD(Aggregate)
+  MATCH_FIELD(PlainOldData)
+  MATCH_FIELD(Empty)
+  MATCH_FIELD(Polymorphic)
+  MATCH_FIELD(Abstract)
+  MATCH_FIELD(IsStandardLayout)
+  MATCH_FIELD(HasNoNonEmptyBases)
+  MATCH_FIELD(HasPrivateFields)
+  MATCH_FIELD(HasProtectedFields)
+  MATCH_FIELD(HasPublicFields)
+  MATCH_FIELD(HasMutableFields)
+  MATCH_FIELD(HasVariantMembers)
+  MATCH_FIELD(HasOnlyCMembers)
+  MATCH_FIELD(HasInClassInitializer)
+  MATCH_FIELD(HasUninitializedReferenceMember)
+  MATCH_FIELD(NeedOverloadResolutionForMoveConstructor)
+  MATCH_FIELD(NeedOverloadResolutionForMoveAssignment)
+  MATCH_FIELD(NeedOverloadResolutionForDestructor)
+  MATCH_FIELD(DefaultedMoveConstructorIsDeleted)
+  MATCH_FIELD(DefaultedMoveAssignmentIsDeleted)
+  MATCH_FIELD(DefaultedDestructorIsDeleted)
+  OR_FIELD(HasTrivialSpecialMembers)
+  OR_FIELD(DeclaredNonTrivialSpecialMembers)
+  MATCH_FIELD(HasIrrelevantDestructor)
+  OR_FIELD(HasConstexprNonCopyMoveConstructor)
+  MATCH_FIELD(DefaultedDefaultConstructorIsConstexpr)
+  OR_FIELD(HasConstexprDefaultConstructor)
+  MATCH_FIELD(HasNonLiteralTypeFieldsOrBases)
+  // ComputedVisibleConversions is handled below.
+  MATCH_FIELD(UserProvidedDefaultConstructor)
+  OR_FIELD(DeclaredSpecialMembers)
+  MATCH_FIELD(ImplicitCopyConstructorHasConstParam)
+  MATCH_FIELD(ImplicitCopyAssignmentHasConstParam)
+  OR_FIELD(HasDeclaredCopyConstructorWithConstParam)
+  OR_FIELD(HasDeclaredCopyAssignmentWithConstParam)
+  MATCH_FIELD(IsLambda)
+#undef OR_FIELD
+#undef MATCH_FIELD
+
+  if (DD.NumBases != MergeDD.NumBases || DD.NumVBases != MergeDD.NumVBases)
+    DetectedOdrViolation = true;
+  // FIXME: Issue a diagnostic if the base classes don't match when we come
+  // to lazily load them.
+
+  // FIXME: Issue a diagnostic if the list of conversion functions doesn't
+  // match when we come to lazily load them.
+  if (MergeDD.ComputedVisibleConversions && !DD.ComputedVisibleConversions) {
+    DD.VisibleConversions = std::move(MergeDD.VisibleConversions);
+    DD.ComputedVisibleConversions = true;
+  }
+
+  // FIXME: Issue a diagnostic if FirstFriend doesn't match when we come to
+  // lazily load it.
+
+  if (DD.IsLambda) {
+    // FIXME: ODR-checking for merging lambdas (this happens, for instance,
+    // when they occur within the body of a function template specialization).
+  }
+
+  if (DetectedOdrViolation)
+    Reader.PendingOdrMergeFailures[DD.Definition].push_back(MergeDD.Definition);
+}
+
+void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D) {
+  struct CXXRecordDecl::DefinitionData *DD;
+  ASTContext &C = Reader.getContext();
+
+  // Determine whether this is a lambda closure type, so that we can
+  // allocate the appropriate DefinitionData structure.
+  bool IsLambda = Record[Idx++];
+  if (IsLambda)
+    DD = new (C) CXXRecordDecl::LambdaDefinitionData(D, 0, false, false,
+                                                     LCD_None);
+  else
+    DD = new (C) struct CXXRecordDecl::DefinitionData(D);
+
+  ReadCXXDefinitionData(*DD, Record, Idx);
+
+  // If we're reading an update record, we might already have a definition for
+  // this record. If so, just merge into it.
+  if (D->DefinitionData) {
+    MergeDefinitionData(D, *DD);
+    return;
+  }
+
+  // Propagate the DefinitionData pointer to the canonical declaration, so
+  // that all other deserialized declarations will see it.
+  CXXRecordDecl *Canon = D->getCanonicalDecl();
+  if (Canon == D) {
+    D->DefinitionData = DD;
+    D->IsCompleteDefinition = true;
+  } else if (!Canon->DefinitionData) {
+    Canon->DefinitionData = D->DefinitionData = DD;
+    D->IsCompleteDefinition = true;
+
+    // Note that we have deserialized a definition. Any declarations
+    // deserialized before this one will be be given the DefinitionData
+    // pointer at the end.
+    Reader.PendingDefinitions.insert(D);
+  } else {
+    // We have already deserialized a definition of this record. This
+    // definition is no longer really a definition. Note that the pre-existing
+    // definition is the *real* definition.
+    Reader.MergedDeclContexts.insert(
+        std::make_pair(D, Canon->DefinitionData->Definition));
+    D->DefinitionData = D->getCanonicalDecl()->DefinitionData;
+    D->IsCompleteDefinition = false;
+    MergeDefinitionData(D, *DD);
+  }
+}
+
 ASTDeclReader::RedeclarableResult
 ASTDeclReader::VisitCXXRecordDeclImpl(CXXRecordDecl *D) {
   RedeclarableResult Redecl = VisitRecordDeclImpl(D);
 
   ASTContext &C = Reader.getContext();
-  bool WasDefinition = Record[Idx++];
-  if (WasDefinition) {
-    // Determine whether this is a lambda closure type, so that we can
-    // allocate the appropriate DefinitionData structure.
-    bool IsLambda = Record[Idx++];
-    if (IsLambda)
-      D->DefinitionData = new (C) CXXRecordDecl::LambdaDefinitionData(D, 0,
-                                                                      false,
-                                                                      false, LCD_None);
-    else
-      D->DefinitionData = new (C) struct CXXRecordDecl::DefinitionData(D);
-
-    ReadCXXDefinitionData(*D->DefinitionData, Record, Idx);
-
-    // Propagate the DefinitionData pointer to the canonical declaration, so
-    // that all other deserialized declarations will see it.
-    CXXRecordDecl *Canon = D->getCanonicalDecl();
-    if (Canon == D) {
-      // Nothing to do.
-    } else if (!Canon->DefinitionData) {
-      Canon->DefinitionData = D->DefinitionData;
-
-      // Note that we have deserialized a definition. Any declarations
-      // deserialized before this one will be be given the DefinitionData
-      // pointer at the end.
-      Reader.PendingDefinitions.insert(D);
-    } else {
-      // We have already deserialized a definition of this record. This
-      // definition is no longer really a definition. Note that the pre-existing
-      // definition is the *real* definition.
-      // FIXME: Check DefinitionData for consistency with prior definition.
-      Reader.MergedDeclContexts.insert(
-          std::make_pair(D, D->getCanonicalDecl()->DefinitionData->Definition));
-      D->IsCompleteDefinition = false;
-      D->DefinitionData = D->getCanonicalDecl()->DefinitionData;
-    }
-  } else {
-    // Propagate DefinitionData pointer from the canonical declaration.
-    D->DefinitionData = D->getCanonicalDecl()->DefinitionData;
-  }
 
   enum CXXRecKind {
     CXXRecNotTemplate = 0, CXXRecTemplate, CXXRecMemberSpecialization
   };
   switch ((CXXRecKind)Record[Idx++]) {
   case CXXRecNotTemplate:
+    mergeRedeclarable(D, Redecl);
     break;
   case CXXRecTemplate:
     D->TemplateOrInstantiation = ReadDeclAs<ClassTemplateDecl>(Record, Idx);
@@ -1321,10 +1424,18 @@ ASTDeclReader::VisitCXXRecordDeclImpl(CXXRecordDecl *D) {
     MemberSpecializationInfo *MSI = new (C) MemberSpecializationInfo(RD, TSK);
     MSI->setPointOfInstantiation(POI);
     D->TemplateOrInstantiation = MSI;
+    mergeRedeclarable(D, Redecl);
     break;
   }
   }
 
+  bool WasDefinition = Record[Idx++];
+  if (WasDefinition)
+    ReadCXXRecordDefinition(D);
+  else
+    // Propagate DefinitionData pointer from the canonical declaration.
+    D->DefinitionData = D->getCanonicalDecl()->DefinitionData;
+
   // Lazily load the key function to avoid deserializing every method so we can
   // compute it.
   if (WasDefinition) {
@@ -1353,6 +1464,7 @@ void ASTDeclReader::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
   if (auto *CD = ReadDeclAs<CXXConstructorDecl>(Record, Idx))
     D->setInheritedConstructor(CD);
   D->IsExplicitSpecified = Record[Idx++];
+  // FIXME: We should defer loading this until we need the constructor's body.
   std::tie(D->CtorInitializers, D->NumCtorInitializers) =
       Reader.ReadCXXCtorInitializers(F, Record, Idx);
 }
@@ -1590,7 +1702,7 @@ ASTDeclReader::VisitClassTemplateSpecializationDeclImpl(
           if (!CanonSpec->DefinitionData) {
             CanonSpec->DefinitionData = D->DefinitionData;
           } else {
-            // FIXME: Check DefinitionData for consistency with prior definition
+            MergeDefinitionData(CanonSpec, *D->DefinitionData);
             Reader.PendingDefinitions.erase(D);
             Reader.MergedDeclContexts.insert(
                 std::make_pair(D, CanonSpec->DefinitionData->Definition));
@@ -2150,9 +2262,7 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) {
   // Fields with the same name and the same type match.
   if (FieldDecl *FDX = dyn_cast<FieldDecl>(X)) {
     FieldDecl *FDY = cast<FieldDecl>(Y);
-    // FIXME: Diagnose if the types don't match. More generally, diagnose if we
-    // get a declaration in a class definition that isn't in the canonical class
-    // definition.
+    // FIXME: Diagnose if the types don't match.
     // FIXME: Also check the bitwidth is odr-equivalent, if any.
     return X->getASTContext().hasSameType(FDX->getType(), FDY->getType());
   }
@@ -2259,6 +2369,9 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) {
 
   // If this declaration is from a merged context, make a note that we need to
   // check that the canonical definition of that context contains the decl.
+  //
+  // FIXME: We should do something similar if we merge two definitions of the
+  // same template specialization into the same CXXRecordDecl.
   if (Reader.MergedDeclContexts.count(D->getLexicalDeclContext()))
     Reader.PendingOdrMergeChecks.push_back(D);
 
@@ -2923,12 +3036,14 @@ void ASTReader::loadObjCCategories(serialization::GlobalDeclID ID,
 
 void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile,
                                const RecordData &Record) {
-  unsigned Idx = 0;
   while (Idx < Record.size()) {
     switch ((DeclUpdateKind)Record[Idx++]) {
-    case UPD_CXX_ADDED_IMPLICIT_MEMBER:
-      cast<CXXRecordDecl>(D)->addedMember(Reader.ReadDecl(ModuleFile, Record, Idx));
+    case UPD_CXX_ADDED_IMPLICIT_MEMBER: {
+      Decl *MD = Reader.ReadDecl(ModuleFile, Record, Idx);
+      assert(MD && "couldn't read decl from update record");
+      cast<CXXRecordDecl>(D)->addedMember(MD);
       break;
+    }
 
     case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
       // It will be added to the template's specializations set when loaded.
@@ -2958,9 +3073,11 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile,
 
     case UPD_CXX_INSTANTIATED_FUNCTION_DEFINITION: {
       FunctionDecl *FD = cast<FunctionDecl>(D);
-      if (Reader.PendingBodies[FD])
+      if (Reader.PendingBodies[FD]) {
         // FIXME: Maybe check for ODR violations.
-        break;
+        // It's safe to stop now because this update record is always last.
+        return;
+      }
 
       if (Record[Idx++])
         FD->setImplicitlyInline();
@@ -2975,6 +3092,45 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile,
       break;
     }
 
+    case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: {
+      auto *RD = cast<CXXRecordDecl>(D);
+      bool HadDefinition = RD->getDefinition();
+      ReadCXXRecordDefinition(RD);
+      // Visible update is handled separately.
+      uint64_t LexicalOffset = Record[Idx++];
+      if (!HadDefinition && LexicalOffset) {
+        RD->setHasExternalLexicalStorage(true);
+        Reader.ReadDeclContextStorage(ModuleFile, ModuleFile.DeclsCursor,
+                                          std::make_pair(LexicalOffset, 0),
+                                          ModuleFile.DeclContextInfos[RD]);
+      }
+
+      auto TSK = (TemplateSpecializationKind)Record[Idx++];
+      SourceLocation POI = Reader.ReadSourceLocation(ModuleFile, Record, Idx);
+      if (MemberSpecializationInfo *MSInfo =
+              RD->getMemberSpecializationInfo()) {
+        MSInfo->setTemplateSpecializationKind(TSK);
+        MSInfo->setPointOfInstantiation(POI);
+      } else {
+        ClassTemplateSpecializationDecl *Spec =
+            cast<ClassTemplateSpecializationDecl>(RD);
+        Spec->setTemplateSpecializationKind(TSK);
+        Spec->setPointOfInstantiation(POI);
+      }
+
+      RD->setTagKind((TagTypeKind)Record[Idx++]);
+      RD->setLocation(Reader.ReadSourceLocation(ModuleFile, Record, Idx));
+      RD->setLocStart(Reader.ReadSourceLocation(ModuleFile, Record, Idx));
+      RD->setRBraceLoc(Reader.ReadSourceLocation(ModuleFile, Record, Idx));
+
+      if (Record[Idx++]) {
+        AttrVec Attrs;
+        Reader.ReadAttributes(F, Attrs, Record, Idx);
+        D->setAttrsImpl(Attrs, Reader.getContext());
+      }
+      break;
+    }
+
     case UPD_CXX_RESOLVED_EXCEPTION_SPEC: {
       auto *FD = cast<FunctionDecl>(D);
       auto *FPT = FD->getType()->castAs<FunctionProtoType>();
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index d770f8df26e..7402961ae83 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3471,11 +3471,54 @@ public:
 };
 } // end anonymous namespace
 
+template<typename Visitor>
+static void visitLocalLookupResults(const DeclContext *ConstDC,
+                                    bool NeedToReconcileExternalVisibleStorage,
+                                    Visitor AddLookupResult) {
+  // FIXME: We need to build the lookups table, which is logically const.
+  DeclContext *DC = const_cast<DeclContext*>(ConstDC);
+  assert(DC == DC->getPrimaryContext() && "only primary DC has lookup table");
+
+  SmallVector<DeclarationName, 16> ExternalNames;
+  for (auto &Lookup : *DC->buildLookup()) {
+    if (Lookup.second.hasExternalDecls() ||
+        NeedToReconcileExternalVisibleStorage) {
+      // We don't know for sure what declarations are found by this name,
+      // because the external source might have a different set from the set
+      // that are in the lookup map, and we can't update it now without
+      // risking invalidating our lookup iterator. So add it to a queue to
+      // deal with later.
+      ExternalNames.push_back(Lookup.first);
+      continue;
+    }
+
+    AddLookupResult(Lookup.first, Lookup.second.getLookupResult());
+  }
+
+  // Add the names we needed to defer. Note, this shouldn't add any new decls
+  // to the list we need to serialize: any new declarations we find here should
+  // be imported from an external source.
+  // FIXME: What if the external source isn't an ASTReader?
+  for (const auto &Name : ExternalNames)
+    AddLookupResult(Name, DC->lookup(Name));
+}
+
+void ASTWriter::AddUpdatedDeclContext(const DeclContext *DC) {
+  if (UpdatedDeclContexts.insert(DC) && WritingAST) {
+    // Ensure we emit all the visible declarations.
+    visitLocalLookupResults(DC, DC->NeedToReconcileExternalVisibleStorage,
+                            [&](DeclarationName Name,
+                                DeclContext::lookup_const_result Result) {
+      for (auto *Decl : Result)
+        GetDeclRef(Decl);
+    });
+  }
+}
+
 uint32_t
 ASTWriter::GenerateNameLookupTable(const DeclContext *DC,
                                    llvm::SmallVectorImpl<char> &LookupTable) {
   assert(!DC->LookupPtr.getInt() && "must call buildLookups first");
-  assert(DC == DC->getPrimaryContext() && "only primary DC has lookup table");
 
   llvm::OnDiskChainedHashTableGenerator<ASTDeclContextNameLookupTrait>
       Generator;
@@ -3487,8 +3530,9 @@ ASTWriter::GenerateNameLookupTable(const DeclContext *DC,
   SmallVector<NamedDecl *, 8> ConstructorDecls;
   SmallVector<NamedDecl *, 4> ConversionDecls;
 
-  auto AddLookupResult = [&](DeclarationName Name,
-                             DeclContext::lookup_result Result) {
+  visitLocalLookupResults(DC, DC->NeedToReconcileExternalVisibleStorage,
+                          [&](DeclarationName Name,
+                              DeclContext::lookup_result Result) {
     if (Result.empty())
       return;
 
@@ -3504,41 +3548,19 @@ ASTWriter::GenerateNameLookupTable(const DeclContext *DC,
         ConstructorName = Name;
       ConstructorDecls.append(Result.begin(), Result.end());
       return;
+
     case DeclarationName::CXXConversionFunctionName:
       if (!ConversionName)
         ConversionName = Name;
       ConversionDecls.append(Result.begin(), Result.end());
       return;
+
     default:
       break;
     }
 
     Generator.insert(Name, Result, Trait);
-  };
-
-  SmallVector<DeclarationName, 16> ExternalNames;
-  for (auto &Lookup : *DC->getLookupPtr()) {
-    if (Lookup.second.hasExternalDecls() ||
-        DC->NeedToReconcileExternalVisibleStorage) {
-      // We don't know for sure what declarations are found by this name,
-      // because the external source might have a different set from the set
-      // that are in the lookup map, and we can't update it now without
-      // risking invalidating our lookup iterator. So add it to a queue to
-      // deal with later.
-      ExternalNames.push_back(Lookup.first);
-      continue;
-    }
-
-    AddLookupResult(Lookup.first, Lookup.second.getLookupResult());
-  }
-
-  // Add the names we needed to defer. Note, this shouldn't add any new decls
-  // to the list we need to serialize: any new declarations we find here should
-  // be imported from an external source.
-  // FIXME: What if the external source isn't an ASTReader?
-  for (const auto &Name : ExternalNames)
-    // FIXME: const_cast since OnDiskHashTable wants a non-const lookup result.
-    AddLookupResult(Name, const_cast<DeclContext*>(DC)->lookup(Name));
+  });
 
   // Add the constructors.
   if (!ConstructorDecls.empty()) {
@@ -3547,6 +3569,7 @@ ASTWriter::GenerateNameLookupTable(const DeclContext *DC,
                                                 ConstructorDecls.end()),
                      Trait);
   }
+
   // Add the conversion functions.
   if (!ConversionDecls.empty()) {
     Generator.insert(ConversionName,
@@ -3792,7 +3815,7 @@ void ASTWriter::WriteMergedDecls() {
                                         IEnd = Chain->MergedDecls.end();
        I != IEnd; ++I) {
     DeclID CanonID = I->first->isFromASTFile()? I->first->getGlobalID()
-                                              : getDeclID(I->first);
+                                              : GetDeclRef(I->first);
     assert(CanonID && "Merged declaration not known?");
     
     Record.push_back(CanonID);
@@ -4029,6 +4052,15 @@ void ASTWriter::WriteASTCore(Sema &SemaRef,
     }
   }
 
+  // If we saw any DeclContext updates before we started writing the AST file,
+  // make sure all visible decls in those DeclContexts are written out.
+  if (!UpdatedDeclContexts.empty()) {
+    auto OldUpdatedDeclContexts = std::move(UpdatedDeclContexts);
+    UpdatedDeclContexts.clear();
+    for (auto *DC : OldUpdatedDeclContexts)
+      AddUpdatedDeclContext(DC);
+  }
+
   // Build a record containing all of the tentative definitions in this file, in
   // TentativeDefinitions order.  Generally, this record will be empty for
   // headers.
@@ -4388,11 +4420,8 @@ void ASTWriter::WriteASTCore(Sema &SemaRef,
     Stream.EmitRecord(UNDEFINED_BUT_USED, UndefinedButUsed);
   
   // Write the visible updates to DeclContexts.
-  for (llvm::SmallPtrSet<const DeclContext *, 16>::iterator
-       I = UpdatedDeclContexts.begin(),
-       E = UpdatedDeclContexts.end();
-       I != E; ++I)
-    WriteDeclContextVisibleUpdate(*I);
+  for (auto *DC : UpdatedDeclContexts)
+    WriteDeclContextVisibleUpdate(DC);
 
   if (!WritingModule) {
     // Write the submodules that were imported, if any.
@@ -4459,9 +4488,6 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
     if (isRewritten(D))
       continue; // The decl will be written completely,no need to store updates.
 
-    OffsetsRecord.push_back(GetDeclRef(D));
-    OffsetsRecord.push_back(Stream.GetCurrentBitNo());
-
     bool HasUpdatedBody = false;
     RecordData Record;
     for (auto &Update : DeclUpdate.second) {
@@ -4472,6 +4498,7 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
       case UPD_CXX_ADDED_IMPLICIT_MEMBER:
       case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
       case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE:
+        assert(Update.getDecl() && "no decl to add?");
         Record.push_back(GetDeclRef(Update.getDecl()));
         break;
 
@@ -4486,6 +4513,39 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
         HasUpdatedBody = true;
         break;
 
+      case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: {
+        auto *RD = cast<CXXRecordDecl>(D);
+        AddUpdatedDeclContext(RD->getPrimaryContext());
+        AddCXXDefinitionData(RD, Record);
+        Record.push_back(WriteDeclContextLexicalBlock(
+            *Context, const_cast<CXXRecordDecl *>(RD)));
+
+        // This state is sometimes updated by template instantiation, when we
+        // switch from the specialization referring to the template declaration
+        // to it referring to the template definition.
+        if (auto *MSInfo = RD->getMemberSpecializationInfo()) {
+          Record.push_back(MSInfo->getTemplateSpecializationKind());
+          AddSourceLocation(MSInfo->getPointOfInstantiation(), Record);
+        } else {
+          auto *Spec = cast<ClassTemplateSpecializationDecl>(RD);
+          Record.push_back(Spec->getTemplateSpecializationKind());
+          AddSourceLocation(Spec->getPointOfInstantiation(), Record);
+        }
+        Record.push_back(RD->getTagKind());
+        AddSourceLocation(RD->getLocation(), Record);
+        AddSourceLocation(RD->getLocStart(), Record);
+        AddSourceLocation(RD->getRBraceLoc(), Record);
+
+        // Instantiation may change attributes; write them all out afresh.
+        Record.push_back(D->hasAttrs());
+        if (Record.back())
+          WriteAttributes(ArrayRef<const Attr*>(D->getAttrs().begin(),
+                                                D->getAttrs().size()), Record);
+
+        // FIXME: Ensure we don't get here for explicit instantiations.
+        break;
+      }
+
       case UPD_CXX_RESOLVED_EXCEPTION_SPEC:
         addExceptionSpec(
             *this,
@@ -4515,10 +4575,16 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
       AddFunctionDefinition(Def, Record);
     }
 
+    OffsetsRecord.push_back(GetDeclRef(D));
+    OffsetsRecord.push_back(Stream.GetCurrentBitNo());
+
     Stream.EmitRecord(DECL_UPDATES, Record);
 
     // Flush any statements that were written as part of this update record.
     FlushStmts();
+
+    // Flush C++ base specifiers, if there are any.
+    FlushCXXBaseSpecifiers();
   }
 }
 
@@ -5405,7 +5471,10 @@ void ASTWriter::CompletedTagDefinition(const TagDecl *D) {
       // A forward reference was mutated into a definition. Rewrite it.
       // FIXME: This happens during template instantiation, should we
       // have created a new definition decl instead ?
-      RewriteDecl(RD);
+      assert(isTemplateInstantiation(RD->getTemplateSpecializationKind()) &&
+             "completed a tag from another module but not by instantiation?");
+      DeclUpdates[RD].push_back(
+          DeclUpdate(UPD_CXX_INSTANTIATED_CLASS_DEFINITION));
     }
   }
 }
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 14304ab6e1a..6841a9224a9 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -984,9 +984,6 @@ void ASTDeclWriter::VisitUnresolvedUsingTypenameDecl(
 
 void ASTDeclWriter::VisitCXXRecordDecl(CXXRecordDecl *D) {
   VisitRecordDecl(D);
-  Record.push_back(D->isThisDeclarationADefinition());
-  if (D->isThisDeclarationADefinition())
-    Writer.AddCXXDefinitionData(D, Record);
 
   enum {
     CXXRecNotTemplate = 0, CXXRecTemplate, CXXRecMemberSpecialization
@@ -1004,6 +1001,10 @@ void ASTDeclWriter::VisitCXXRecordDecl(CXXRecordDecl *D) {
     Record.push_back(CXXRecNotTemplate);
   }
 
+  Record.push_back(D->isThisDeclarationADefinition());
+  if (D->isThisDeclarationADefinition())
+    Writer.AddCXXDefinitionData(D, Record);
+
   // Store (what we currently believe to be) the key function to avoid
   // deserializing every method so we can compute it.
   if (D->IsCompleteDefinition)