Skip to content

[region-isolation] Treat async let as a isolation inference boundary closure and fix diagnostics due to change. #75069

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
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
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,10 @@ NOTE(regionbasedisolation_named_isolated_closure_yields_race, none,
"%0%1 is captured by a %2 closure. %2 uses in closure may race against later %3 uses",
(StringRef, Identifier, ActorIsolation, ActorIsolation))

NOTE(regionbasedisolation_named_transfer_nt_asynclet_capture, none,
"sending %1 %0 into async let risks causing data races between nonisolated and %1 uses",
(Identifier, StringRef))

// Misc Error.
ERROR(regionbasedisolation_task_or_actor_isolated_transferred, none,
"task or actor isolated value cannot be sent", ())
Expand Down
114 changes: 113 additions & 1 deletion lib/SILOptimizer/Mandatory/TransferNonSendable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ getTransferringApplyCalleeInfo(SILInstruction *inst) {
return {};

auto *decl = declRef->getDecl();
if (!decl->hasName())
if (!decl || !decl->hasName())
return {};

return {{decl->getDescriptiveKind(), decl->getName()}};
Expand Down Expand Up @@ -1217,6 +1217,10 @@ class TransferNonTransferrableDiagnosticEmitter {
return getTransferringApplyCalleeInfo(info.transferredOperand->getUser());
}

SILLocation getLoc() const {
return info.transferredOperand->getUser()->getLoc();
}

/// Return the isolation region info for \p getNonTransferrableValue().
SILDynamicMergedIsolationInfo getIsolationRegionInfo() const {
return info.isolationRegionInfo;
Expand Down Expand Up @@ -1292,6 +1296,24 @@ class TransferNonTransferrableDiagnosticEmitter {
.limitBehaviorIf(getBehaviorLimit());
}

void emitNamedAsyncLetCapture(SILLocation loc, Identifier name,
SILIsolationInfo transferredValueIsolation) {
assert(!getIsolationRegionInfo().isDisconnected() &&
"Should never be disconnected?!");
emitNamedOnlyError(loc, name);

SmallString<64> descriptiveKindStr;
{
llvm::raw_svector_ostream os(descriptiveKindStr);
getIsolationRegionInfo().printForDiagnostics(os);
}

diagnoseNote(loc,
diag::regionbasedisolation_named_transfer_nt_asynclet_capture,
name, descriptiveKindStr)
.limitBehaviorIf(getBehaviorLimit());
}

void emitNamedIsolation(SILLocation loc, Identifier name,
ApplyIsolationCrossing isolationCrossing) {
emitNamedOnlyError(loc, name);
Expand Down Expand Up @@ -1402,6 +1424,8 @@ class TransferNonTransferrableDiagnosticEmitter {
};

class TransferNonTransferrableDiagnosticInferrer {
struct AutoClosureWalker;

TransferNonTransferrableDiagnosticEmitter diagnosticEmitter;

public:
Expand Down Expand Up @@ -1454,6 +1478,74 @@ bool TransferNonTransferrableDiagnosticInferrer::initForIsolatedPartialApply(
return false;
}

/// This walker visits an AutoClosureExpr and looks for uses of a specific
/// captured value. We want to error on the uses in the autoclosure.
struct TransferNonTransferrableDiagnosticInferrer::AutoClosureWalker
: ASTWalker {
TransferNonTransferrableDiagnosticEmitter &foundTypeInfo;
ValueDecl *targetDecl;
SILIsolationInfo targetDeclIsolationInfo;
SmallPtrSet<Expr *, 8> visitedCallExprDeclRefExprs;
SILLocation captureLoc;
bool isAsyncLet;

AutoClosureWalker(TransferNonTransferrableDiagnosticEmitter &foundTypeInfo,
ValueDecl *targetDecl,
SILIsolationInfo targetDeclIsolationInfo,
SILLocation captureLoc, bool isAsyncLet)
: foundTypeInfo(foundTypeInfo), targetDecl(targetDecl),
targetDeclIsolationInfo(targetDeclIsolationInfo),
captureLoc(captureLoc), isAsyncLet(isAsyncLet) {}

Expr *lookThroughArgExpr(Expr *expr) {
while (true) {
if (auto *memberRefExpr = dyn_cast<MemberRefExpr>(expr)) {
expr = memberRefExpr->getBase();
continue;
}

if (auto *cvt = dyn_cast<ImplicitConversionExpr>(expr)) {
expr = cvt->getSubExpr();
continue;
}

if (auto *e = dyn_cast<ForceValueExpr>(expr)) {
expr = e->getSubExpr();
continue;
}

if (auto *t = dyn_cast<TupleElementExpr>(expr)) {
expr = t->getBase();
continue;
}

return expr;
}
}

PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
if (auto *declRef = dyn_cast<DeclRefExpr>(expr)) {
// If this decl ref expr was not visited as part of a callExpr and is our
// target decl... emit a simple async let error.
//
// This occurs if we do:
//
// ```
// let x = ...
// async let y = x
// ```
if (declRef->getDecl() == targetDecl) {
foundTypeInfo.emitNamedAsyncLetCapture(captureLoc,
targetDecl->getBaseIdentifier(),
targetDeclIsolationInfo);
return Action::Continue(expr);
}
}

return Action::Continue(expr);
}
};

bool TransferNonTransferrableDiagnosticInferrer::run() {
// We need to find the isolation info.
auto *op = diagnosticEmitter.getOperand();
Expand Down Expand Up @@ -1570,6 +1662,26 @@ bool TransferNonTransferrableDiagnosticInferrer::run() {
}
}

// If we are failing due to an autoclosure... see if we can find the captured
// value that is causing the issue.
if (auto *autoClosureExpr = loc.getAsASTNode<AutoClosureExpr>()) {
// To split up this work, we only do this for async let for now.
if (autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet) {
auto *i = op->getUser();
auto pai = ApplySite::isa(i);
unsigned captureIndex = pai.getAppliedArgIndex(*op);
auto captureInfo =
autoClosureExpr->getCaptureInfo().getCaptures()[captureIndex];
auto loc = RegularLocation(captureInfo.getLoc(), false /*implicit*/);
AutoClosureWalker walker(
diagnosticEmitter, captureInfo.getDecl(),
diagnosticEmitter.getIsolationRegionInfo().getIsolationInfo(), loc,
autoClosureExpr->getThunkKind() == AutoClosureExpr::Kind::AsyncLet);
autoClosureExpr->walk(walker);
return true;
}
}

diagnosticEmitter.emitUnknownUse(loc);
return true;
}
Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,13 @@ isIsolationInferenceBoundaryClosure(const AbstractClosureExpr *closure,
}
}

// An autoclosure for an async let acts as a boundary. It is non-Sendable
// regardless of its context.
if (auto *autoclosure = dyn_cast<AutoClosureExpr>(closure)) {
if (autoclosure->getThunkKind() == AutoClosureExpr::Kind::AsyncLet)
return true;
}

return isSendableClosure(closure, forActorIsolation);
}

