r226977 - [modules] Sometimes we can deserialize a class member but not have yet
Richard Smith
richard-llvm at metafoo.co.uk
Fri Jan 23 17:07:20 PST 2015
Author: rsmith
Date: Fri Jan 23 19:07:20 2015
New Revision: 226977
URL: http://llvm.org/viewvc/llvm-project?rev=226977&view=rev
Log:
[modules] Sometimes we can deserialize a class member but not have yet
encountered any definition for the class; this happens when the definition is
added by an update record that is not yet loaded. In such a case, eagerly pick
the original parent of the member as the canonical definition of the class
rather than muddling through with the canonical declaration (the latter can
lead to us failing to merge properly later if the canonical definition turns
out to be some other declaration).
Added:
cfe/trunk/test/Modules/Inputs/merge-nested-templates/
cfe/trunk/test/Modules/Inputs/merge-nested-templates/a.h
cfe/trunk/test/Modules/Inputs/merge-nested-templates/b.h
cfe/trunk/test/Modules/Inputs/merge-nested-templates/c.h
cfe/trunk/test/Modules/Inputs/merge-nested-templates/module.modulemap
cfe/trunk/test/Modules/Inputs/merge-nested-templates/string.ii
cfe/trunk/test/Modules/merge-nested-templates.cpp
Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=226977&r1=226976&r2=226977&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Jan 23 19:07:20 2015
@@ -435,6 +435,10 @@ private:
llvm::SmallVector<std::pair<serialization::GlobalDeclID, Decl*>, 16>
PendingUpdateRecords;
+ /// \brief The DefinitionData pointers that we faked up for class definitions
+ /// that we needed but hadn't loaded yet.
+ llvm::SmallPtrSet<void*, 4> PendingFakeDefinitionData;
+
struct ReplacedDeclInfo {
ModuleFile *Mod;
uint64_t Offset;
Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=226977&r1=226976&r2=226977&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Fri Jan 23 19:07:20 2015
@@ -1099,10 +1099,13 @@ void ASTDumper::VisitFunctionDecl(const
E = D->getDeclsInPrototypeScope().end(); I != E; ++I)
dumpDecl(*I);
- for (FunctionDecl::param_const_iterator I = D->param_begin(),
- E = D->param_end();
- I != E; ++I)
- dumpDecl(*I);
+ if (!D->param_begin() && D->getNumParams())
+ dumpChild([=] { OS << "<<NULL params x " << D->getNumParams() << ">>"; });
+ else
+ for (FunctionDecl::param_const_iterator I = D->param_begin(),
+ E = D->param_end();
+ I != E; ++I)
+ dumpDecl(*I);
if (const CXXConstructorDecl *C = dyn_cast<CXXConstructorDecl>(D))
for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=226977&r1=226976&r2=226977&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Jan 23 19:07:20 2015
@@ -8298,7 +8298,12 @@ void ASTReader::finishPendingActions() {
loadDeclUpdateRecords(Update.first, Update.second);
}
}
-
+
+ // At this point, all update records for loaded decls are in place, so any
+ // fake class definitions should have become real.
+ assert(PendingFakeDefinitionData.empty() &&
+ "faked up a class definition but never saw the real one");
+
// If we deserialized any C++ or Objective-C class definitions, any
// Objective-C protocol definitions, or any redeclarable templates, make sure
// that all redeclarations point to the definitions. Note that this can only
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=226977&r1=226976&r2=226977&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Jan 23 19:07:20 2015
@@ -107,7 +107,7 @@ namespace clang {
void ReadCXXDefinitionData(struct CXXRecordDecl::DefinitionData &Data,
const RecordData &R, unsigned &I);
void MergeDefinitionData(CXXRecordDecl *D,
- struct CXXRecordDecl::DefinitionData &NewDD);
+ struct CXXRecordDecl::DefinitionData &&NewDD);
static NamedDecl *getAnonymousDeclForMerging(ASTReader &Reader,
DeclContext *DC,
@@ -205,6 +205,8 @@ namespace clang {
operator T*() const { return dyn_cast_or_null<T>(Existing); }
};
+ static DeclContext *getPrimaryContextForMerging(ASTReader &Reader,
+ DeclContext *DC);
FindExistingResult findExisting(NamedDecl *D);
public:
@@ -1353,11 +1355,25 @@ void ASTDeclReader::ReadCXXDefinitionDat
}
void ASTDeclReader::MergeDefinitionData(
- CXXRecordDecl *D, struct CXXRecordDecl::DefinitionData &MergeDD) {
+ CXXRecordDecl *D, struct CXXRecordDecl::DefinitionData &&MergeDD) {
assert(D->DefinitionData.getNotUpdated() &&
"merging class definition into non-definition");
auto &DD = *D->DefinitionData.getNotUpdated();
+ if (Reader.PendingFakeDefinitionData.count(&DD)) {
+ // 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?");
+
+ // Don't change which declaration is the definition; that is required
+ // to be invariant once we select it.
+ auto *Def = DD.Definition;
+ DD = std::move(MergeDD);
+ DD.Definition = Def;
+ return;
+ }
+
// If the new definition has new special members, let the name lookup
// code know that it needs to look in the new definition too.
//
@@ -1460,17 +1476,18 @@ void ASTDeclReader::ReadCXXRecordDefinit
// We might already have a definition for this record. This can happen either
// because we're reading an update record, or because we've already done some
// merging. Either way, just merge into it.
- if (auto *CanonDD = D->DefinitionData.getNotUpdated()) {
+ CXXRecordDecl *Canon = D->getCanonicalDecl();
+ if (auto *CanonDD = Canon->DefinitionData.getNotUpdated()) {
if (CanonDD->Definition != DD->Definition)
Reader.MergedDeclContexts.insert(
std::make_pair(DD->Definition, CanonDD->Definition));
- MergeDefinitionData(D, *DD);
+ MergeDefinitionData(Canon, std::move(*DD));
+ D->DefinitionData = Canon->DefinitionData;
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;
@@ -1482,7 +1499,7 @@ void ASTDeclReader::ReadCXXRecordDefinit
std::make_pair(D, CanonDD->Definition));
D->DefinitionData = Canon->DefinitionData;
D->IsCompleteDefinition = false;
- MergeDefinitionData(D, *DD);
+ MergeDefinitionData(D, std::move(*DD));
} else {
Canon->DefinitionData = DD;
D->DefinitionData = Canon->DefinitionData;
@@ -1827,7 +1844,7 @@ ASTDeclReader::VisitClassTemplateSpecial
// definition.
if (auto *DDD = D->DefinitionData.getNotUpdated()) {
if (auto *CanonDD = CanonSpec->DefinitionData.getNotUpdated()) {
- MergeDefinitionData(CanonSpec, *DDD);
+ MergeDefinitionData(CanonSpec, std::move(*DDD));
Reader.PendingDefinitions.erase(D);
Reader.MergedDeclContexts.insert(
std::make_pair(D, CanonDD->Definition));
@@ -2125,6 +2142,7 @@ void ASTDeclReader::mergeTemplatePattern
auto *ExistingPattern = Existing->getTemplatedDecl();
RedeclarableResult Result(Reader, DPattern->getCanonicalDecl()->getGlobalID(),
DPattern->getKind());
+
if (auto *DClass = dyn_cast<CXXRecordDecl>(DPattern)) {
// Merge with any existing definition.
// FIXME: This is duplicated in several places. Refactor.
@@ -2132,7 +2150,7 @@ void ASTDeclReader::mergeTemplatePattern
cast<CXXRecordDecl>(ExistingPattern)->getCanonicalDecl();
if (auto *DDD = DClass->DefinitionData.getNotUpdated()) {
if (auto *ExistingDD = ExistingClass->DefinitionData.getNotUpdated()) {
- MergeDefinitionData(ExistingClass, *DDD);
+ MergeDefinitionData(ExistingClass, std::move(*DDD));
Reader.PendingDefinitions.erase(DClass);
Reader.MergedDeclContexts.insert(
std::make_pair(DClass, ExistingDD->Definition));
@@ -2533,18 +2551,33 @@ static bool isSameEntity(NamedDecl *X, N
/// Find the context in which we should search for previous declarations when
/// looking for declarations to merge.
-static DeclContext *getPrimaryContextForMerging(DeclContext *DC) {
+DeclContext *ASTDeclReader::getPrimaryContextForMerging(ASTReader &Reader,
+ DeclContext *DC) {
if (NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC))
return ND->getOriginalNamespace();
- // There is one tricky case here: if DC is a class with no definition, then
- // we're merging a declaration whose definition is added by an update record,
- // but we've not yet loaded that update record. In this case, we use the
- // canonical declaration for merging until we get a real definition.
- // FIXME: When we add a definition, we may need to move the partial lookup
- // information from the canonical declaration onto the chosen definition.
- if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC))
- return RD->getPrimaryContext();
+ if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC)) {
+ // Try to dig out the definition.
+ auto *DD = RD->DefinitionData.getNotUpdated();
+ if (!DD)
+ DD = RD->getCanonicalDecl()->DefinitionData.getNotUpdated();
+
+ // If there's no definition yet, then DC's definition is added by an update
+ // record, but we've not yet loaded that update record. In this case, we
+ // commit to DC being the canonical definition now, and will fix this when
+ // we load the update record.
+ if (!DD) {
+ DD = new (Reader.Context) struct CXXRecordDecl::DefinitionData(RD);
+ RD->IsCompleteDefinition = true;
+ RD->DefinitionData = DD;
+ RD->getCanonicalDecl()->DefinitionData = DD;
+
+ // Track that we did this horrible thing so that we can fix it later.
+ Reader.PendingFakeDefinitionData.insert(DD);
+ }
+
+ return DD->Definition;
+ }
if (EnumDecl *ED = dyn_cast<EnumDecl>(DC))
return ED->getASTContext().getLangOpts().CPlusPlus? ED->getDefinition()
@@ -2574,7 +2607,7 @@ ASTDeclReader::FindExistingResult::~Find
AnonymousDeclNumber, New);
} else if (DC->isTranslationUnit() && Reader.SemaObj) {
Reader.SemaObj->IdResolver.tryAddTopLevelDecl(New, Name);
- } else if (DeclContext *MergeDC = getPrimaryContextForMerging(DC)) {
+ } else if (DeclContext *MergeDC = getPrimaryContextForMerging(Reader, DC)) {
// Add the declaration to its redeclaration context so later merging
// lookups will find it.
MergeDC->makeDeclVisibleInContextImpl(New, /*Internal*/true);
@@ -2720,7 +2753,7 @@ ASTDeclReader::FindExistingResult ASTDec
return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
TypedefNameForLinkage);
}
- } else if (DeclContext *MergeDC = getPrimaryContextForMerging(DC)) {
+ } else if (DeclContext *MergeDC = getPrimaryContextForMerging(Reader, DC)) {
DeclContext::lookup_result R = MergeDC->noload_lookup(Name);
for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) {
if (NamedDecl *Existing = getDeclForMerging(*I, TypedefNameForLinkage))
@@ -3605,11 +3638,13 @@ void ASTDeclReader::UpdateDecl(Decl *D,
case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: {
auto *RD = cast<CXXRecordDecl>(D);
- bool HadDefinition = RD->getDefinition();
+ bool HadRealDefinition = RD->getDefinition() &&
+ !Reader.PendingFakeDefinitionData.count(
+ RD->DefinitionData.getNotUpdated());
ReadCXXRecordDefinition(RD);
// Visible update is handled separately.
uint64_t LexicalOffset = Record[Idx++];
- if (!HadDefinition && LexicalOffset) {
+ if (!HadRealDefinition && LexicalOffset) {
RD->setHasExternalLexicalStorage(true);
Reader.ReadDeclContextStorage(ModuleFile, ModuleFile.DeclsCursor,
std::make_pair(LexicalOffset, 0),
Added: cfe/trunk/test/Modules/Inputs/merge-nested-templates/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-nested-templates/a.h?rev=226977&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-nested-templates/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-nested-templates/a.h Fri Jan 23 19:07:20 2015
@@ -0,0 +1 @@
+#include "string.ii"
Added: cfe/trunk/test/Modules/Inputs/merge-nested-templates/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-nested-templates/b.h?rev=226977&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-nested-templates/b.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-nested-templates/b.h Fri Jan 23 19:07:20 2015
@@ -0,0 +1,2 @@
+#include "a.h"
+std::wstring::iterator j;
Added: cfe/trunk/test/Modules/Inputs/merge-nested-templates/c.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-nested-templates/c.h?rev=226977&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-nested-templates/c.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-nested-templates/c.h Fri Jan 23 19:07:20 2015
@@ -0,0 +1,3 @@
+#include "string.ii"
+std::wstring::iterator i;
+#include "b.h"
Added: cfe/trunk/test/Modules/Inputs/merge-nested-templates/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-nested-templates/module.modulemap?rev=226977&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-nested-templates/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/merge-nested-templates/module.modulemap Fri Jan 23 19:07:20 2015
@@ -0,0 +1,3 @@
+module a { header "a.h" export * }
+module b { header "b.h" export * }
+module c { header "c.h" export * }
Added: cfe/trunk/test/Modules/Inputs/merge-nested-templates/string.ii
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-nested-templates/string.ii?rev=226977&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-nested-templates/string.ii (added)
+++ cfe/trunk/test/Modules/Inputs/merge-nested-templates/string.ii Fri Jan 23 19:07:20 2015
@@ -0,0 +1,14 @@
+namespace std {
+ template <typename, typename Container> struct normal_iterator {
+ normal_iterator() {}
+
+ template <typename I>
+ normal_iterator(normal_iterator<I, typename Container::iterator>) {}
+ };
+
+ template <typename pointer> struct basic_string {
+ typedef normal_iterator<pointer, basic_string> iterator;
+ };
+
+ typedef basic_string<wchar_t *> wstring;
+}
Added: cfe/trunk/test/Modules/merge-nested-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-nested-templates.cpp?rev=226977&view=auto
==============================================================================
--- cfe/trunk/test/Modules/merge-nested-templates.cpp (added)
+++ cfe/trunk/test/Modules/merge-nested-templates.cpp Fri Jan 23 19:07:20 2015
@@ -0,0 +1,4 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/merge-nested-templates -verify %s
+// expected-no-diagnostics
+#include "c.h"
More information about the cfe-commits
mailing list