From: Richard Smith Date: Tue, 3 Feb 2015 03:32:14 +0000 (+0000) Subject: [modules] Be sure to load the lexical definition of a class template X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b64063c8c6b5d0530904664cfc5991a01a939aea;p=clang [modules] Be sure to load the lexical definition of a class template specialization from an update record exactly once, even if we needed to fake up the definition. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@227939 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index 8dd9b145c0..227e617948 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/include/clang/Serialization/ASTReader.h @@ -435,9 +435,11 @@ private: llvm::SmallVector, 16> PendingUpdateRecords; + enum class PendingFakeDefinitionKind { NotFake, Fake, FakeLoaded }; + /// \brief The DefinitionData pointers that we faked up for class definitions /// that we needed but hadn't loaded yet. - llvm::SmallPtrSet PendingFakeDefinitionData; + llvm::DenseMap PendingFakeDefinitionData; struct ReplacedDeclInfo { ModuleFile *Mod; diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index 754b0a0a53..7f666a4607 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -103,7 +103,7 @@ namespace clang { return Reader.getSubmodule(readSubmoduleID(R, I)); } - void ReadCXXRecordDefinition(CXXRecordDecl *D); + void ReadCXXRecordDefinition(CXXRecordDecl *D, bool Update); void ReadCXXDefinitionData(struct CXXRecordDecl::DefinitionData &Data, const RecordData &R, unsigned &I); void MergeDefinitionData(CXXRecordDecl *D, @@ -1360,11 +1360,13 @@ void ASTDeclReader::MergeDefinitionData( "merging class definition into non-definition"); auto &DD = *D->DefinitionData.getNotUpdated(); - if (Reader.PendingFakeDefinitionData.count(&DD)) { + auto PFDI = Reader.PendingFakeDefinitionData.find(&DD); + if (PFDI != Reader.PendingFakeDefinitionData.end() && + PFDI->second == ASTReader::PendingFakeDefinitionKind::Fake) { // We faked up this definition data because we found a class for which we'd // not yet loaded the definition. Replace it with the real thing now. - Reader.PendingFakeDefinitionData.erase(&DD); assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?"); + PFDI->second = ASTReader::PendingFakeDefinitionKind::FakeLoaded; // Don't change which declaration is the definition; that is required // to be invariant once we select it. @@ -1458,7 +1460,7 @@ void ASTDeclReader::MergeDefinitionData( Reader.PendingOdrMergeFailures[DD.Definition].push_back(MergeDD.Definition); } -void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D) { +void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D, bool Update) { struct CXXRecordDecl::DefinitionData *DD; ASTContext &C = Reader.getContext(); @@ -1491,6 +1493,11 @@ void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D) { if (Canon == D) { D->DefinitionData = DD; D->IsCompleteDefinition = true; + + // If this is an update record, we can have redeclarations already. Make a + // note that we need to propagate the DefinitionData pointer onto them. + if (Update) + Reader.PendingDefinitions.insert(D); } else if (auto *CanonDD = Canon->DefinitionData.getNotUpdated()) { // We have already deserialized a definition of this record. This // definition is no longer really a definition. Note that the pre-existing @@ -1556,7 +1563,7 @@ ASTDeclReader::VisitCXXRecordDeclImpl(CXXRecordDecl *D) { bool WasDefinition = Record[Idx++]; if (WasDefinition) - ReadCXXRecordDefinition(D); + ReadCXXRecordDefinition(D, /*Update*/false); else // Propagate DefinitionData pointer from the canonical declaration. D->DefinitionData = D->getCanonicalDecl()->DefinitionData; @@ -2573,7 +2580,8 @@ DeclContext *ASTDeclReader::getPrimaryContextForMerging(ASTReader &Reader, RD->getCanonicalDecl()->DefinitionData = DD; // Track that we did this horrible thing so that we can fix it later. - Reader.PendingFakeDefinitionData.insert(DD); + Reader.PendingFakeDefinitionData.insert( + std::make_pair(DD, ASTReader::PendingFakeDefinitionKind::Fake)); } return DD->Definition; @@ -3638,18 +3646,19 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile, case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: { auto *RD = cast(D); - bool HadRealDefinition = RD->getDefinition() && - !Reader.PendingFakeDefinitionData.count( - RD->DefinitionData.getNotUpdated()); - ReadCXXRecordDefinition(RD); + auto *OldDD = RD->DefinitionData.getNotUpdated(); + bool HadRealDefinition = + OldDD && !Reader.PendingFakeDefinitionData.count(OldDD); + ReadCXXRecordDefinition(RD, /*Update*/true); + // Visible update is handled separately. uint64_t LexicalOffset = Record[Idx++]; if (!HadRealDefinition && LexicalOffset) { RD->setHasExternalLexicalStorage(true); Reader.ReadDeclContextStorage(ModuleFile, ModuleFile.DeclsCursor, - std::make_pair(LexicalOffset, 0), - ModuleFile.DeclContextInfos[RD]); - Reader.PendingDefinitions.insert(RD); + std::make_pair(LexicalOffset, 0), + ModuleFile.DeclContextInfos[RD]); + Reader.PendingFakeDefinitionData.erase(OldDD); } auto TSK = (TemplateSpecializationKind)Record[Idx++]; diff --git a/test/Modules/Inputs/merge-template-members/a.h b/test/Modules/Inputs/merge-template-members/a.h new file mode 100644 index 0000000000..9212a3f0de --- /dev/null +++ b/test/Modules/Inputs/merge-template-members/a.h @@ -0,0 +1,9 @@ +namespace N { + template struct A { + int n; + A() : n() {} + }; + + // Create declaration of A. + typedef A AI; +} diff --git a/test/Modules/Inputs/merge-template-members/b.h b/test/Modules/Inputs/merge-template-members/b.h new file mode 100644 index 0000000000..df537b0d59 --- /dev/null +++ b/test/Modules/Inputs/merge-template-members/b.h @@ -0,0 +1,6 @@ +#include "a.h" + +// Add update record for definition of A and constructors. +// We need an eagerly-emitted function here to get the problematic +// deserialization ordering. +void foobar() { N::A x; } diff --git a/test/Modules/Inputs/merge-template-members/c.h b/test/Modules/Inputs/merge-template-members/c.h new file mode 100644 index 0000000000..54f34799c3 --- /dev/null +++ b/test/Modules/Inputs/merge-template-members/c.h @@ -0,0 +1,14 @@ +namespace N { + template struct A { + int n; + A() : n() {} + }; + + // Trigger instantiation of definition of A. + struct C { + A a; + }; +} + +// Merge in another declaration and update records. +#include "b.h" diff --git a/test/Modules/Inputs/merge-template-members/module.modulemap b/test/Modules/Inputs/merge-template-members/module.modulemap index 9ce569074d..280667fcd7 100644 --- a/test/Modules/Inputs/merge-template-members/module.modulemap +++ b/test/Modules/Inputs/merge-template-members/module.modulemap @@ -1,2 +1,6 @@ module def { header "def.h" export * } module update { header "update.h" export * } + +module a { header "a.h" export * } +module b { header "b.h" export * } +module c { header "c.h" export * } diff --git a/test/Modules/merge-template-members.cpp b/test/Modules/merge-template-members.cpp index 99696d8ad1..c0bfd2f22b 100644 --- a/test/Modules/merge-template-members.cpp +++ b/test/Modules/merge-template-members.cpp @@ -1,7 +1,10 @@ // RUN: rm -rf %t -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/merge-template-members -verify %s +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/merge-template-members -verify -emit-llvm-only %s // expected-no-diagnostics +#include "c.h" +N::A ai; + template struct A { int n; }; template struct B { typedef A C; }; template class B;