Skip to content

Commit 8912d4a

Browse files
DougGregorDougGregor
committed
[SE-0413] Adopt typed throws in withoutActuallyEscaping(_:do:)
There is a small bug fix here in the identification of the catch node, where the leading `{` of a closure was considered to be "inside" the closure for code like { ... }() causing us to assume that the call to the closure would catch the error within the closure. Other than that, introduce the thrown error type into the type checker's modeling of `withoutActuallyEscaping(_:do:)`, and mirror that in the library declaration.
1 parent 0b2cc84 commit 8912d4a

File tree

7 files changed

+92
-10
lines changed

7 files changed

+92
-10
lines changed

‎lib/AST/ASTScopeLookup.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,21 @@ getCatchNode(const ASTScopeImpl *scope) {
704704
return { CatchNode(), nullptr };
705705
}
706706

707+
/// Check whether the given location precedes the start of the catch location
708+
/// despite being technically within the catch node's source range.
709+
static bool locationIsPriorToStartOfCatchScope(SourceLoc loc, CatchNode node) {
710+
auto closure = node.dyn_cast<ClosureExpr *>();
711+
if (!closure)
712+
return false;
713+
714+
SourceManager &sourceMgr = closure->getASTContext().SourceMgr;
715+
SourceLoc inLoc = closure->getInLoc();
716+
if (inLoc.isValid())
717+
return sourceMgr.isBefore(loc, inLoc);
718+
719+
return sourceMgr.isAtOrBefore(loc, closure->getStartLoc());
720+
}
721+
707722
CatchNode ASTScopeImpl::lookupCatchNode(ModuleDecl *module, SourceLoc loc) {
708723
auto sourceFile = module->getSourceFileContainingLocation(loc);
709724
if (!sourceFile)
@@ -721,7 +736,8 @@ CatchNode ASTScopeImpl::lookupCatchNode(ModuleDecl *module, SourceLoc loc) {
721736
// If we are at a catch node and in the body of the region from which that
722737
// node catches thrown errors, we have our result.
723738
auto caught = getCatchNode(scope);
724-
if (caught.first && caught.second == innerBodyScope) {
739+
if (caught.first && caught.second == innerBodyScope &&
740+
!locationIsPriorToStartOfCatchScope(loc, caught.first)) {
725741
return caught.first;
726742
}
727743

‎lib/Sema/ConstraintSystem.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3108,12 +3108,15 @@ static DeclReferenceType getTypeOfReferenceWithSpecialTypeCheckingSemantics(
31083108
auto result = CS.createTypeVariable(
31093109
CS.getConstraintLocator(locator, ConstraintLocator::FunctionResult),
31103110
TVO_CanBindToNoEscape);
3111+
auto thrownError = CS.createTypeVariable(
3112+
CS.getConstraintLocator(locator, ConstraintLocator::ThrownErrorType),
3113+
0);
31113114
FunctionType::Param arg(escapeClosure);
31123115
auto bodyClosure = FunctionType::get(arg, result,
31133116
FunctionType::ExtInfoBuilder()
31143117
.withNoEscape(true)
31153118
.withAsync(true)
3116-
.withThrows(true, /*FIXME:*/Type())
3119+
.withThrows(true, thrownError)
31173120
.build());
31183121
FunctionType::Param args[] = {
31193122
FunctionType::Param(noescapeClosure),
@@ -3124,7 +3127,7 @@ static DeclReferenceType getTypeOfReferenceWithSpecialTypeCheckingSemantics(
31243127
FunctionType::ExtInfoBuilder()
31253128
.withNoEscape(false)
31263129
.withAsync(true)
3127-
.withThrows(true, /*FIXME:*/Type())
3130+
.withThrows(true, thrownError)
31283131
.build());
31293132
return {refType, refType, refType, refType, Type()};
31303133
}

‎stdlib/public/core/Builtin.swift

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,12 +1022,28 @@ public func type<T, Metatype>(of value: T) -> Metatype {
10221022
/// - body: A closure that is executed immediately with an escapable copy of
10231023
/// `closure` as its argument.
10241024
/// - Returns: The return value, if any, of the `body` closure.
1025+
@_alwaysEmitIntoClient
10251026
@_transparent
10261027
@_semantics("typechecker.withoutActuallyEscaping(_:do:)")
1027-
public func withoutActuallyEscaping<ClosureType, ResultType>(
1028+
public func withoutActuallyEscaping<ClosureType, ResultType, Failure>(
1029+
_ closure: ClosureType,
1030+
do body: (_ escapingClosure: ClosureType) throws(Failure) -> ResultType
1031+
) throws(Failure) -> ResultType {
1032+
// This implementation is never used, since calls to
1033+
// `Swift.withoutActuallyEscaping(_:do:)` are resolved as a special case by
1034+
// the type checker.
1035+
Builtin.staticReport(_trueAfterDiagnostics(), true._value,
1036+
("internal consistency error: 'withoutActuallyEscaping(_:do:)' operation failed to resolve"
1037+
as StaticString).utf8Start._rawValue)
1038+
Builtin.unreachable()
1039+
}
1040+
1041+
@_silgen_name("$ss23withoutActuallyEscaping_2doq_x_q_xKXEtKr0_lF")
1042+
@usableFromInline
1043+
func __abi_withoutActuallyEscaping<ClosureType, ResultType>(
10281044
_ closure: ClosureType,
10291045
do body: (_ escapingClosure: ClosureType) throws -> ResultType
1030-
) rethrows -> ResultType {
1046+
) throws -> ResultType {
10311047
// This implementation is never used, since calls to
10321048
// `Swift.withoutActuallyEscaping(_:do:)` are resolved as a special case by
10331049
// the type checker.

‎test/Constraints/without_actually_escaping.swift

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,8 @@ func rethrowThroughWAE(_ zz: (Int, Int, Int) throws -> Int, _ value: Int) throws
6060
}
6161
}
6262

63-
// There should be two errors - here one for async -> sync, and another throws -> non-throws
6463
let _: ((Int) -> Int, (@escaping (Int) -> Int) -> ()) -> () = withoutActuallyEscaping(_:do:)
65-
// expected-error@-1 {{invalid conversion from throwing function of type '((Int) -> Int, (@escaping (Int) -> Int) async throws -> ()) async throws -> ()' to non-throwing function type '((Int) -> Int, (@escaping (Int) -> Int) -> ()) -> ()'}}
66-
// expected-error@-2 {{invalid conversion from 'async' function of type '((Int) -> Int, (@escaping (Int) -> Int) async throws -> ()) async throws -> ()' to synchronous function type '((Int) -> Int, (@escaping (Int) -> Int) -> ()) -> ()'}}
64+
// expected-error@-1 {{invalid conversion from 'async' function of type '((Int) -> Int, (@escaping (Int) -> Int) async -> ()) async -> ()' to synchronous function type '((Int) -> Int, (@escaping (Int) -> Int) -> ()) -> ()'}}
6765

6866

6967
// Failing to propagate @noescape into non-single-expression
@@ -91,3 +89,24 @@ class Box<T> {
9189
}
9290
}
9391
}
92+
93+
enum HomeworkError: Error {
94+
case forgot
95+
case dogAteIt
96+
}
97+
98+
enum MyError: Error {
99+
case fail
100+
}
101+
102+
func letEscapeThrowTyped(f: () throws(HomeworkError) -> () -> ()) throws(HomeworkError) -> () -> () {
103+
// Note: thrown error type inference for closures will fix this error below.
104+
return try withoutActuallyEscaping(f) { return try $0() }
105+
// expected-error@-1{{thrown expression type 'any Error' cannot be converted to error type 'HomeworkError'}}
106+
}
107+
108+
func letEscapeThrowTypedBad(f: () throws(HomeworkError) -> () -> ()) throws(MyError) -> () -> () {
109+
// Note: thrown error type inference for closures will change this error below.
110+
return try withoutActuallyEscaping(f) { return try $0() }
111+
// expected-error@-1{{thrown expression type 'any Error' cannot be converted to error type 'MyError'}}
112+
}

‎test/SILGen/without_actually_escaping.swift

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,31 @@ func letEscapeThrow(f: () throws -> () -> ()) throws -> () -> () {
5151
return try withoutActuallyEscaping(f) { return try $0() }
5252
}
5353

54-
// thunk for @callee_guaranteed () -> (@owned @escaping @callee_guaranteed () -> (), @error @owned Error)
54+
enum HomeworkError: Error {
55+
case forgot
56+
case dogAteIt
57+
}
58+
59+
// CHECK-LABEL: sil hidden [ossa] @$s25without_actually_escaping19letEscapeThrowTyped1fyycyycyAA13HomeworkErrorOYKXE_tKF : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> (@owned @callee_guaranteed () -> (), @error HomeworkError)) -> (@owned @callee_guaranteed () -> (), @error any Error)
60+
// CHECK: bb0([[ARG:%.*]] : @guaranteed $@noescape @callee_guaranteed () -> (@owned @callee_guaranteed () -> (), @error HomeworkError)):
61+
// CHECK: function_ref @$sIeg_25without_actually_escaping13HomeworkErrorOIgozo_Ieg_ACIegozo_TR : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> (@owned @callee_guaranteed () -> (), @error HomeworkError)) -> (@owned @callee_guaranteed () -> (), @error HomeworkError)
62+
// CHECK: bb2([[ERR:%.*]] : @owned $any Error):
63+
// CHECK: end_borrow [[BORROW]]
64+
// CHECK: destroy_value [[MD]]
65+
// CHECK: throw [[ERR]] : $any Error
66+
// CHECK: }
67+
68+
func letEscapeThrowTyped(f: () throws(HomeworkError) -> () -> ()) throws -> () -> () {
69+
return try withoutActuallyEscaping(f) { return try $0() }
70+
}
71+
5572
// The thunk must be [without_actually_escaping].
56-
// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [without_actually_escaping] [ossa] @$sIeg_s5Error_pIgozo_Ieg_sAA_pIegozo_TR : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> (@owned @callee_guaranteed () -> (), @error any Error)) -> (@owned @callee_guaranteed () -> (), @error any Error) {
73+
// CHECK-LABEL: sil shared [transparent] [serialized] [reabstraction_thunk] [without_actually_escaping] [ossa] @$sIeg_25without_actually_escaping13HomeworkErrorOIgozo_Ieg_ACIegozo_TR : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> (@owned @callee_guaranteed () -> (), @error HomeworkError)) -> (@owned @callee_guaranteed () -> (), @error HomeworkError)
74+
75+
func letEscapeThrowTyped2(f: () throws(HomeworkError) -> () -> ()) throws(HomeworkError) -> () -> () {
76+
return try withoutActuallyEscaping(f) { (g) throws(HomeworkError) in return try g() }
77+
}
78+
5779

5880
// We used to crash on this example because we would use the wrong substitution
5981
// map.

‎test/api-digester/Outputs/stability-stdlib-source-x86_64.swift.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,5 +41,7 @@ Func Collection.map(_:) is now without @rethrows
4141
Func Sequence.map(_:) has generic signature change from <Self, T where Self : Swift.Sequence> to <Self, T, E where Self : Swift.Sequence, E : Swift.Error>
4242
Func Sequence.map(_:) is now without @rethrows
4343
Constructor Result.init(catching:) has generic signature change from <Success, Failure where Failure == any Swift.Error> to <Success, Failure where Failure : Swift.Error>
44+
Func withoutActuallyEscaping(_:do:) has generic signature change from <ClosureType, ResultType> to <ClosureType, ResultType, Failure where Failure : Swift.Error>
45+
Func withoutActuallyEscaping(_:do:) is now without @rethrows
4446

4547
Protocol SIMDScalar has generic signature change from <Self == Self.SIMD16Storage.Scalar, Self.SIMD16Storage : Swift.SIMDStorage, Self.SIMD2Storage : Swift.SIMDStorage, Self.SIMD32Storage : Swift.SIMDStorage, Self.SIMD4Storage : Swift.SIMDStorage, Self.SIMD64Storage : Swift.SIMDStorage, Self.SIMD8Storage : Swift.SIMDStorage, Self.SIMDMaskScalar : Swift.FixedWidthInteger, Self.SIMDMaskScalar : Swift.SIMDScalar, Self.SIMDMaskScalar : Swift.SignedInteger, Self.SIMD16Storage.Scalar == Self.SIMD2Storage.Scalar, Self.SIMD2Storage.Scalar == Self.SIMD32Storage.Scalar, Self.SIMD32Storage.Scalar == Self.SIMD4Storage.Scalar, Self.SIMD4Storage.Scalar == Self.SIMD64Storage.Scalar, Self.SIMD64Storage.Scalar == Self.SIMD8Storage.Scalar> to <Self == Self.SIMD16Storage.Scalar, Self.SIMD16Storage : Swift.SIMDStorage, Self.SIMD2Storage : Swift.SIMDStorage, Self.SIMD32Storage : Swift.SIMDStorage, Self.SIMD4Storage : Swift.SIMDStorage, Self.SIMD64Storage : Swift.SIMDStorage, Self.SIMD8Storage : Swift.SIMDStorage, Self.SIMDMaskScalar : Swift.FixedWidthInteger, Self.SIMDMaskScalar : Swift.SIMDScalar, Self.SIMDMaskScalar : Swift.SignedInteger, Self.SIMDMaskScalar == Self.SIMDMaskScalar.SIMDMaskScalar, Self.SIMD16Storage.Scalar == Self.SIMD2Storage.Scalar, Self.SIMD2Storage.Scalar == Self.SIMD32Storage.Scalar, Self.SIMD32Storage.Scalar == Self.SIMD4Storage.Scalar, Self.SIMD4Storage.Scalar == Self.SIMD64Storage.Scalar, Self.SIMD64Storage.Scalar == Self.SIMD8Storage.Scalar>

‎test/api-digester/stability-stdlib-abi-without-asserts.test

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ Constructor Result.init(__abi_catching:) is a new API without @available attribu
9999
Constructor Result.init(catching:) has been removed
100100
Func Result.get() has been renamed to Func __abi_get()
101101
Func Result.get() has mangled name changing from 'Swift.Result.get() throws -> A' to 'Swift.Result.__abi_get() throws -> A'
102+
Func withoutActuallyEscaping(_:do:) has been renamed to Func __abi_withoutActuallyEscaping(_:do:)
103+
Func withoutActuallyEscaping(_:do:) has mangled name changing from 'Swift.withoutActuallyEscaping<A, B>(_: A, do: (A) throws -> B) throws -> B' to 'Swift.__abi_withoutActuallyEscaping<A, B>(_: A, do: (A) throws -> B) throws -> B'
104+
Func withoutActuallyEscaping(_:do:) is now without @rethrows
105+
102106

103107
// These haven't actually been removed; they are simply marked unavailable.
104108
// This seems to be a false positive in the ABI checker. This is not an ABI

0 commit comments

Comments
 (0)