From 26d8bad7c2674ef96a547d4344beb2bd217aadbf Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Wed, 4 Dec 2019 19:02:41 -0800 Subject: [PATCH 1/5] Add DirectLookupRequest --- include/swift/AST/Decl.h | 4 ++ include/swift/AST/NameLookupRequests.h | 52 ++++++++++++++++++++ include/swift/AST/NameLookupTypeIDZone.def | 3 ++ include/swift/Basic/Statistics.def | 3 -- lib/AST/Decl.cpp | 9 ++++ lib/AST/NameLookup.cpp | 56 ++++++++++++---------- lib/AST/NameLookupRequests.cpp | 18 +++++++ 7 files changed, 118 insertions(+), 27 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index f11c0fe3e5eb9..e23e1f006f24f 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -3326,6 +3326,7 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext { friend class ExtensionDecl; friend class DeclContext; friend class IterableDeclContext; + friend class DirectLookupRequest; friend ArrayRef ValueDecl::getSatisfiedProtocolRequirements(bool Sorted) const; @@ -7413,6 +7414,9 @@ ParameterList *getParameterList(ValueDecl *source); /// nullptr if the source does not have a parameter list. const ParamDecl *getParameterAt(const ValueDecl *source, unsigned index); +void simple_display(llvm::raw_ostream &out, + OptionSet options); + /// Display Decl subclasses. void simple_display(llvm::raw_ostream &out, const Decl *decl); diff --git a/include/swift/AST/NameLookupRequests.h b/include/swift/AST/NameLookupRequests.h index dbc9ddb796200..f59f13ccd9982 100644 --- a/include/swift/AST/NameLookupRequests.h +++ b/include/swift/AST/NameLookupRequests.h @@ -467,6 +467,58 @@ class QualifiedLookupRequest NLOptions opts) const; }; +/// The input type for a direct lookup request. +class DirectLookupDescriptor final { + using LookupOptions = OptionSet; + +public: + NominalTypeDecl *DC; + DeclName Name; + LookupOptions Options; + + DirectLookupDescriptor(NominalTypeDecl *dc, DeclName name, + LookupOptions options = {}) + : DC(dc), Name(name), Options(options) {} + + friend llvm::hash_code hash_value(const DirectLookupDescriptor &desc) { + return llvm::hash_combine(desc.Name, desc.DC, desc.Options.toRaw()); + } + + friend bool operator==(const DirectLookupDescriptor &lhs, + const DirectLookupDescriptor &rhs) { + return lhs.Name == rhs.Name && lhs.DC == rhs.DC && + lhs.Options.toRaw() == rhs.Options.toRaw(); + } + + friend bool operator!=(const DirectLookupDescriptor &lhs, + const DirectLookupDescriptor &rhs) { + return !(lhs == rhs); + } +}; + +void simple_display(llvm::raw_ostream &out, const DirectLookupDescriptor &desc); + +SourceLoc extractNearestSourceLoc(const DirectLookupDescriptor &desc); + +class DirectLookupRequest + : public SimpleRequest(DirectLookupDescriptor), + CacheKind::Uncached> { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + // Evaluation. + llvm::Expected> + evaluate(Evaluator &evaluator, DirectLookupDescriptor desc) const; + +public: + // Cycle handling + void diagnoseCycle(DiagnosticEngine &diags) const; +}; + #define SWIFT_TYPEID_ZONE NameLookup #define SWIFT_TYPEID_HEADER "swift/AST/NameLookupTypeIDZone.def" #include "swift/Basic/DefineTypeIDZone.h" diff --git a/include/swift/AST/NameLookupTypeIDZone.def b/include/swift/AST/NameLookupTypeIDZone.def index f506dafb911f7..a4563fa875b3c 100644 --- a/include/swift/AST/NameLookupTypeIDZone.def +++ b/include/swift/AST/NameLookupTypeIDZone.def @@ -21,6 +21,9 @@ SWIFT_REQUEST(NameLookup, AnyObjectLookupRequest, SWIFT_REQUEST(NameLookup, CustomAttrNominalRequest, NominalTypeDecl *(CustomAttr *, DeclContext *), Cached, NoLocationInfo) +SWIFT_REQUEST(NameLookup, DirectLookupRequest, + TinyPtrVector(DirectLookupDescriptor), Uncached, + NoLocationInfo) SWIFT_REQUEST(NameLookup, ExpandASTScopeRequest, ast_scope::ASTScopeImpl* (ast_scope::ASTScopeImpl*, ast_scope::ScopeCreator*), SeparatelyCached, diff --git a/include/swift/Basic/Statistics.def b/include/swift/Basic/Statistics.def index 657568f3ea3a0..f5b4841bf49f7 100644 --- a/include/swift/Basic/Statistics.def +++ b/include/swift/Basic/Statistics.def @@ -240,9 +240,6 @@ FRONTEND_STATISTIC(Sema, NumLazyRequirementSignaturesLoaded) /// Number of lazy iterable declaration contexts constructed. FRONTEND_STATISTIC(Sema, NumLazyIterableDeclContexts) -/// Number of direct member-name lookups performed on nominal types. -FRONTEND_STATISTIC(Sema, NominalTypeLookupDirectCount) - /// Number of member-name lookups that avoided loading all members. FRONTEND_STATISTIC(Sema, NamedLazyMemberLoadSuccessCount) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 508787a5b7348..3236b64a50d5e 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -7917,6 +7917,15 @@ void swift::simple_display(llvm::raw_ostream &out, const Decl *decl) { } } +void swift::simple_display(llvm::raw_ostream &out, + OptionSet opts) { + out << "{ "; + using LookupFlags = NominalTypeDecl::LookupDirectFlags; + if (opts.contains(LookupFlags::IncludeAttrImplements)) + out << "IncludeAttrImplements"; + out << " }"; +} + void swift::simple_display(llvm::raw_ostream &out, const ValueDecl *decl) { if (decl) decl->dumpRef(out); else out << "(null)"; diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index d2d411a8160a3..20ec60bd8237c 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -1257,34 +1257,42 @@ maybeFilterOutAttrImplements(TinyPtrVector decls, TinyPtrVector NominalTypeDecl::lookupDirect(DeclName name, OptionSet flags) { - ASTContext &ctx = getASTContext(); - if (auto s = ctx.Stats) { - ++s->getFrontendCounters().NominalTypeLookupDirectCount; - } + return evaluateOrDefault(getASTContext().evaluator, + DirectLookupRequest({this, name, flags}), {}); +} + +llvm::Expected> +DirectLookupRequest::evaluate(Evaluator &evaluator, + DirectLookupDescriptor desc) const { + const auto &name = desc.Name; + const auto flags = desc.Options; + auto *decl = desc.DC; // We only use NamedLazyMemberLoading when a user opts-in and we have // not yet loaded all the members into the IDC list in the first place. + ASTContext &ctx = decl->getASTContext(); const bool useNamedLazyMemberLoading = (ctx.LangOpts.NamedLazyMemberLoading && - hasLazyMembers()); + decl->hasLazyMembers()); const bool includeAttrImplements = - flags.contains(LookupDirectFlags::IncludeAttrImplements); + flags.contains(NominalTypeDecl::LookupDirectFlags::IncludeAttrImplements); - LLVM_DEBUG(llvm::dbgs() << getNameStr() << ".lookupDirect(" + LLVM_DEBUG(llvm::dbgs() << decl->getNameStr() << ".lookupDirect(" << name << ")" - << ", hasLazyMembers()=" << hasLazyMembers() - << ", useNamedLazyMemberLoading=" << useNamedLazyMemberLoading + << ", hasLazyMembers()=" << decl->hasLazyMembers() + << ", useNamedLazyMemberLoading=" + << useNamedLazyMemberLoading << "\n"); - prepareLookupTable(); + decl->prepareLookupTable(); auto tryCacheLookup = - [=](MemberLookupTable *table, + [=](MemberLookupTable &table, DeclName name) -> Optional> { // Look for a declaration with this name. - auto known = table->find(name); - if (known == table->end()) { + auto known = table.find(name); + if (known == table.end()) { return None; } @@ -1293,36 +1301,36 @@ NominalTypeDecl::lookupDirect(DeclName name, includeAttrImplements); }; - auto updateLookupTable = [this](MemberLookupTable *table) { + auto updateLookupTable = [&decl](MemberLookupTable &table) { // Make sure we have the complete list of members (in this nominal and in // all extensions). - (void)getMembers(); + (void)decl->getMembers(); - for (auto E : getExtensions()) + for (auto E : decl->getExtensions()) (void)E->getMembers(); - LookupTable->updateLookupTable(this); + table.updateLookupTable(decl); }; + auto &Table = *decl->LookupTable; if (!useNamedLazyMemberLoading) { - updateLookupTable(LookupTable); - } else if (!LookupTable->isLazilyComplete(name.getBaseName())) { + updateLookupTable(Table); + } else if (!Table.isLazilyComplete(name.getBaseName())) { // The lookup table believes it doesn't have a complete accounting of this // name - either because we're never seen it before, or another extension // was registered since the last time we searched. Ask the loaders to give // us a hand. - auto &Table = *LookupTable; DeclBaseName baseName(name.getBaseName()); - if (populateLookupTableEntryFromLazyIDCLoader(ctx, Table, baseName, this)) { - updateLookupTable(LookupTable); + if (populateLookupTableEntryFromLazyIDCLoader(ctx, Table, baseName, decl)) { + updateLookupTable(Table); } else { - populateLookupTableEntryFromExtensions(ctx, Table, this, baseName); + populateLookupTableEntryFromExtensions(ctx, Table, decl, baseName); } Table.markLazilyComplete(baseName); } // Look for a declaration with this name. - return tryCacheLookup(LookupTable, name) + return tryCacheLookup(Table, name) .getValueOr(TinyPtrVector()); } diff --git a/lib/AST/NameLookupRequests.cpp b/lib/AST/NameLookupRequests.cpp index d6013f2c10bc0..0a48749b5e394 100644 --- a/lib/AST/NameLookupRequests.cpp +++ b/lib/AST/NameLookupRequests.cpp @@ -188,6 +188,24 @@ swift::extractNearestSourceLoc(const UnqualifiedLookupDescriptor &desc) { return extractNearestSourceLoc(desc.DC); } +//----------------------------------------------------------------------------// +// DirectLookupRequest computation. +//----------------------------------------------------------------------------// + +void swift::simple_display(llvm::raw_ostream &out, + const DirectLookupDescriptor &desc) { + out << "directly looking up "; + simple_display(out, desc.Name); + out << " on "; + simple_display(out, desc.DC); + out << " with options "; + simple_display(out, desc.Options); +} + +SourceLoc swift::extractNearestSourceLoc(const DirectLookupDescriptor &desc) { + return extractNearestSourceLoc(desc.DC); +} + // Define request evaluation functions for each of the name lookup requests. static AbstractRequestFunction *nameLookupRequestFunctions[] = { #define SWIFT_REQUEST(Zone, Name, Sig, Caching, LocOptions) \ From b09c9957ada4ae96e9758aa8ece87ec177f1c825 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 2 Jan 2020 10:50:07 -0800 Subject: [PATCH 2/5] Reintroduce NameLookupFlags::IgnoreNewExtensions Soft revert a09382c. It should now be safe to add this flag back as an optimization to specifically disable lazy member loading instead of all extension loading. Push the flag back everywhere it was needed, but also push it into lookup for associated type members which will never appear in extensions. --- include/swift/AST/Decl.h | 6 ++++++ include/swift/AST/NameLookupRequests.h | 4 ---- lib/AST/Decl.cpp | 3 ++- lib/AST/NameLookup.cpp | 15 ++++++++++----- lib/ClangImporter/ImportDecl.cpp | 6 ++++-- lib/Sema/TypeCheckProtocol.cpp | 7 +++++-- 6 files changed, 27 insertions(+), 14 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index e23e1f006f24f..789de8f2df163 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -3401,6 +3401,12 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext { /// Whether to include @_implements members. /// Used by conformance-checking to find special @_implements members. IncludeAttrImplements = 1 << 0, + /// Whether to avoid loading lazy members from any new extensions that would otherwise be found + /// by deserialization. + /// + /// Used by the module loader to break recursion and as an optimization e.g. when it is known that a + /// particular member declaration will never appear in an extension. + IgnoreNewExtensions = 1 << 1, }; /// Find all of the declarations with the given name within this nominal type diff --git a/include/swift/AST/NameLookupRequests.h b/include/swift/AST/NameLookupRequests.h index f59f13ccd9982..3094cf071c5df 100644 --- a/include/swift/AST/NameLookupRequests.h +++ b/include/swift/AST/NameLookupRequests.h @@ -513,10 +513,6 @@ class DirectLookupRequest // Evaluation. llvm::Expected> evaluate(Evaluator &evaluator, DirectLookupDescriptor desc) const; - -public: - // Cycle handling - void diagnoseCycle(DiagnosticEngine &diags) const; }; #define SWIFT_TYPEID_ZONE NameLookup diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 3236b64a50d5e..25f90c3d45002 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -4648,7 +4648,8 @@ ValueDecl *ProtocolDecl::getSingleRequirement(DeclName name) const { } AssociatedTypeDecl *ProtocolDecl::getAssociatedType(Identifier name) const { - auto results = const_cast(this)->lookupDirect(name); + const auto flags = NominalTypeDecl::LookupDirectFlags::IgnoreNewExtensions; + auto results = const_cast(this)->lookupDirect(name, flags); for (auto candidate : results) { if (candidate->getDeclContext() == this && isa(candidate)) { diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index 20ec60bd8237c..7b607b14a00a8 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -1273,7 +1273,8 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, ASTContext &ctx = decl->getASTContext(); const bool useNamedLazyMemberLoading = (ctx.LangOpts.NamedLazyMemberLoading && decl->hasLazyMembers()); - + const bool disableAdditionalExtensionLoading = + flags.contains(NominalTypeDecl::LookupDirectFlags::IgnoreNewExtensions); const bool includeAttrImplements = flags.contains(NominalTypeDecl::LookupDirectFlags::IncludeAttrImplements); @@ -1301,11 +1302,15 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, includeAttrImplements); }; - auto updateLookupTable = [&decl](MemberLookupTable &table) { + auto updateLookupTable = [&decl](MemberLookupTable &table, + bool noExtensions) { // Make sure we have the complete list of members (in this nominal and in // all extensions). (void)decl->getMembers(); + if (noExtensions) + return; + for (auto E : decl->getExtensions()) (void)E->getMembers(); @@ -1314,7 +1319,7 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, auto &Table = *decl->LookupTable; if (!useNamedLazyMemberLoading) { - updateLookupTable(Table); + updateLookupTable(Table, disableAdditionalExtensionLoading); } else if (!Table.isLazilyComplete(name.getBaseName())) { // The lookup table believes it doesn't have a complete accounting of this // name - either because we're never seen it before, or another extension @@ -1322,8 +1327,8 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, // us a hand. DeclBaseName baseName(name.getBaseName()); if (populateLookupTableEntryFromLazyIDCLoader(ctx, Table, baseName, decl)) { - updateLookupTable(Table); - } else { + updateLookupTable(Table, disableAdditionalExtensionLoading); + } else if (!disableAdditionalExtensionLoading) { populateLookupTableEntryFromExtensions(ctx, Table, decl, baseName); } Table.markLazilyComplete(baseName); diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 752c3fc9056e5..505eeb8e29c8c 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -767,8 +767,9 @@ static VarDecl *findAnonymousInnerFieldDecl(VarDecl *importedFieldDecl, auto anonymousFieldTypeDecl = anonymousFieldType->getStructOrBoundGenericStruct(); + const auto flags = NominalTypeDecl::LookupDirectFlags::IgnoreNewExtensions; for (auto decl : anonymousFieldTypeDecl->lookupDirect( - importedFieldDecl->getName())) { + importedFieldDecl->getName(), flags)) { if (isa(decl)) { return cast(decl); } @@ -8303,7 +8304,8 @@ synthesizeConstantGetterBody(AbstractFunctionDecl *afd, void *voidContext) { DeclName initName = DeclName(ctx, DeclBaseName::createConstructor(), { ctx.Id_rawValue }); auto nominal = type->getAnyNominal(); - for (auto found : nominal->lookupDirect(initName)) { + const auto flags = NominalTypeDecl::LookupDirectFlags::IgnoreNewExtensions; + for (auto found : nominal->lookupDirect(initName, flags)) { init = dyn_cast(found); if (init && init->getDeclContext() == nominal) break; diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 0654b5da52786..c292bbbe6d2ce 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -5143,8 +5143,10 @@ void TypeChecker::checkConformancesInContext(DeclContext *dc, continue; bool valueIsType = isa(value); + const auto flags = + NominalTypeDecl::LookupDirectFlags::IgnoreNewExtensions; for (auto requirement - : diag.Protocol->lookupDirect(value->getFullName())) { + : diag.Protocol->lookupDirect(value->getFullName(), flags)) { if (requirement->getDeclContext() != diag.Protocol) continue; @@ -5345,7 +5347,8 @@ swift::findWitnessedObjCRequirements(const ValueDecl *witness, if (!proto->isObjC()) continue; Optional conformance; - for (auto req : proto->lookupDirect(name)) { + const auto flags = NominalTypeDecl::LookupDirectFlags::IgnoreNewExtensions; + for (auto req : proto->lookupDirect(name, flags)) { // Skip anything in a protocol extension. if (req->getDeclContext() != proto) continue; From e626cfb3785f42ee185fb5b14b17ec5600e2a761 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 2 Jan 2020 11:12:51 -0800 Subject: [PATCH 3/5] Remove lazy member loading re-entrancy guards Effectively revert #28907. The request evaluator will also catch re-entrancy here, and those cycles can be broken with NameLookupFlags::IgnoreNewExtensions. --- include/swift/AST/DeclContext.h | 26 ++------------------------ lib/AST/DeclContext.cpp | 10 ++++------ lib/AST/NameLookup.cpp | 4 ---- 3 files changed, 6 insertions(+), 34 deletions(-) diff --git a/include/swift/AST/DeclContext.h b/include/swift/AST/DeclContext.h index abeaca1225464..246467e8481e3 100644 --- a/include/swift/AST/DeclContext.h +++ b/include/swift/AST/DeclContext.h @@ -681,18 +681,9 @@ enum class IterableDeclContextKind : uint8_t { /// Note that an iterable declaration context must inherit from both /// \c IterableDeclContext and \c DeclContext. class IterableDeclContext { - enum LazyMembers : unsigned { - Present = 1 << 0, - - /// Lazy member loading has a variety of feedback loops that need to - /// switch to pseudo-empty-member behaviour to avoid infinite recursion; - /// we use this flag to control them. - InProgress = 1 << 1, - }; - /// The first declaration in this context along with a bit indicating whether /// the members of this context will be lazily produced. - mutable llvm::PointerIntPair FirstDeclAndLazyMembers; + mutable llvm::PointerIntPair FirstDeclAndLazyMembers; /// The last declaration in this context, used for efficient insertion, /// along with the kind of iterable declaration context. @@ -780,20 +771,7 @@ class IterableDeclContext { /// Check whether there are lazily-loaded members. bool hasLazyMembers() const { - return FirstDeclAndLazyMembers.getInt() & LazyMembers::Present; - } - - bool isLoadingLazyMembers() { - return FirstDeclAndLazyMembers.getInt() & LazyMembers::InProgress; - } - - void setLoadingLazyMembers(bool inProgress) { - LazyMembers status = FirstDeclAndLazyMembers.getInt(); - if (inProgress) - status = LazyMembers(status | LazyMembers::InProgress); - else - status = LazyMembers(status & ~LazyMembers::InProgress); - FirstDeclAndLazyMembers.setInt(status); + return FirstDeclAndLazyMembers.getInt(); } /// Setup the loader for lazily-loaded members. diff --git a/lib/AST/DeclContext.cpp b/lib/AST/DeclContext.cpp index 38ab0d57cb391..5b3da15aa0ba7 100644 --- a/lib/AST/DeclContext.cpp +++ b/lib/AST/DeclContext.cpp @@ -798,8 +798,7 @@ void IterableDeclContext::setMemberLoader(LazyMemberLoader *loader, ASTContext &ctx = getASTContext(); auto contextInfo = ctx.getOrCreateLazyIterableContextData(this, loader); - auto lazyMembers = FirstDeclAndLazyMembers.getInt() | LazyMembers::Present; - FirstDeclAndLazyMembers.setInt(LazyMembers(lazyMembers)); + FirstDeclAndLazyMembers.setInt(true); contextInfo->memberData = contextData; ++NumLazyIterableDeclContexts; @@ -855,12 +854,11 @@ void IterableDeclContext::loadAllMembers() const { return; // Don't try to load all members re-entrant-ly. - auto contextInfo = ctx.getOrCreateLazyIterableContextData(this, - /*lazyLoader=*/nullptr); - auto lazyMembers = FirstDeclAndLazyMembers.getInt() & ~LazyMembers::Present; - FirstDeclAndLazyMembers.setInt(LazyMembers(lazyMembers)); + FirstDeclAndLazyMembers.setInt(false); const Decl *container = getDecl(); + auto contextInfo = ctx.getOrCreateLazyIterableContextData(this, + /*lazyLoader=*/nullptr); contextInfo->loader->loadAllMembers(const_cast(container), contextInfo->memberData); diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index 7b607b14a00a8..f653c44fd437b 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -1148,11 +1148,9 @@ populateLookupTableEntryFromLazyIDCLoader(ASTContext &ctx, MemberLookupTable &LookupTable, DeclBaseName name, IterableDeclContext *IDC) { - IDC->setLoadingLazyMembers(true); auto ci = ctx.getOrCreateLazyIterableContextData(IDC, /*lazyLoader=*/nullptr); if (auto res = ci->loader->loadNamedMembers(IDC, name, ci->memberData)) { - IDC->setLoadingLazyMembers(false); if (auto s = ctx.Stats) { ++s->getFrontendCounters().NamedLazyMemberLoadSuccessCount; } @@ -1161,7 +1159,6 @@ populateLookupTableEntryFromLazyIDCLoader(ASTContext &ctx, } return false; } else { - IDC->setLoadingLazyMembers(false); if (auto s = ctx.Stats) { ++s->getFrontendCounters().NamedLazyMemberLoadFailureCount; } @@ -1285,7 +1282,6 @@ DirectLookupRequest::evaluate(Evaluator &evaluator, << useNamedLazyMemberLoading << "\n"); - decl->prepareLookupTable(); auto tryCacheLookup = From d1823ebec8dcb995bc4e5dafa820933baa8b4197 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 23 Jan 2020 18:40:28 -0800 Subject: [PATCH 4/5] [NFC] Make prepareLookupTable lazier about loading Extension members Use the same laziest-possible extension member loading path for everything. --- lib/AST/NameLookup.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index f653c44fd437b..3e9797008ff24 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -1207,25 +1207,21 @@ void NominalTypeDecl::prepareLookupTable() { if (hasLazyMembers()) { assert(!hasUnparsedMembers()); - - // Lazy members: if the table needs population, populate the table _only - // from those members already in the IDC member list_ such as implicits or - // globals-as-members. LookupTable->addMembers(getCurrentMembersWithoutLoading()); - for (auto e : getExtensions()) { - // If we can lazy-load this extension, only take the members we've loaded - // so far. - if (e->wasDeserialized() || e->hasClangNode()) { - LookupTable->addMembers(e->getCurrentMembersWithoutLoading()); - continue; - } - - // Else, load all the members into the table. - LookupTable->addMembers(e->getMembers()); - } } else { LookupTable->addMembers(getMembers()); - LookupTable->updateLookupTable(this); + } + + for (auto e : getExtensions()) { + // If we can lazy-load this extension, only take the members we've loaded + // so far. + if (e->wasDeserialized() || e->hasClangNode()) { + LookupTable->addMembers(e->getCurrentMembersWithoutLoading()); + continue; + } + + // Else, load all the members into the table. + LookupTable->addMembers(e->getMembers()); } } From f690a9e18714ae6d3fe04f82db126991f18172db Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Fri, 24 Jan 2020 12:54:41 -0800 Subject: [PATCH 5/5] Also search nested types tables in overlay modules --- lib/Serialization/Deserialization.cpp | 35 +++++++++++++++++++++------ 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 64ab317c826ab..4c62bb3728b1a 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -1207,6 +1207,21 @@ static void filterValues(Type expectedTy, ModuleDecl *expectedModule, values.erase(newEnd, values.end()); } +static TypeDecl * +findNestedTypeDeclInModule(FileUnit *thisFile, ModuleDecl *extensionModule, + Identifier name, NominalTypeDecl *parent) { + assert(extensionModule && "NULL is not a valid module"); + for (FileUnit *file : extensionModule->getFiles()) { + if (file == thisFile) + continue; + + if (auto nestedType = file->lookupNestedType(name, parent)) { + return nestedType; + } + } + return nullptr; +} + Expected ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) { using namespace decls_block; @@ -1442,13 +1457,19 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) { // Fault in extensions, then ask every file in the module. (void)baseType->getExtensions(); - TypeDecl *nestedType = nullptr; - for (FileUnit *file : extensionModule->getFiles()) { - if (file == getFile()) - continue; - nestedType = file->lookupNestedType(memberName, baseType); - if (nestedType) - break; + auto *nestedType = + findNestedTypeDeclInModule(getFile(), extensionModule, + memberName, baseType); + + // For clang module units, also search tables in the overlays. + if (!nestedType) { + if (auto LF = + dyn_cast(baseType->getModuleScopeContext())) { + if (auto overlayModule = LF->getOverlayModule()) { + nestedType = findNestedTypeDeclInModule(getFile(), overlayModule, + memberName, baseType); + } + } } if (nestedType) {