Expand Down
7 changes: 5 additions & 2 deletions test/Concurrency/global_actor_function_types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,12 @@ func stripActor(_ expr: @Sendable @autoclosure () -> (() -> ())) async {
return await stripActor(mainActorFn) // expected-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}}
}

// NOTE: this warning is correct, but is only being caught by TypeCheckConcurrency's extra check.
// We used to not emit an error here with strict-concurrency enabled since we
// were inferring the async let to main actor isolated (which was incorrect). We
// now always treat async let as non-isolated, so we get the same error in all
// contexts.
@MainActor func exampleWhereConstraintSolverHasWrongDeclContext_v2() async -> Int {
async let a: () = noActor(mainActorFn) // expected-without-transferring-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}}
async let a: () = noActor(mainActorFn) // expected-warning {{converting function value of type '@MainActor () -> ()' to '() -> ()' loses global actor 'MainActor'}}
await a
}

Expand Down
42 changes: 24 additions & 18 deletions test/Concurrency/issue-57376.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,47 +26,53 @@ func testAsyncSequence1Sendable<Seq: AsyncSequence>(_ seq: Seq) async throws whe

func testAsyncSequenceTypedPattern<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{54-54=, Sendable}}
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{54-54=, Sendable}}
async let result: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
async let result: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
let _ = try! await result
}

func testAsyncSequenceTypedPattern1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{55-55=, Sendable}}
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{55-55=, Sendable}}
async let _: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
async let _: Int = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
}

func testAsyncSequence<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{42-42=, Sendable}}
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{42-42=, Sendable}}
async let result = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
async let result = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
let _ = try! await result
}

func testAsyncSequence1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{43-43=, Sendable}}
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{43-43=, Sendable}}
async let _ = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
async let _ = seq.reduce(0) { $0 + $1 } // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
}

func testAsyncSequence3<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
async let result = seq // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
async let result = seq // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
let _ = await result
}

