r186236 - C++ modules: Don't call DeclContext::lookup when half-way through deserializing
Richard Smith
richard-llvm at metafoo.co.uk
Fri Jul 12 19:00:20 PDT 2013
Author: rsmith
Date: Fri Jul 12 21:00:19 2013
New Revision: 186236
URL: http://llvm.org/viewvc/llvm-project?rev=186236&view=rev
Log:
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.
Modified:
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/test/PCH/cxx-templates.cpp
cfe/trunk/test/PCH/cxx-templates.h
Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=186236&r1=186235&r2=186236&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Fri Jul 12 21:00:19 2013
@@ -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<DeclContext*>(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<NamedDecl *> &Results);
@@ -1573,6 +1582,8 @@ private:
friend class DependentDiagnostic;
StoredDeclsMap *CreateStoredDeclsMap(ASTContext &C) const;
+ template<decl_iterator (DeclContext::*Begin)() const,
+ decl_iterator (DeclContext::*End)() const>
void buildLookupImpl(DeclContext *DCtx);
void makeDeclVisibleInContextWithFlags(NamedDecl *D, bool Internal,
bool Rediscoverable);
Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=186236&r1=186235&r2=186236&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Fri Jul 12 21:00:19 2013
@@ -1170,7 +1170,8 @@ StoredDeclsMap *DeclContext::buildLookup
SmallVector<DeclContext *, 2> 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<DeclContext::decl_iterator (DeclContext::*Begin)() const,
+ DeclContext::decl_iterator (DeclContext::*End)() const>
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(DeclCo
// context (recursively).
if (DeclContext *InnerCtx = dyn_cast<DeclContext>(D))
if (InnerCtx->isTransparentContext() || InnerCtx->isInlineNamespace())
- buildLookupImpl(InnerCtx);
+ buildLookupImpl<Begin, End>(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<DeclContext *, 2> 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<NamedDecl *> &Results) {
Results.clear();
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=186236&r1=186235&r2=186236&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Jul 12 21:00:19 2013
@@ -1914,14 +1914,15 @@ static bool isSameEntity(NamedDecl *X, N
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<NamespaceDecl>(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 ASTDec
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 ASTDec
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<NamespaceDecl>(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);
}
Modified: cfe/trunk/test/PCH/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/cxx-templates.cpp?rev=186236&r1=186235&r2=186236&view=diff
==============================================================================
--- cfe/trunk/test/PCH/cxx-templates.cpp (original)
+++ cfe/trunk/test/PCH/cxx-templates.cpp Fri Jul 12 21:00:19 2013
@@ -94,3 +94,8 @@ namespace rdar13135282 {
void CallDependentSpecializedFunc(DependentSpecializedFuncClass<int> &x) {
DependentSpecializedFunc(x);
}
+
+namespace cyclic_module_load {
+ extern std::valarray<int> x;
+ std::valarray<int> y(x);
+}
Modified: cfe/trunk/test/PCH/cxx-templates.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/cxx-templates.h?rev=186236&r1=186235&r2=186236&view=diff
==============================================================================
--- cfe/trunk/test/PCH/cxx-templates.h (original)
+++ cfe/trunk/test/PCH/cxx-templates.h Fri Jul 12 21:00:19 2013
@@ -276,3 +276,23 @@ template<typename T> class DependentSpec
void foo() {}
friend void DependentSpecializedFunc<>(DependentSpecializedFuncClass);
};
+
+namespace cyclic_module_load {
+ // Reduced from a libc++ modules crasher.
+ namespace std {
+ template<class> class mask_array;
+ template<class> class valarray {
+ public:
+ valarray(const valarray &v);
+ };
+
+ class gslice {
+ valarray<int> x;
+ valarray<int> stride() const { return x; }
+ };
+
+ template<class> class mask_array {
+ template<class> friend class valarray;
+ };
+ }
+}
More information about the cfe-commits
mailing list