Hi rsmith,
Before this patch there were two problems with the serialization and
deserialization of inherited constructors in AstWriter and AstReader.
First, the inherited constructor of a CxxConstructorDecl wasn't (de)serialized.
Second, the constructors of a class and the using declarations for constructors
of base classes had different DeclarationNames but identical keys in the
OnDiskHashTable. This lead to WriteDeclContextVisibleBlock writing multiple
entries with the same key into the OnDiskHashTable, violating an expected
invariant. (Whether this issue alone actually triggers a compilation error
depends on the (somewhat unstable) order in which WriteDeclContextVisibleBlock
iterates over the declarations in the DeclContext.
I fixed the second issue by letting WriteDeclContextVisibleBlock treat
CXXConstructorNames like CXXConversionFunctionNames. I'm not overly fond of
this approach and would appreciate suggestions on how to do this better
(without requiring major refactorings).
A comment that I rewrote in WriteDeclContextVisibleBlock originally stated that
type information wasn't used in the key "since such type information is not
stable across different modules". I'm not sure whether this statement is
(still) accurate and should be preserved in the new comment.
http://llvm-reviews.chandlerc.com/D3102
Files:
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
lib/Serialization/ASTWriterDecl.cpp
test/PCH/pr18307.cpp
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1348,7 +1348,9 @@
void ASTDeclReader::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
VisitCXXMethodDecl(D);
-
+
+ if (auto *CD = ReadDeclAs<CXXConstructorDecl>(Record, Idx))
+ D->setInheritedConstructor(CD);
D->IsExplicitSpecified = Record[Idx++];
std::tie(D->CtorInitializers, D->NumCtorInitializers) =
Reader.ReadCXXCtorInitializers(F, Record, Idx);
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -3429,30 +3429,46 @@
ASTDeclContextNameLookupTrait Trait(*this);
// Create the on-disk hash table representation.
+ DeclarationName ConstructorName;
DeclarationName ConversionName;
+ SmallVector<NamedDecl *, 8> ConstructorDecls;
SmallVector<NamedDecl *, 4> ConversionDecls;
for (StoredDeclsMap::iterator D = Map->begin(), DEnd = Map->end();
D != DEnd; ++D) {
DeclarationName Name = D->first;
DeclContext::lookup_result Result = D->second.getLookupResult();
if (!Result.empty()) {
- if (Name.getNameKind() == DeclarationName::CXXConversionFunctionName) {
- // Hash all conversion function names to the same name. The actual
- // type information in conversion function name is not used in the
- // key (since such type information is not stable across different
- // modules), so the intended effect is to coalesce all of the
conversion
- // functions under a single key.
+ // Different DeclarationName values of certain kinds are mapped to
+ // identical serialized keys, because we don't to use type identifiers
+ // in the keys.
+ switch (Name.getNameKind()) {
+ case DeclarationName::CXXConstructorName:
+ // There may be different CXXConstructorName DeclarationName values
+ // in a DeclContext because a UsingDecl that inherits constructors
+ // has the DeclarationName of the inherited constructors.
+ if (!ConstructorName)
+ ConstructorName = Name;
+ ConstructorDecls.append(Result.begin(), Result.end());
+ continue;
+ case DeclarationName::CXXConversionFunctionName:
if (!ConversionName)
ConversionName = Name;
ConversionDecls.append(Result.begin(), Result.end());
continue;
+ default:
+ break;
}
-
Generator.insert(Name, Result, Trait);
}
}
-
- // Add the conversion functions
+ // Add the constructors.
+ if (!ConstructorDecls.empty()) {
+ Generator.insert(ConstructorName,
+ DeclContext::lookup_result(ConstructorDecls.begin(),
+ ConstructorDecls.end()),
+ Trait);
+ }
+ // Add the conversion functions.
if (!ConversionDecls.empty()) {
Generator.insert(ConversionName,
DeclContext::lookup_result(ConversionDecls.begin(),
Index: lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -1005,6 +1005,7 @@
void ASTDeclWriter::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
VisitCXXMethodDecl(D);
+ Writer.AddDeclRef(D->getInheritedConstructor(), Record);
Record.push_back(D->IsExplicitSpecified);
Writer.AddCXXCtorInitializers(D->CtorInitializers, D->NumCtorInitializers,
Record);
Index: test/PCH/pr18307.cpp
===================================================================
--- /dev/null
+++ test/PCH/pr18307.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -std=c++11 -include-pch %t -verify %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER_INCLUDED
+#define HEADER_INCLUDED
+
+struct Base {
+ Base(int) {}
+};
+
+struct Test : Base {
+ using Base::Base;
+};
+
+#else
+
+Test test(42);
+
+#endif // HEADER_INCLUDED
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1348,7 +1348,9 @@
void ASTDeclReader::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
VisitCXXMethodDecl(D);
-
+
+ if (auto *CD = ReadDeclAs<CXXConstructorDecl>(Record, Idx))
+ D->setInheritedConstructor(CD);
D->IsExplicitSpecified = Record[Idx++];
std::tie(D->CtorInitializers, D->NumCtorInitializers) =
Reader.ReadCXXCtorInitializers(F, Record, Idx);
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -3429,30 +3429,46 @@
ASTDeclContextNameLookupTrait Trait(*this);
// Create the on-disk hash table representation.
+ DeclarationName ConstructorName;
DeclarationName ConversionName;
+ SmallVector<NamedDecl *, 8> ConstructorDecls;
SmallVector<NamedDecl *, 4> ConversionDecls;
for (StoredDeclsMap::iterator D = Map->begin(), DEnd = Map->end();
D != DEnd; ++D) {
DeclarationName Name = D->first;
DeclContext::lookup_result Result = D->second.getLookupResult();
if (!Result.empty()) {
- if (Name.getNameKind() == DeclarationName::CXXConversionFunctionName) {
- // Hash all conversion function names to the same name. The actual
- // type information in conversion function name is not used in the
- // key (since such type information is not stable across different
- // modules), so the intended effect is to coalesce all of the conversion
- // functions under a single key.
+ // Different DeclarationName values of certain kinds are mapped to
+ // identical serialized keys, because we don't to use type identifiers
+ // in the keys.
+ switch (Name.getNameKind()) {
+ case DeclarationName::CXXConstructorName:
+ // There may be different CXXConstructorName DeclarationName values
+ // in a DeclContext because a UsingDecl that inherits constructors
+ // has the DeclarationName of the inherited constructors.
+ if (!ConstructorName)
+ ConstructorName = Name;
+ ConstructorDecls.append(Result.begin(), Result.end());
+ continue;
+ case DeclarationName::CXXConversionFunctionName:
if (!ConversionName)
ConversionName = Name;
ConversionDecls.append(Result.begin(), Result.end());
continue;
+ default:
+ break;
}
-
Generator.insert(Name, Result, Trait);
}
}
-
- // Add the conversion functions
+ // Add the constructors.
+ if (!ConstructorDecls.empty()) {
+ Generator.insert(ConstructorName,
+ DeclContext::lookup_result(ConstructorDecls.begin(),
+ ConstructorDecls.end()),
+ Trait);
+ }
+ // Add the conversion functions.
if (!ConversionDecls.empty()) {
Generator.insert(ConversionName,
DeclContext::lookup_result(ConversionDecls.begin(),
Index: lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -1005,6 +1005,7 @@
void ASTDeclWriter::VisitCXXConstructorDecl(CXXConstructorDecl *D) {
VisitCXXMethodDecl(D);
+ Writer.AddDeclRef(D->getInheritedConstructor(), Record);
Record.push_back(D->IsExplicitSpecified);
Writer.AddCXXCtorInitializers(D->CtorInitializers, D->NumCtorInitializers,
Record);
Index: test/PCH/pr18307.cpp
===================================================================
--- /dev/null
+++ test/PCH/pr18307.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++11 -emit-pch -o %t %s
+// RUN: %clang_cc1 -std=c++11 -include-pch %t -verify %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER_INCLUDED
+#define HEADER_INCLUDED
+
+struct Base {
+ Base(int) {}
+};
+
+struct Test : Base {
+ using Base::Base;
+};
+
+#else
+
+Test test(42);
+
+#endif // HEADER_INCLUDED
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