func testAsyncSequence4<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int { // expected-targeted-and-complete-note {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
// expected-no-transferring-tns-note @-1 {{consider making generic parameter 'Seq' conform to the 'Sendable' protocol}} {{28-28=: Sendable}}
async let _ = seq // expected-transferring-tns-warning {{task or actor isolated value cannot be sent}}
// expected-targeted-and-complete-warning @-1 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
async let _ = seq // expected-transferring-tns-warning {{sending 'seq' risks causing data races}}
// expected-transferring-tns-note @-1 {{sending task-isolated 'seq' into async let risks causing data races between nonisolated and task-isolated uses}}
// expected-targeted-and-complete-warning @-2 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
// expected-no-transferring-tns-warning @-3 {{capture of 'seq' with non-sendable type 'Seq' in 'async let' binding}}
}

func search(query: String, entities: [String]) async throws -> [String] {
Expand Down
56 changes: 53 additions & 3 deletions test/Concurrency/transfernonsendable_asynclet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ class NonSendableKlass {

class SendableKlass : @unchecked Sendable {}

actor Actor {
struct NonSendableStruct { // expected-note {{}}
var ns = NonSendableKlass()
}

actor MyActor {
var klass = NonSendableKlass()
final var finalKlass = NonSendableKlass()

Expand All @@ -28,7 +32,7 @@ actor Actor {
func useNonSendableFunction(_: () -> Void) {}
}

final actor FinalActor {
final actor FinalMyActor {
var klass = NonSendableKlass()
func useKlass(_ x: NonSendableKlass) {}
}
Expand All @@ -45,6 +49,13 @@ func useInOut<T>(_ x: inout T) {}
func useValue<T>(_ x: T) -> T { x }
func useValueWrapInOptional<T>(_ x: T) -> T? { x }

func useValueNoReturnWithInstance<T, V : Actor>(_ x: T, _ y: V) -> () { fatalError() }
func useValueAsyncNoReturnWithInstance<T, V : Actor>(_ x: T, _ y: V) async -> () { fatalError() }
@MainActor
func useMainActorValueAsyncNoReturn<T>(_ x: T) async -> () { fatalError() }
@MainActor
func useMainActorValueNoReturn<T>(_ x: T) -> () { fatalError() }

@MainActor func returnValueFromMain<T>() async -> T { fatalError() }
@MainActor func transferToMain<T>(_ t: T) async {}
@MainActor func transferToMainInt<T>(_ t: T) async -> Int { 5 }
Expand Down Expand Up @@ -720,11 +731,50 @@ func asyncLetWithoutCapture() async {
}

func asyncLet_Let_ActorIsolated_Method() async {
let a = Actor()
let a = MyActor()
let x = NonSendableKlass()
async let y = a.useKlass(x) // expected-warning {{sending 'x' risks causing data races}}
// expected-note @-1 {{sending 'x' to actor-isolated instance method 'useKlass' risks causing data races between actor-isolated and local nonisolated uses}}

useValue(x) // expected-note {{access can happen concurrently}}
let _ = await y
}

extension NonSendableStruct {
func asyncLetInferAsNonIsolated<T : Actor>(
isolation actor: isolated T
) async throws {
async let subTask: Void = {
await useValueAsyncNoReturnWithInstance(self, actor)
// expected-warning @-1:47 {{sending 'self' risks causing data races}}
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
}()
await subTask

async let subTask2: () = await useValueAsyncNoReturnWithInstance(self, actor)
// expected-warning @-1 {{sending 'self' risks causing data races}}
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
await subTask2

async let subTask3: () = useValueNoReturnWithInstance(self, actor)
// expected-warning @-1 {{sending 'self' risks causing data races}}
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
await subTask3

async let subTask4: () = await useMainActorValueAsyncNoReturn(self)
// expected-warning @-1 {{sending 'self' risks causing data races}}
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
await subTask4

async let subTask5: () = useMainActorValueNoReturn(self)
// expected-warning @-1 {{sending 'self' risks causing data races}}
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
await subTask5

async let subTask6: NonSendableStruct = self
// expected-warning @-1 {{sending 'self' risks causing data races}}
// expected-note @-2 {{sending 'actor'-isolated 'self' into async let risks causing data races between nonisolated and 'actor'-isolated uses}}
// expected-warning @-3 {{non-sendable type 'NonSendableStruct' returned by implicitly asynchronous call to nonisolated function cannot cross actor boundary}}
_ = await subTask6
}
}
Loading