Skip to content

[Serialization] Encode depth for cross-refs to generic parameters #20091

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 2 commits into from
Nov 1, 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
5 changes: 3 additions & 2 deletions include/swift/Serialization/ModuleFormat.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
/// describe what change you made. The content of this comment isn't important;
/// it just ensures a conflict if two people change the module format.
/// Don't worry about adhering to the 80-column limit for this line.
const uint16_t SWIFTMODULE_VERSION_MINOR = 455; // Last change: reorder block IDs
const uint16_t SWIFTMODULE_VERSION_MINOR = 456; // Last change: encode depth in generic param XREFs

using DeclIDField = BCFixed<31>;

Expand Down Expand Up @@ -1403,7 +1403,8 @@ namespace decls_block {

using XRefGenericParamPathPieceLayout = BCRecordLayout<
XREF_GENERIC_PARAM_PATH_PIECE,
BCVBR<5> // index
BCVBR<5>, // depth
BCVBR<5> // index
>;

using SILGenNameDeclAttrLayout = BCRecordLayout<
Expand Down
18 changes: 15 additions & 3 deletions lib/Serialization/Deserialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1622,8 +1622,8 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) {
getXRefDeclNameForError());
}

uint32_t paramIndex;
XRefGenericParamPathPieceLayout::readRecord(scratch, paramIndex);
uint32_t depth, paramIndex;
XRefGenericParamPathPieceLayout::readRecord(scratch, depth, paramIndex);

pathTrace.addGenericParam(paramIndex);

Expand All @@ -1645,7 +1645,7 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) {
assert(paramList && "Couldn't find constrained extension");
} else {
// Simple case: use the nominal type's generic parameters.
paramList = nominal->getGenericParams();
paramList = nominal->getGenericParamsOfContext();
}
} else if (auto alias = dyn_cast<TypeAliasDecl>(base)) {
paramList = alias->getGenericParams();
Expand All @@ -1660,6 +1660,18 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) {
"cross-reference to generic param for non-generic type",
pathTrace, getXRefDeclNameForError());
}

unsigned currentDepth = paramList->getDepth();
if (currentDepth < depth) {
return llvm::make_error<XRefError>(
"a containing type has been made non-generic",
pathTrace, getXRefDeclNameForError());
}
while (currentDepth > depth) {
paramList = paramList->getOuterParameters();
--currentDepth;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth asserting that the GenericTypeParamDecl you find below has the right depth and index?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That seems like a good idea for the AST verifier in general. It's set up by construction that the depth matches the GenericParamList's notion of depth, but the GenericParamList and the GenericTypeParamDecl could be out of sync. I'll do that in a follow-up PR.

}

