Skip to content

[Member name lookup] Eliminate non-lazy member loading. #66266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,6 @@ namespace swift {
/// Flags for developers
///

/// Enable named lazy member loading.
bool NamedLazyMemberLoading = true;

/// Whether to record request references for incremental builds.
bool RecordRequestReferences = true;

Expand Down
19 changes: 10 additions & 9 deletions include/swift/SIL/SILBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -413,18 +413,18 @@ class SILBuilder {
AllocStackInst *createAllocStack(SILLocation Loc, SILType elementType,
Optional<SILDebugVariable> Var = None,
bool hasDynamicLifetime = false,
bool isLexical = false, bool wasMoved = false
#ifndef NDEBUG
,
bool isLexical = false,
bool wasMoved = false,
bool skipVarDeclAssert = false
#endif
) {
llvm::SmallString<4> Name;
Loc.markAsPrologue();
#ifndef NDEBUG
if (dyn_cast_or_null<VarDecl>(Loc.getAsASTNode<Decl>()))
assert((skipVarDeclAssert || Loc.isSynthesizedAST() || Var) &&
"location is a VarDecl, but SILDebugVariable is empty");
#else
(void)skipVarDeclAssert;
#endif
return insert(AllocStackInst::create(
getSILDebugLocation(Loc, true), elementType, getFunction(),
Expand Down Expand Up @@ -473,19 +473,20 @@ class SILBuilder {
return createAllocBox(loc, SILBoxType::get(fieldType.getASTType()), Var,
hasDynamicLifetime, reflection,
usesMoveableValueDebugInfo,
/*skipVarDeclAssert*/ false, hasPointerEscape);
/*skipVarDeclAssert*/ false,
hasPointerEscape);
}

AllocBoxInst *createAllocBox(SILLocation Loc, CanSILBoxType BoxType,
Optional<SILDebugVariable> Var = None,
bool hasDynamicLifetime = false,
bool reflection = false,
bool usesMoveableValueDebugInfo = false
#ifndef NDEBUG
,
bool usesMoveableValueDebugInfo = false,
bool skipVarDeclAssert = false,
#endif
bool hasPointerEscape = false) {
#if NDEBUG
(void)skipVarDeclAssert;
#endif
llvm::SmallString<4> Name;
Loc.markAsPrologue();
assert((skipVarDeclAssert ||
Expand Down
37 changes: 19 additions & 18 deletions lib/AST/NameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1218,6 +1218,9 @@ class swift::MemberLookupTable : public ASTAllocated<swift::MemberLookupTable> {
/// Lookup table mapping names to the set of declarations with that name.
LookupTable Lookup;

/// List of containers that have lazily-loaded members
llvm::SmallVector<ExtensionDecl *, 2> ExtensionsWithLazyMembers;

/// The set of names of lazily-loaded members that the lookup table has a
/// complete accounting of with respect to all known extensions of its
/// parent nominal type.
Expand Down Expand Up @@ -1246,6 +1249,14 @@ class swift::MemberLookupTable : public ASTAllocated<swift::MemberLookupTable> {
/// Add the given members to the lookup table.
void addMembers(DeclRange members);

void addExtensionWithLazyMembers(ExtensionDecl *ext) {
ExtensionsWithLazyMembers.push_back(ext);
}

ArrayRef<ExtensionDecl *> getExtensionsWithLazyMembers() const {
return ExtensionsWithLazyMembers;
}

/// Returns \c true if the lookup table has a complete accounting of the
/// given name.
bool isLazilyComplete(DeclBaseName name) const {
Expand Down Expand Up @@ -1435,10 +1446,11 @@ void NominalTypeDecl::addedExtension(ExtensionDecl *ext) {
auto *table = LookupTable.getPointer();
assert(table);

if (ext->hasLazyMembers()) {
if (ext->wasDeserialized() || ext->hasClangNode()) {
table->addMembers(ext->getCurrentMembersWithoutLoading());
table->clearLazilyCompleteCache();
table->clearLazilyCompleteForMacroExpansionCache();
table->addExtensionWithLazyMembers(ext);
} else {
table->addMembers(ext->getMembers());
}
Expand Down Expand Up @@ -1534,6 +1546,9 @@ populateLookupTableEntryFromLazyIDCLoader(ASTContext &ctx,
MemberLookupTable &LookupTable,
DeclBaseName name,
IterableDeclContext *IDC) {
if (!IDC->hasLazyMembers())
return;

auto ci = ctx.getOrCreateLazyIterableContextData(IDC,
/*lazyLoader=*/nullptr);
auto res = ci->loader->loadNamedMembers(IDC, name, ci->memberData);
Expand All @@ -1553,7 +1568,7 @@ populateLookupTableEntryFromExtensions(ASTContext &ctx,
assert(!table.isLazilyComplete(name) &&
"Should not be searching extensions for complete name!");

for (auto e : nominal->getExtensions()) {
for (auto e : table.getExtensionsWithLazyMembers()) {
// If there's no lazy members to look at, all the members of this extension
// are present in the lookup table.
if (!e->hasLazyMembers()) {
Expand Down Expand Up @@ -1789,6 +1804,7 @@ void NominalTypeDecl::prepareLookupTable() {
// LazyMemberLoader::loadNamedMembers().
if (e->wasDeserialized() || e->hasClangNode()) {
table->addMembers(e->getCurrentMembersWithoutLoading());
table->addExtensionWithLazyMembers(e);
continue;
}

Expand Down Expand Up @@ -1848,21 +1864,14 @@ DirectLookupRequest::evaluate(Evaluator &evaluator,
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 &&
decl->hasLazyMembers());
const bool includeAttrImplements =
flags.contains(NominalTypeDecl::LookupDirectFlags::IncludeAttrImplements);
const bool excludeMacroExpansions =
flags.contains(NominalTypeDecl::LookupDirectFlags::ExcludeMacroExpansions);

LLVM_DEBUG(llvm::dbgs() << decl->getNameStr() << ".lookupDirect("
<< name << ")"
<< ", hasLazyMembers()=" << decl->hasLazyMembers()
<< ", useNamedLazyMemberLoading="
<< useNamedLazyMemberLoading
<< ", excludeMacroExpansions="
<< excludeMacroExpansions
<< "\n");
Expand All @@ -1875,15 +1884,7 @@ DirectLookupRequest::evaluate(Evaluator &evaluator,
decl->prepareExtensions();

auto &Table = *decl->getLookupTable();
if (!useNamedLazyMemberLoading) {
// Make sure we have the complete list of members (in this nominal and in
// all extensions).
(void)decl->getMembers();

for (auto E : decl->getExtensions())
(void)E->getMembers();

} else if (!Table.isLazilyComplete(name.getBaseName())) {
if (!Table.isLazilyComplete(name.getBaseName())) {
DeclBaseName baseName(name.getBaseName());

if (isa_and_nonnull<clang::NamespaceDecl>(decl->getClangDecl())) {
Expand Down
20 changes: 14 additions & 6 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5006,7 +5006,8 @@ DeclAttributes cloneImportedAttributes(ValueDecl *decl, ASTContext &context) {
return attrs;
}

ValueDecl *cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
static ValueDecl *
cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) {
if (auto fn = dyn_cast<FuncDecl>(decl)) {
// TODO: function templates are specialized during type checking so to
// support these we need to tell Swift to type check the synthesized bodies.
Expand Down Expand Up @@ -6290,16 +6291,23 @@ Decl *ClangImporter::importDeclDirectly(const clang::NamedDecl *decl) {
return Impl.importDecl(decl, Impl.CurrentVersion);
}

ValueDecl *ClangImporter::importBaseMemberDecl(ValueDecl *decl,
DeclContext *newContext) {
ValueDecl *ClangImporter::Implementation::importBaseMemberDecl(
ValueDecl *decl, DeclContext *newContext) {
// Make sure we don't clone the decl again for this class, as that would
// result in multiple definitions of the same symbol.
std::pair<ValueDecl *, DeclContext *> key = {decl, newContext};
if (!Impl.clonedBaseMembers.count(key)) {
auto known = clonedBaseMembers.find(key);
if (known == clonedBaseMembers.end()) {
ValueDecl *cloned = cloneBaseMemberDecl(decl, newContext);
Impl.clonedBaseMembers[key] = cloned;
known = clonedBaseMembers.insert({key, cloned}).first;
}
return Impl.clonedBaseMembers[key];

return known->second;
}

ValueDecl *ClangImporter::importBaseMemberDecl(ValueDecl *decl,
DeclContext *newContext) {
return Impl.importBaseMemberDecl(decl, newContext);
}

void ClangImporter::diagnoseTopLevelValue(const DeclName &name) {
Expand Down
10 changes: 7 additions & 3 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3581,6 +3581,12 @@ namespace {
if (!dc)
return nullptr;

// While importing the DeclContext, we might have imported the decl
// itself.
auto known = Impl.importDeclCached(decl, getVersion());
if (known.has_value())
return known.value();

// TODO: do we want to emit a diagnostic here?
// Types that are marked as foreign references cannot be stored by value.
if (auto recordType =
Expand Down Expand Up @@ -8761,8 +8767,6 @@ static void loadAllMembersOfSuperclassIfNeeded(ClassDecl *CD) {
E->loadAllMembers();
}

ValueDecl *cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext);

void ClangImporter::Implementation::loadAllMembersOfRecordDecl(
NominalTypeDecl *swiftDecl, const clang::RecordDecl *clangRecord) {
// Import all of the members.
Expand Down Expand Up @@ -8793,7 +8797,7 @@ void ClangImporter::Implementation::loadAllMembersOfRecordDecl(
// This means we found a member in a C++ record's base class.
if (swiftDecl->getClangDecl() != clangRecord) {
// So we need to clone the member into the derived class.
if (auto newDecl = cloneBaseMemberDecl(cast<ValueDecl>(member), swiftDecl)) {
if (auto newDecl = importBaseMemberDecl(cast<ValueDecl>(member), swiftDecl)) {
swiftDecl->addMember(newDecl);
}
continue;
Expand Down
4 changes: 4 additions & 0 deletions lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -642,10 +642,14 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
llvm::MapVector<std::pair<NominalTypeDecl *, Type>,
std::pair<FuncDecl *, FuncDecl *>> cxxSubscripts;

private:
// Keep track of the decls that were already cloned for this specific class.
llvm::DenseMap<std::pair<ValueDecl *, DeclContext *>, ValueDecl *>
clonedBaseMembers;

ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext);

public:
// Cache for already-specialized function templates and any thunks they may
// have.
llvm::DenseMap<clang::FunctionDecl *, ValueDecl *>
Expand Down
2 changes: 0 additions & 2 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -682,8 +682,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
/*default*/ false);
Opts.UseClangFunctionTypes |= Args.hasArg(OPT_use_clang_function_types);

Opts.NamedLazyMemberLoading &= !Args.hasArg(OPT_disable_named_lazy_member_loading);

if (Args.hasArg(OPT_emit_fine_grained_dependency_sourcefile_dot_files))
Opts.EmitFineGrainedDependencySourcefileDotFiles = true;

Expand Down
2 changes: 1 addition & 1 deletion test/ClangImporter/attr-swift_name-errors.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -typecheck %s -disable-named-lazy-member-loading \
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -typecheck %s \
// RUN: -verify -verify-additional-file %S/Inputs/custom-modules/ConflictingNames.h -verify-ignore-unknown

// REQUIRES: objc_interop
Expand Down
8 changes: 1 addition & 7 deletions test/ClangImporter/attr-swift_name.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %empty-directory(%t.mcp)
// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -Xcc -w -typecheck %s -module-cache-path %t.mcp -disable-named-lazy-member-loading 2>&1 | %FileCheck %s
// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -Xcc -w -typecheck %s -module-cache-path %t.mcp 2>&1 | %FileCheck %s

// REQUIRES: objc_interop

Expand All @@ -26,12 +26,6 @@ func test(_ i: Int) {
// CHECK: note: please report this issue to the owners of 'ObjCIRExtras'
// CHECK-NOT: warning:

// CHECK: warning: too few parameters in swift_name attribute (expected 2; got 1)
// CHECK: + (instancetype)testW:(id)x out:(id *)outObject SWIFT_NAME(ww(_:));
// CHECK-NOT: warning:
// CHECK: note: please report this issue to the owners of 'ObjCIRExtras'
// CHECK-NOT: warning:

// CHECK: warning: cycle detected while resolving 'CircularName' in swift_name attribute for 'CircularName'
// CHECK: SWIFT_NAME(CircularName.Inner) @interface CircularName : NSObject @end
// CHECK-NOT: {{warning|note}}:
Expand Down
20 changes: 0 additions & 20 deletions test/NameLookup/named_lazy_member_loading_objc_category.swift

This file was deleted.

32 changes: 0 additions & 32 deletions test/NameLookup/named_lazy_member_loading_objc_interface.swift

This file was deleted.

23 changes: 0 additions & 23 deletions test/NameLookup/named_lazy_member_loading_objc_protocol.swift

This file was deleted.

Loading