[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.
llvm-svn: 227939
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 8dd9b14..227e617 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -435,9 +435,11 @@
llvm::SmallVector<std::pair<serialization::GlobalDeclID, Decl*>, 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<void*, 4> PendingFakeDefinitionData;
+ llvm::DenseMap<void *, PendingFakeDefinitionKind> PendingFakeDefinitionData;
struct ReplacedDeclInfo {
ModuleFile *Mod;
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 754b0a0..7f666a4 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -103,7 +103,7 @@
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 @@
"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 @@
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 @@
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 @@
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 @@
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 @@
case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: {
auto *RD = cast<CXXRecordDecl>(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/clang/test/Modules/Inputs/merge-template-members/a.h b/clang/test/Modules/Inputs/merge-template-members/a.h
new file mode 100644
index 0000000..9212a3f
--- /dev/null
+++ b/clang/test/Modules/Inputs/merge-template-members/a.h
@@ -0,0 +1,9 @@
+namespace N {
+ template <typename> struct A {
+ int n;
+ A() : n() {}
+ };
+
+ // Create declaration of A<int>.
+ typedef A<int> AI;
+}
diff --git a/clang/test/Modules/Inputs/merge-template-members/b.h b/clang/test/Modules/Inputs/merge-template-members/b.h
new file mode 100644
index 0000000..df537b0
--- /dev/null
+++ b/clang/test/Modules/Inputs/merge-template-members/b.h
@@ -0,0 +1,6 @@
+#include "a.h"
+
+// Add update record for definition of A<int> and constructors.
+// We need an eagerly-emitted function here to get the problematic
+// deserialization ordering.
+void foobar() { N::A<int> x; }
diff --git a/clang/test/Modules/Inputs/merge-template-members/c.h b/clang/test/Modules/Inputs/merge-template-members/c.h
new file mode 100644
index 0000000..54f3479
--- /dev/null
+++ b/clang/test/Modules/Inputs/merge-template-members/c.h
@@ -0,0 +1,14 @@
+namespace N {
+ template <typename> struct A {
+ int n;
+ A() : n() {}
+ };
+
+ // Trigger instantiation of definition of A<int>.
+ struct C {
+ A<int> a;
+ };
+}
+
+// Merge in another declaration and update records.
+#include "b.h"
diff --git a/clang/test/Modules/Inputs/merge-template-members/module.modulemap b/clang/test/Modules/Inputs/merge-template-members/module.modulemap
index 9ce5690..280667f 100644
--- a/clang/test/Modules/Inputs/merge-template-members/module.modulemap
+++ b/clang/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/clang/test/Modules/merge-template-members.cpp b/clang/test/Modules/merge-template-members.cpp
index 99696d8..c0bfd2f 100644
--- a/clang/test/Modules/merge-template-members.cpp
+++ b/clang/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<int> ai;
+
template<typename> struct A { int n; };
template<typename> struct B { typedef A<void> C; };
template class B<int>;