if (paramIndex >= paramList->size()) {
return llvm::make_error<XRefError>(
"generic argument index out of bounds",
Expand Down
1 change: 1 addition & 0 deletions lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2026,6 +2026,7 @@ void Serializer::writeCrossReference(const Decl *D) {
"Cannot cross reference a generic type decl at module scope.");
abbrCode = DeclTypeAbbrCodes[XRefGenericParamPathPieceLayout::Code];
XRefGenericParamPathPieceLayout::emitRecord(Out, ScratchRecord, abbrCode,
genericParam->getDepth(),
genericParam->getIndex());
return;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
public struct OuterNonGeneric {}
extension OuterNonGeneric {
public struct InnerNonGeneric {}
public struct InnerGeneric<Y1, Y2> {}
}

public struct OuterGeneric<X1, X2> {}
extension OuterGeneric {
public struct InnerNonGeneric {}
public struct InnerGeneric<Y1, Y2> {}
}


extension OuterNonGeneric.InnerNonGeneric {
public typealias AliasTy = ()
}

extension OuterNonGeneric.InnerGeneric where Y1: Equatable {
public typealias AliasTy = (Y1, Y2)
}

extension OuterGeneric.InnerNonGeneric where X1: Equatable {
public typealias AliasTy = (X1, X2)
}

extension OuterGeneric.InnerGeneric where X1: Equatable, Y1: Equatable {
public typealias AliasTy = (X1, X2, Y1, Y2)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
public struct OuterNonGeneric {}
extension OuterNonGeneric {
public struct InnerNonGeneric {
public typealias AliasTy = ()
}
public struct InnerGeneric<Y1, Y2> {
public typealias AliasTy = (Y1, Y2)
}
}

public struct OuterGeneric<X1, X2> {}
extension OuterGeneric {
public struct InnerNonGeneric {
public typealias AliasTy = (X1, X2)
}
public struct InnerGeneric<Y1, Y2> {
public typealias AliasTy = (X1, X2, Y1, Y2)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
public struct OuterNonGeneric {}
extension OuterNonGeneric {
public struct InnerNonGeneric {}
public struct InnerGeneric<Y1, Y2> {}
}

public struct OuterGeneric<X1, X2> {}
extension OuterGeneric {
public struct InnerNonGeneric {}
public struct InnerGeneric<Y1, Y2> {}
}


extension OuterNonGeneric.InnerNonGeneric {
public typealias AliasTy = ()
}

extension OuterNonGeneric.InnerGeneric {
public typealias AliasTy = (Y1, Y2)
}

extension OuterGeneric.InnerNonGeneric {
public typealias AliasTy = (X1, X2)
}

extension OuterGeneric.InnerGeneric {
public typealias AliasTy = (X1, X2, Y1, Y2)
}
17 changes: 17 additions & 0 deletions test/Serialization/Inputs/xref-generic-params-other.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
public struct OuterNonGeneric {
public struct InnerNonGeneric {
public typealias AliasTy = ()
}
public struct InnerGeneric<Y1, Y2> {
public typealias AliasTy = (Y1, Y2)
}
}

public struct OuterGeneric<X1, X2> {
public struct InnerNonGeneric {
public typealias AliasTy = (X1, X2)
}
public struct InnerGeneric<Y1, Y2> {
public typealias AliasTy = (X1, X2, Y1, Y2)
}
}
40 changes: 40 additions & 0 deletions test/Serialization/xref-generic-params.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// RUN: %empty-directory(%t)

// Run the same test several times, providing the nested types a different way
// each time.

// RUN: %target-swift-frontend -emit-module -o %t/a.swiftmodule -primary-file %s %S/Inputs/xref-generic-params-other.swift -module-name main
// RUN: %target-swift-frontend -emit-module -o %t/b.swiftmodule %s -primary-file %S/Inputs/xref-generic-params-other.swift -module-name main
// RUN: %target-swift-frontend -merge-modules -emit-module -o %t/main.swiftmodule %t/a.swiftmodule %t/b.swiftmodule -module-name main
// RUN: %target-swift-ide-test -print-module -module-to-print=main -I %t -source-filename=x | %FileCheck %s

// RUN: %target-swift-frontend -emit-module -o %t/a-extensions.swiftmodule -primary-file %s %S/Inputs/xref-generic-params-other-extensions.swift -module-name extensions
// RUN: %target-swift-frontend -emit-module -o %t/b-extensions.swiftmodule %s -primary-file %S/Inputs/xref-generic-params-other-extensions.swift -module-name extensions
// RUN: %target-swift-frontend -merge-modules -emit-module -o %t/extensions.swiftmodule %t/a-extensions.swiftmodule %t/b-extensions.swiftmodule -module-name extensions
// RUN: %target-swift-ide-test -print-module -module-to-print=extensions -I %t -source-filename=x | %FileCheck %s

// RUN: %target-swift-frontend -emit-module -o %t/a-extensions_mixed.swiftmodule -primary-file %s %S/Inputs/xref-generic-params-other-extensions-mixed.swift -module-name extensions_mixed
// RUN: %target-swift-frontend -emit-module -o %t/b-extensions_mixed.swiftmodule %s -primary-file %S/Inputs/xref-generic-params-other-extensions-mixed.swift -module-name extensions_mixed
// RUN: %target-swift-frontend -merge-modules -emit-module -o %t/extensions_mixed.swiftmodule %t/a-extensions_mixed.swiftmodule %t/b-extensions_mixed.swiftmodule -module-name extensions_mixed
// RUN: %target-swift-ide-test -print-module -module-to-print=extensions_mixed -I %t -source-filename=x | %FileCheck %s

// RUN: %target-swift-frontend -emit-module -o %t/a-extensions_constrained.swiftmodule -primary-file %s %S/Inputs/xref-generic-params-other-extensions-constrained.swift -module-name extensions_constrained
// RUN: %target-swift-frontend -emit-module -o %t/b-extensions_constrained.swiftmodule %s -primary-file %S/Inputs/xref-generic-params-other-extensions-constrained.swift -module-name extensions_constrained
// RUN: %target-swift-frontend -merge-modules -emit-module -o %t/extensions_constrained.swiftmodule %t/a-extensions_constrained.swiftmodule %t/b-extensions_constrained.swiftmodule -module-name extensions_constrained
// RUN: %target-swift-ide-test -print-module -module-to-print=extensions_constrained -I %t -source-filename=x | %FileCheck %s

public struct A: Equatable {}
public struct B: Equatable {}
public struct C: Equatable {}
public struct D: Equatable {}

// CHECK-LABEL: func test(
public func test(
// CHECK-SAME: _: OuterNonGeneric.InnerNonGeneric.AliasTy
_: OuterNonGeneric.InnerNonGeneric.AliasTy,
// CHECK-SAME: _: OuterNonGeneric.InnerGeneric<C, D>.AliasTy
_: OuterNonGeneric.InnerGeneric<C, D>.AliasTy,
// CHECK-SAME: _: OuterGeneric<A, B>.InnerNonGeneric.AliasTy
_: OuterGeneric<A, B>.InnerNonGeneric.AliasTy,
// CHECK-SAME: _: OuterGeneric<A, B>.InnerGeneric<C, D>.AliasTy
_: OuterGeneric<A, B>.InnerGeneric<C, D>.AliasTy) {}