Skip to content

[Serialization] Fast lookup of nested types in modules with overlays #20024

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 1 commit into from
Oct 25, 2018
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
20 changes: 11 additions & 9 deletions lib/Serialization/Deserialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ STATISTIC(NumNormalProtocolConformancesLoaded,
STATISTIC(NumNormalProtocolConformancesCompleted,
"# of normal protocol conformances completed");
STATISTIC(NumNestedTypeShortcuts,
"# of same-module nested types resolved without lookup");
"# of nested types resolved without full lookup");

using namespace swift;
using namespace swift::serialization;
Expand Down Expand Up @@ -1444,10 +1444,6 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) {
if (!extensionModule)
extensionModule = baseType->getModuleContext();

// FIXME: If 'importedFromClang' is true but 'extensionModule' is an
// overlay module, the search below will fail and we'll fall back to
// the slow path.

// Fault in extensions, then ask every file in the module.
(void)baseType->getExtensions();
TypeDecl *nestedType = nullptr;
Expand All @@ -1460,10 +1456,16 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) {
}

if (nestedType) {
values.clear();
values.push_back(nestedType);
++NumNestedTypeShortcuts;
break;
SmallVector<ValueDecl *, 1> singleValueBuffer{nestedType};
filterValues(/*expectedTy*/Type(), extensionModule, genericSig,
/*isType*/true, /*inProtocolExt*/false,
importedFromClang, /*isStatic*/false, /*ctorInit*/None,
singleValueBuffer);
if (!singleValueBuffer.empty()) {
values.assign({nestedType});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, why assign a 1-element initializer list vs. calling push_back?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A serialization cross-reference is a chain of nested names; when resolving component N, values contains the results from resolving component N-1.* So here we're saying we've resolved component N to a single result, and are replacing the results from component N-1.

* It's not always actually the immediately previous component; it's the previous component that represents a type. But, close enough.

++NumNestedTypeShortcuts;
break;
}
}

pathTrace.removeLast();
Expand Down
36 changes: 20 additions & 16 deletions lib/Serialization/ModuleFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1575,25 +1575,29 @@ TypeDecl *ModuleFile::lookupNestedType(Identifier name,
const NominalTypeDecl *parent) {
PrettyStackTraceModuleFile stackEntry(*this);

if (!NestedTypeDecls)
return nullptr;
if (NestedTypeDecls) {
auto iter = NestedTypeDecls->find(name);
if (iter != NestedTypeDecls->end()) {
for (std::pair<DeclID, DeclID> entry : *iter) {
assert(entry.first);
auto declOrOffset = Decls[entry.first - 1];
if (!declOrOffset.isComplete())
continue;

auto iter = NestedTypeDecls->find(name);
if (iter == NestedTypeDecls->end())
return nullptr;
Decl *decl = declOrOffset;
if (decl != parent)
continue;
return cast<TypeDecl>(getDecl(entry.second));
}
}
}

auto data = *iter;
for (std::pair<DeclID, DeclID> entry : data) {
assert(entry.first);
auto declOrOffset = Decls[entry.first - 1];
if (!declOrOffset.isComplete())
continue;
if (!ShadowedModule)
return nullptr;

Decl *decl = declOrOffset;
if (decl != parent)
continue;
return cast<TypeDecl>(getDecl(entry.second));
}
for (FileUnit *file : ShadowedModule->getFiles())
if (auto *nestedType = file->lookupNestedType(name, parent))
return nestedType;

return nullptr;
}
Expand Down
13 changes: 13 additions & 0 deletions test/Serialization/Inputs/nested-type-with-overlay/HasOverlay.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
struct Base {
int dummy;
};

struct Nested {
int dummy;
} __attribute((swift_name("Base.NestedFromClang")));

struct NestedAndShadowed {
int dummy;
} __attribute((swift_name("Base.NestedAndShadowed")));

struct NestedAndShadowed getShadowedFromClang();
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module HasOverlay {
header "HasOverlay.h"
}
10 changes: 10 additions & 0 deletions test/Serialization/Inputs/nested-type-with-overlay/overlay.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
@_exported import HasOverlay

extension Base {
public struct NestedFromSwift {}
public struct NestedAndShadowed {
init(dummy: ()) {}
}
}

public var shadowedFromSwift = Base.NestedAndShadowed(dummy: ())
12 changes: 12 additions & 0 deletions test/Serialization/Inputs/nested-type-with-overlay/verify.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import main
import HasOverlay

func test() {
var a = getShadowedFromClang()
a = main.shadowedFromClang // okay
a = HasOverlay.shadowedFromSwift // expected-error {{cannot assign}}

var b = HasOverlay.shadowedFromSwift
b = main.shadowedFromSwift // okay
b = getShadowedFromClang() // expected-error {{cannot assign}}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// REQUIRES: asserts

// CHECK: Statistics
// CHECK: 1 Serialization - # of same-module nested types resolved without lookup
// CHECK: 1 Serialization - # of nested types resolved without full lookup

// Note the Optional here and below; this was once necessary to produce a crash.
// Without it, the type of the parameter is initialized "early" enough to not
Expand Down
2 changes: 1 addition & 1 deletion test/Serialization/multi-file-nested-type-extension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// REQUIRES: asserts

// CHECK: Statistics
// CHECK: 1 Serialization - # of same-module nested types resolved without lookup
// CHECK: 1 Serialization - # of nested types resolved without full lookup

// Note the Optional here and below; this was once necessary to produce a crash.
// Without it, the type of the parameter is initialized "early" enough to not
Expand Down
4 changes: 2 additions & 2 deletions test/Serialization/multi-file-nested-type-simple.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@

// REQUIRES: asserts

// CHECK: 4 Serialization - # of same-module nested types resolved without lookup
// CHECK: 4 Serialization - # of nested types resolved without full lookup
// DISABLED: Statistics
// DISABLED-NOT: same-module nested types resolved without lookup
// DISABLED-NOT: nested types resolved without full lookup

public func useTypes(
_: Outer.Inner,
Expand Down
22 changes: 22 additions & 0 deletions test/Serialization/nested-type-with-overlay.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend -emit-module -o %t/HasOverlay.swiftmodule %S/Inputs/nested-type-with-overlay/overlay.swift -I %S/Inputs/nested-type-with-overlay -module-name HasOverlay

// RUN: %target-swift-frontend -emit-module -o %t/main~partial.swiftmodule -primary-file %s -module-name main -I %t -I %S/Inputs/nested-type-helper
// RUN: %target-swift-frontend -merge-modules -emit-module -o %t/main.swiftmodule %t/main~partial.swiftmodule -print-stats -module-name main -I %t -I %S/Inputs/nested-type-with-overlay 2>&1 | %FileCheck %s

// RUN: %target-swift-frontend -emit-silgen %S/Inputs/nested-type-with-overlay/verify.swift -I %t -I %S/Inputs/nested-type-with-overlay -verify

// REQUIRES: asserts

// CHECK: 3 Serialization - # of nested types resolved without full lookup
// Unfortunately this isn't 4 because of the shadowed nested type from Clang.

import HasOverlay

public func resolveNestedTypes(
_: Base.NestedFromClang,
_: Base.NestedFromSwift
) {}

public var shadowedFromClang = getShadowedFromClang()
public var shadowedFromSwift = HasOverlay.shadowedFromSwift