Skip to content

Commit 785e7b1

Browse files
authored
Merge pull request #82858 from gottesmm/rdar154969621
[concurrency] Perform some fixes so that transfernonsendable_closureliterals_isolationinference.swift now passes.
2 parents dab6f3d + 648bb8f commit 785e7b1

File tree

6 files changed

+99
-29
lines changed

6 files changed

+99
-29
lines changed

lib/SILGen/SILGenExpr.cpp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,10 @@ namespace {
497497
RValue
498498
emitFunctionCvtFromExecutionCallerToGlobalActor(FunctionConversionExpr *E,
499499
SGFContext C);
500+
501+
RValue emitFunctionCvtForNonisolatedNonsendingClosureExpr(
502+
FunctionConversionExpr *E, SGFContext C);
503+
500504
RValue visitActorIsolationErasureExpr(ActorIsolationErasureExpr *E,
501505
SGFContext C);
502506
RValue visitExtractFunctionIsolationExpr(ExtractFunctionIsolationExpr *E,
@@ -2031,6 +2035,44 @@ RValueEmitter::emitFunctionCvtToExecutionCaller(FunctionConversionExpr *e,
20312035
return RValue(SGF, e, destType, result);
20322036
}
20332037

2038+
RValue RValueEmitter::emitFunctionCvtForNonisolatedNonsendingClosureExpr(
2039+
FunctionConversionExpr *E, SGFContext C) {
2040+
// The specific AST pattern for this looks as follows:
2041+
//
2042+
// (function_conversion_expr type="nonisolated(nonsending) () async -> Void"
2043+
// (closure_expr type="() async -> ()" isolated_to_caller_isolation))
2044+
CanAnyFunctionType destType =
2045+
cast<FunctionType>(E->getType()->getCanonicalType());
2046+
auto subExpr = E->getSubExpr()->getSemanticsProvidingExpr();
2047+
2048+
// If we do not have a closure or if that closure is not caller isolation
2049+
// inheriting, bail.
2050+
auto *closureExpr = dyn_cast<ClosureExpr>(subExpr);
2051+
if (!closureExpr ||
2052+
!closureExpr->getActorIsolation().isCallerIsolationInheriting())
2053+
return RValue();
2054+
2055+
// Then grab our closure type... make sure it is non isolated and then make
2056+
// sure it is the same as our destType but with nonisolated.
2057+
CanAnyFunctionType closureType =
2058+
cast<FunctionType>(closureExpr->getType()->getCanonicalType());
2059+
if (!closureType->getIsolation().isNonIsolated() ||
2060+
closureType !=
2061+
destType->withIsolation(FunctionTypeIsolation::forNonIsolated())
2062+
->getCanonicalType())
2063+
return RValue();
2064+
2065+
// NOTE: This is a partial inline of getClosureTypeInfo. We do this so we have
2066+
// more control and make this change less viral in the compiler for 6.2.
2067+
auto newExtInfo = closureType->getExtInfo().withIsolation(
2068+
FunctionTypeIsolation::forNonIsolatedCaller());
2069+
closureType = closureType.withExtInfo(newExtInfo);
2070+
auto info = SGF.getFunctionTypeInfo(closureType);
2071+
2072+
auto closure = emitClosureReference(closureExpr, info);
2073+
return RValue(SGF, closureExpr, destType, closure);
2074+
}
2075+
20342076
RValue RValueEmitter::emitFunctionCvtFromExecutionCallerToGlobalActor(
20352077
FunctionConversionExpr *e, SGFContext C) {
20362078
// We are pattern matching a conversion sequence like the following:
@@ -2142,6 +2184,28 @@ RValue RValueEmitter::visitFunctionConversionExpr(FunctionConversionExpr *e,
21422184
// TODO: Move this up when we can emit closures directly with C calling
21432185
// convention.
21442186
auto subExpr = e->getSubExpr()->getSemanticsProvidingExpr();
2187+
2188+
// Before we go any further into emitting the convert function expr, see if
2189+
// our SubExpr is a ClosureExpr with the exact same type as our
2190+
// FunctionConversionExpr except with the FunctionConversionExpr adding
2191+
// nonisolated(nonsending). Then see if the ClosureExpr itself (even though it
2192+
// is not nonisolated(nonsending) typed is considered to have
2193+
// nonisolated(nonsending) isolation. In such a case, emit the closure
2194+
// directly. We are going to handle it especially in closure emission to work
2195+
// around the missing information in the type.
2196+
//
2197+
// DISCUSSION: We need to do this here since in the Expression TypeChecker we
2198+
// do not have access to capture information when we would normally want to
2199+
// mark the closure type as being nonisolated(nonsending). As a result, we
2200+
// cannot know if the nonisolated(nonsending) should be overridden by for
2201+
// example an actor that is captured by the closure. So to work around this in
2202+
// Sema, we still mark the ClosureExpr as having the appropriate isolation
2203+
// even though its type does not have it... and then we work around this here
2204+
// and also in getClosureTypeInfo.
2205+
if (destType->getIsolation().isNonIsolatedCaller())
2206+
if (auto rv = emitFunctionCvtForNonisolatedNonsendingClosureExpr(e, C))
2207+
return rv;
2208+
21452209
// Look through `as` type ascriptions that don't induce bridging too.
21462210
while (auto subCoerce = dyn_cast<CoerceExpr>(subExpr)) {
21472211
// Coercions that introduce bridging aren't simple type ascriptions.

lib/Sema/CSApply.cpp

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7849,23 +7849,6 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
78497849
}
78507850
}
78517851

7852-
// If we have a ClosureExpr, then we can safely propagate the
7853-
// 'nonisolated(nonsending)' isolation if it's not explicitly
7854-
// marked as `@concurrent`.
7855-
if (toEI.getIsolation().isNonIsolatedCaller() &&
7856-
(fromEI.getIsolation().isNonIsolated() &&
7857-
!isClosureMarkedAsConcurrent(expr))) {
7858-
auto newFromFuncType = fromFunc->withIsolation(
7859-
FunctionTypeIsolation::forNonIsolatedCaller());
7860-
if (applyTypeToClosureExpr(cs, expr, newFromFuncType)) {
7861-
fromFunc = newFromFuncType->castTo<FunctionType>();
7862-
// Propagating 'nonisolated(nonsending)' might have satisfied the entire
7863-
// conversion. If so, we're done, otherwise keep converting.
7864-
if (fromFunc->isEqual(toType))
7865-
return expr;
7866-
}
7867-
}
7868-
78697852
if (ctx.LangOpts.isDynamicActorIsolationCheckingEnabled()) {
78707853
// Passing a synchronous global actor-isolated function value and
78717854
// parameter that expects a synchronous non-isolated function type could

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4969,8 +4969,33 @@ ActorIsolation ActorIsolationChecker::determineClosureIsolation(
49694969
closure->getParent(), getClosureActorIsolation);
49704970
preconcurrency |= parentIsolation.preconcurrency();
49714971

4972-
return computeClosureIsolationFromParent(closure, parentIsolation,
4973-
checkIsolatedCapture);
4972+
auto normalIsolation = computeClosureIsolationFromParent(
4973+
closure, parentIsolation, checkIsolatedCapture);
4974+
4975+
// The solver has to be conservative and produce a conversion to
4976+
// `nonisolated(nonsending)` because at solution application time
4977+
// we don't yet know whether there are any captures which would
4978+
// make closure isolated.
4979+
//
4980+
// At this point we know that closure is not explicitly annotated with
4981+
// global actor, nonisolated/@concurrent attributes and doesn't have
4982+
// isolated parameters. If our closure is nonisolated and we have a
4983+
// conversion to nonisolated(nonsending), then we should respect that.
4984+
if (auto *explicitClosure = dyn_cast<ClosureExpr>(closure);
4985+
!normalIsolation.isGlobalActor()) {
4986+
if (auto *fce =
4987+
dyn_cast_or_null<FunctionConversionExpr>(Parent.getAsExpr())) {
4988+
auto expectedIsolation =
4989+
fce->getType()->castTo<FunctionType>()->getIsolation();
4990+
if (expectedIsolation.isNonIsolatedCaller()) {
4991+
auto captureInfo = explicitClosure->getCaptureInfo();
4992+
if (!captureInfo.getIsolatedParamCapture())
4993+
return ActorIsolation::forCallerIsolationInheriting();
4994+
}
4995+
}
4996+
}
4997+
4998+
return normalIsolation;
49744999
}();
49755000

49765001
// Apply computed preconcurrency.

test/Concurrency/attr_execution/attr_execution.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func callerTest() async {}
1818
struct Test {
1919
// CHECK-LABEL: // closure #1 in variable initialization expression of Test.x
2020
// CHECK: // Isolation: caller_isolation_inheriting
21-
// CHECK: sil private [ossa] @$s14attr_execution4TestV1xyyYaYCcvpfiyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
21+
// CHECK: sil private [ossa] @$s14attr_execution4TestV1xyyYaYCcvpfiyyYacfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
2222
var x: () async -> Void = {}
2323

2424
// CHECK-LABEL: // Test.test()
@@ -67,15 +67,15 @@ func takesClosure(fn: () async -> Void) {
6767
}
6868

6969
// CHECK-LABEL: sil hidden [ossa] @$s14attr_execution11testClosureyyF : $@convention(thin) () -> ()
70-
// CHECK: [[CLOSURE:%.*]] = function_ref @$s14attr_execution11testClosureyyFyyYaYCXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
70+
// CHECK: [[CLOSURE:%.*]] = function_ref @$s14attr_execution11testClosureyyFyyYaXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
7171
// CHECK: [[THUNKED_CLOSURE:%.*]] = thin_to_thick_function %0 to $@noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
7272
// CHECK: [[TAKES_CLOSURE:%.*]] = function_ref @$s14attr_execution12takesClosure2fnyyyYaYCXE_tF : $@convention(thin) (@guaranteed @noescape @async @callee_guaranteed (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()) -> ()
7373
// CHECK: apply [[TAKES_CLOSURE]]([[THUNKED_CLOSURE]])
7474
// CHECK: } // end sil function '$s14attr_execution11testClosureyyF'
7575

7676
// CHECK-LABEL: // closure #1 in testClosure()
7777
// CHECK: // Isolation: caller_isolation_inheriting
78-
// CHECK: sil private [ossa] @$s14attr_execution11testClosureyyFyyYaYCXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
78+
// CHECK: sil private [ossa] @$s14attr_execution11testClosureyyFyyYaXEfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
7979
func testClosure() {
8080
takesClosure {
8181
}

test/Concurrency/attr_execution/conversions_silgen.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ func conversionsFromSyncToAsync(_ x: @escaping @Sendable (NonSendableKlass) -> V
421421
}
422422

423423
func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) async -> Void) {
424-
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCcfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
424+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYacfU_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
425425
// CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
426426
// CHECK: hop_to_executor [[EXECUTOR]]
427427
let _: nonisolated(nonsending) () async -> Void = {
@@ -430,7 +430,7 @@ func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) asy
430430

431431
func testParam(_: nonisolated(nonsending) () async throws -> Void) {}
432432

433-
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaYCXEfU0_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> @error any Error
433+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFyyYaXEfU0_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
434434
// CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>):
435435
// CHECK: hop_to_executor [[EXECUTOR]]
436436
testParam { 42 }
@@ -440,7 +440,7 @@ func testThatClosuresAssumeIsolation(fn: inout nonisolated(nonsending) (Int) asy
440440
// CHECK: hop_to_executor [[GENERIC_EXECUTOR]]
441441
testParam { @concurrent in 42 }
442442

443-
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYaYCcfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> ()
443+
// CHECK-LABEL: sil private [ossa] @$s21attr_execution_silgen31testThatClosuresAssumeIsolation2fnyySiYaYCcz_tFySiYacfU2_ : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>, Int) -> () {
444444
// CHECK: bb0([[EXECUTOR:%.*]] : @guaranteed $Optional<any Actor>, %1 : $Int):
445445
// CHECK: hop_to_executor [[EXECUTOR]]
446446
fn = { _ in }

test/Concurrency/transfernonsendable_closureliterals_isolationinference.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
1-
// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 5 -strict-concurrency=complete %s -o - | %FileCheck %s
1+
// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 5 -strict-concurrency=complete %s -o - 2>/dev/null | %FileCheck %s
22
// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 6 -verify %s -o /dev/null -verify-additional-prefix ni-
3-
// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 5 -strict-concurrency=complete %s -o - -enable-upcoming-feature NonisolatedNonsendingByDefault | %FileCheck %s
3+
// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 5 -strict-concurrency=complete %s -o - -enable-upcoming-feature NonisolatedNonsendingByDefault 2>/dev/null | %FileCheck %s
44
// RUN: %target-swift-frontend -emit-sil -parse-as-library -target %target-swift-5.1-abi-triple -swift-version 6 -verify %s -o /dev/null -enable-upcoming-feature NonisolatedNonsendingByDefault -verify-additional-prefix ni-ns-
55

66
// REQUIRES: concurrency
77
// REQUIRES: asserts
88
// REQUIRES: swift_feature_NonisolatedNonsendingByDefault
99

10-
// REQUIRES: rdar154969621
11-
1210
// This test validates the behavior of transfernonsendable around
1311
// closure literals
1412

0 commit comments

Comments
 (0)