From 8ca54835b0fd04933b235ff18373b95822d8138f Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 9 Nov 2023 08:38:07 -0800 Subject: [PATCH 1/4] [Typed throws] Narrow typed throws conversions to existential erasure in calls Match what we do in `emitThrow`. Both of these should be unified and generalized, relying on the AST to provide the necessary conversions rather than having SILGen re-derive them. --- lib/SILGen/SILGenApply.cpp | 44 +++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 44d5b05691a94..e00f01d797f3a 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -5754,25 +5754,43 @@ SILValue SILGenFunction::emitApplyWithRethrow(SILLocation loc, SILValue fn, outerError = innerError; } else { // The error requires some kind of translation. - - // Load the inner error, if it was returned indirectly. - if (innerError->getType().isAddress()) { - innerError = emitLoad(loc, innerError, getTypeLowering(innerErrorType), - SGFContext(), IsTake).forward(*this); - } + outerErrorType = outerErrorType.getObjectType(); // If we need to convert the error type, do so now. if (innerErrorType != outerErrorType) { - auto conversion = Conversion::getOrigToSubst( - AbstractionPattern(innerErrorType.getASTType()), - outerErrorType.getASTType(), - outerErrorType); - outerError = emitConvertedRValue(loc, conversion, SGFContext(), - [innerError](SILGenFunction &SGF, SILLocation loc, SGFContext C) { - return ManagedValue::forForwardedRValue(SGF, innerError); + assert(outerErrorType == SILType::getExceptionType(getASTContext())); + + ProtocolConformanceRef conformances[1] = { + getModule().getSwiftModule()->conformsToProtocol( + innerError->getType().getASTType(), + getASTContext().getErrorDecl()) + }; + + outerError = emitExistentialErasure( + loc, + innerErrorType.getASTType(), + getTypeLowering(innerErrorType), + getTypeLowering(outerErrorType), + getASTContext().AllocateCopy(conformances), + SGFContext(), + [&](SGFContext C) -> ManagedValue { + if (innerError->getType().isAddress()) { + return emitLoad(loc, innerError, + getTypeLowering(innerErrorType), SGFContext(), + IsTake); + } + + return ManagedValue::forForwardedRValue(*this, innerError); }).forward(*this); + } else if (innerError->getType().isAddress()) { + // Load the inner error, if it was returned indirectly. + outerError = emitLoad(loc, innerError, getTypeLowering(innerErrorType), + SGFContext(), IsTake).forward(*this); + } else { + outerError = innerError; } + // If the outer error is returned indirectly, copy from the converted // inner error to the outer error slot. if (IndirectErrorResult) { From 4bbfba790a3c8306b8a5322866d929d655e0fbc7 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 9 Nov 2023 10:02:21 -0800 Subject: [PATCH 2/4] [Typed throws] Rethrows functions always throw `any Error` Rethrows functions only throw when their closure arguments throw. However, they are free to translate the errors thrown from the closure arguments in any way they want, and are therefore untyped. Ensure that calls to `rethrows` functions are always treated as throwing `any Error` if their closure arguments throw anything. --- lib/Sema/TypeCheckEffects.cpp | 23 ++++++++++++++++++++--- test/decl/func/typed_throws.swift | 8 ++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/Sema/TypeCheckEffects.cpp b/lib/Sema/TypeCheckEffects.cpp index a73402a6c13ef..3e15d135394d1 100644 --- a/lib/Sema/TypeCheckEffects.cpp +++ b/lib/Sema/TypeCheckEffects.cpp @@ -718,6 +718,17 @@ class Classification { return result; } + /// Return a classification that promotes a typed throws effect to an + /// untyped throws effect. + Classification promoteToUntypedThrows() const { + if (!hasThrows()) + return *this; + + Classification result(*this); + result.ThrownError = ThrownError->getASTContext().getErrorExistentialType(); + return result; + } + /// Return a classification that only retains the parts of this /// classification for the requested effect kind. Classification onlyEffect(EffectKind kind) const { @@ -1109,9 +1120,15 @@ class ApplyClassifier { } for (unsigned i = 0, e = params.size(); i < e; ++i) { - result.merge(classifyArgument(args->getExpr(i), - params[i].getParameterType(), - kind)); + auto argClassification = classifyArgument( + args->getExpr(i), params[i].getParameterType(), kind); + + // Rethrows is untyped, so + if (kind == EffectKind::Throws) { + argClassification = argClassification.promoteToUntypedThrows(); + } + + result.merge(argClassification); } return; diff --git a/test/decl/func/typed_throws.swift b/test/decl/func/typed_throws.swift index 25c5211d85d90..04a79d77782bc 100644 --- a/test/decl/func/typed_throws.swift +++ b/test/decl/func/typed_throws.swift @@ -146,3 +146,11 @@ extension Either: Error where First: Error, Second: Error { } func f(_ error: Either) throws(Either) { throw error } + +// Ensure that calls to 'rethrows' functions are always treated as throwing `any +// Error`. +func rethrowingFunc(body: () throws -> Void) rethrows { } + +func typedCallsRethrowingFunc(body: () throws(E) -> Void) throws(E) { + try rethrowingFunc(body: body) // expected-error{{thrown expression type 'any Error' cannot be converted to error type 'E'}} +} From 4f773c225001d747c6893d4e08ad49b6d774af49 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 9 Nov 2023 11:12:27 -0800 Subject: [PATCH 3/4] [SILGen] Ensure that we have a scope for error conversions in reabstraction thunks --- lib/SILGen/SILGenApply.cpp | 2 ++ test/Profiler/coverage_closure_returns_never.swift | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index e00f01d797f3a..7ef823d4cb251 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -5727,6 +5727,8 @@ SILValue SILGenFunction::emitApplyWithRethrow(SILLocation loc, SILValue fn, { B.emitBlock(errorBB); + Scope scope(Cleanups, CleanupLocation(loc)); + // Grab the inner error. SILValue innerError; bool hasInnerIndirectError = fnConv.hasIndirectSILErrorResults(); diff --git a/test/Profiler/coverage_closure_returns_never.swift b/test/Profiler/coverage_closure_returns_never.swift index d880bb17bdbac..fe22da6e6088c 100644 --- a/test/Profiler/coverage_closure_returns_never.swift +++ b/test/Profiler/coverage_closure_returns_never.swift @@ -6,7 +6,6 @@ // CHECK-NOT: increment_profiler_counter // CHECK: [[LOAD:%.*]] = load {{.*}} : $*Never // CHECK-NEXT: debug_value [[LOAD]] : $Never -// CHECK-NEXT: debug_value undef : $any Error, var, name "$error", argno // CHECK-NEXT: unreachable func closure_with_fatal_error(_ arr: [Never]) { From 07d887111ddf350092cc95930a1c276a47e1a18d Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 9 Nov 2023 14:31:57 -0800 Subject: [PATCH 4/4] Generalize test --- test/Profiler/coverage_closure_returns_never.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Profiler/coverage_closure_returns_never.swift b/test/Profiler/coverage_closure_returns_never.swift index fe22da6e6088c..b91a6c72601d9 100644 --- a/test/Profiler/coverage_closure_returns_never.swift +++ b/test/Profiler/coverage_closure_returns_never.swift @@ -6,7 +6,7 @@ // CHECK-NOT: increment_profiler_counter // CHECK: [[LOAD:%.*]] = load {{.*}} : $*Never // CHECK-NEXT: debug_value [[LOAD]] : $Never -// CHECK-NEXT: unreachable +// CHECK: unreachable func closure_with_fatal_error(_ arr: [Never]) { // CHECK-LABEL: sil_coverage_map {{.*}}// closure #1 (Swift.Never) -> Swift.Never