From: Richard Smith Date: Sat, 13 Jul 2013 02:00:19 +0000 (+0000) Subject: C++ modules: Don't call DeclContext::lookup when half-way through deserializing X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e7bae1597f4a7088f5048695c14a8f1013a86108;p=clang C++ modules: Don't call DeclContext::lookup when half-way through deserializing decls. That can reenter deserialization and explode horribly by trying to merge a declaration that we've not got very far through deserializing yet. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@186236 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h index 157506d2a6..c19d3cf841 100644 --- a/include/clang/AST/DeclBase.h +++ b/include/clang/AST/DeclBase.h @@ -991,6 +991,7 @@ protected: mutable Decl *LastDecl; friend class ExternalASTSource; + friend class ASTDeclReader; friend class ASTWriter; /// \brief Build up a chain of declarations. @@ -1446,12 +1447,20 @@ public: return const_cast(this)->lookup(Name); } + /// \brief Find the declarations with the given name that are visible + /// within this context; don't attempt to retrieve anything from an + /// external source. + lookup_result noload_lookup(DeclarationName Name); + /// \brief A simplistic name lookup mechanism that performs name lookup /// into this declaration context without consulting the external source. /// /// This function should almost never be used, because it subverts the /// usual relationship between a DeclContext and the external source. /// See the ASTImporter for the (few, but important) use cases. + /// + /// FIXME: This is very inefficient; replace uses of it with uses of + /// noload_lookup. void localUncachedLookup(DeclarationName Name, SmallVectorImpl &Results); @@ -1573,6 +1582,8 @@ private: friend class DependentDiagnostic; StoredDeclsMap *CreateStoredDeclsMap(ASTContext &C) const; + template void buildLookupImpl(DeclContext *DCtx); void makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal, bool Rediscoverable); diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp index 7caf698c98..13bde4ca86 100644 --- a/lib/AST/DeclBase.cpp +++ b/lib/AST/DeclBase.cpp @@ -1170,7 +1170,8 @@ StoredDeclsMap *DeclContext::buildLookup() { SmallVector Contexts; collectAllContexts(Contexts); for (unsigned I = 0, N = Contexts.size(); I != N; ++I) - buildLookupImpl(Contexts[I]); + buildLookupImpl<&DeclContext::decls_begin, + &DeclContext::decls_end>(Contexts[I]); // We no longer have any lazy decls. LookupPtr.setInt(false); @@ -1182,8 +1183,10 @@ StoredDeclsMap *DeclContext::buildLookup() { /// declarations contained within DCtx, which will either be this /// DeclContext, a DeclContext linked to it, or a transparent context /// nested within it. +template void DeclContext::buildLookupImpl(DeclContext *DCtx) { - for (decl_iterator I = DCtx->decls_begin(), E = DCtx->decls_end(); + for (decl_iterator I = (DCtx->*Begin)(), E = (DCtx->*End)(); I != E; ++I) { Decl *D = *I; @@ -1207,7 +1210,7 @@ void DeclContext::buildLookupImpl(DeclContext *DCtx) { // context (recursively). if (DeclContext *InnerCtx = dyn_cast(D)) if (InnerCtx->isTransparentContext() || InnerCtx->isInlineNamespace()) - buildLookupImpl(InnerCtx); + buildLookupImpl(InnerCtx); } } @@ -1264,6 +1267,45 @@ DeclContext::lookup(DeclarationName Name) { return I->second.getLookupResult(); } +DeclContext::lookup_result +DeclContext::noload_lookup(DeclarationName Name) { + assert(DeclKind != Decl::LinkageSpec && + "Should not perform lookups into linkage specs!"); + if (!hasExternalVisibleStorage()) + return lookup(Name); + + DeclContext *PrimaryContext = getPrimaryContext(); + if (PrimaryContext != this) + return PrimaryContext->noload_lookup(Name); + + StoredDeclsMap *Map = LookupPtr.getPointer(); + if (LookupPtr.getInt()) { + // Carefully build the lookup map, without deserializing anything. + SmallVector Contexts; + collectAllContexts(Contexts); + for (unsigned I = 0, N = Contexts.size(); I != N; ++I) + buildLookupImpl<&DeclContext::noload_decls_begin, + &DeclContext::noload_decls_end>(Contexts[I]); + + // We no longer have any lazy decls. + LookupPtr.setInt(false); + + // There may now be names for which we have local decls but are + // missing the external decls. + NeedToReconcileExternalVisibleStorage = true; + + Map = LookupPtr.getPointer(); + } + + if (!Map) + return lookup_result(lookup_iterator(0), lookup_iterator(0)); + + StoredDeclsMap::iterator I = Map->find(Name); + return I != Map->end() + ? I->second.getLookupResult() + : lookup_result(lookup_iterator(0), lookup_iterator(0)); +} + void DeclContext::localUncachedLookup(DeclarationName Name, SmallVectorImpl &Results) { Results.clear(); diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index 84a8b09e4e..e713b0e382 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -1914,14 +1914,15 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) { ASTDeclReader::FindExistingResult::~FindExistingResult() { if (!AddResult || Existing) return; - - if (New->getDeclContext()->getRedeclContext()->isTranslationUnit() - && Reader.SemaObj) { + + DeclContext *DC = New->getDeclContext()->getRedeclContext(); + if (DC->isTranslationUnit() && Reader.SemaObj) { Reader.SemaObj->IdResolver.tryAddTopLevelDecl(New, New->getDeclName()); - } else { - DeclContext *DC = New->getLexicalDeclContext(); - if (DC->isNamespace()) - DC->addDecl(New); + } else if (NamespaceDecl *NS = dyn_cast(DC)) { + // Add the declaration to its redeclaration context so later merging + // lookups will find it. + NS->getFirstDeclaration()->makeDeclVisibleInContextImpl(New, + /*Internal*/true); } } @@ -1933,11 +1934,11 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { Result.suppress(); return Result; } - + DeclContext *DC = D->getDeclContext()->getRedeclContext(); if (!DC->isFileContext()) return FindExistingResult(Reader); - + if (DC->isTranslationUnit() && Reader.SemaObj) { IdentifierResolver &IdResolver = Reader.SemaObj->IdResolver; @@ -1970,12 +1971,9 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { if (isSameEntity(*I, D)) return FindExistingResult(Reader, D, *I); } - } - - if (DC->isNamespace()) { - DeclContext::lookup_result R = DC->lookup(Name); - for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; - ++I) { + } else if (NamespaceDecl *NS = dyn_cast(DC)) { + DeclContext::lookup_result R = NS->getFirstDeclaration()->noload_lookup(Name); + for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) { if (isSameEntity(*I, D)) return FindExistingResult(Reader, D, *I); } diff --git a/test/PCH/cxx-templates.cpp b/test/PCH/cxx-templates.cpp index 433f73f12a..f57245e7c1 100644 --- a/test/PCH/cxx-templates.cpp +++ b/test/PCH/cxx-templates.cpp @@ -94,3 +94,8 @@ namespace rdar13135282 { void CallDependentSpecializedFunc(DependentSpecializedFuncClass &x) { DependentSpecializedFunc(x); } + +namespace cyclic_module_load { + extern std::valarray x; + std::valarray y(x); +} diff --git a/test/PCH/cxx-templates.h b/test/PCH/cxx-templates.h index 00064aeefa..ce6b7051ec 100644 --- a/test/PCH/cxx-templates.h +++ b/test/PCH/cxx-templates.h @@ -276,3 +276,23 @@ template class DependentSpecializedFuncClass { void foo() {} friend void DependentSpecializedFunc<>(DependentSpecializedFuncClass); }; + +namespace cyclic_module_load { + // Reduced from a libc++ modules crasher. + namespace std { + template class mask_array; + template class valarray { + public: + valarray(const valarray &v); + }; + + class gslice { + valarray x; + valarray stride() const { return x; } + }; + + template class mask_array { + template friend class valarray; + }; + } +}