Skip to content

[Concurrency] Delete sendable checking subsumed by region isolation. #75579

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 1 commit into from
Jul 31, 2024
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
7 changes: 0 additions & 7 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5520,9 +5520,6 @@ ERROR(non_sendable_isolated_capture,none,
"capture of %1 with non-sendable type %0 in an isolated "
"%select{local function|closure}2",
(Type, DeclName, bool))
ERROR(implicit_async_let_non_sendable_capture,none,
"capture of %1 with non-sendable type %0 in 'async let' binding",
(Type, DeclName))
ERROR(self_capture_deinit_task,none,
"capture of 'self' in a closure that outlives deinit",
())
Expand Down Expand Up @@ -5642,10 +5639,6 @@ ERROR(non_sendable_param_type,none,
"in parameter of superclass method overridden by %3 %kind2|"
"in parameter of %3 '@objc' %kind2}1 cannot cross actor boundary",
(Type, unsigned, const ValueDecl *, ActorIsolation))
ERROR(non_sendable_call_argument,none,
"passing argument of non-sendable type %0 %select{into %2 context|"
"outside of %2 context}1 may introduce data races",
(Type, bool, ActorIsolation))
ERROR(non_sendable_result_type,none,
"non-sendable type %0 returned by %select{call to %3 %kind2|"
"call from %4 context to nonisolated %kind2|"
Expand Down
120 changes: 14 additions & 106 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2273,85 +2273,6 @@ static FuncDecl *findAnnotatableFunction(DeclContext *dc) {
return fn;
}

static bool shouldCheckSendable(Expr *expr) {
if (auto *declRef = dyn_cast<DeclRefExpr>(expr->findOriginalValue())) {
auto isolation = getActorIsolation(declRef->getDecl());
return isolation != ActorIsolation::NonisolatedUnsafe;
}

return true;
}

bool swift::diagnoseApplyArgSendability(ApplyExpr *apply, const DeclContext *declContext) {
auto isolationCrossing = apply->getIsolationCrossing();
if (!isolationCrossing.has_value())
return false;

auto fnExprType = apply->getFn()->getType();
if (!fnExprType)
return false;

// Check the 'self' argument.
if (auto *selfApply = dyn_cast<SelfApplyExpr>(apply->getFn())) {
auto *base = selfApply->getBase();

if (shouldCheckSendable(base) &&
diagnoseNonSendableTypes(
base->getType(),
declContext, /*inDerivedConformance*/Type(),
base->getStartLoc(),
diag::non_sendable_call_argument,
isolationCrossing.value().exitsIsolation(),
isolationCrossing.value().getDiagnoseIsolation()))
return true;
}
auto fnType = fnExprType->getAs<FunctionType>();
if (!fnType)
return false;

auto params = fnType->getParams();
for (unsigned paramIdx : indices(params)) {
const auto &param = params[paramIdx];

// Dig out the location of the argument.
SourceLoc argLoc = apply->getLoc();
Type argType;
bool checkSendable = true;
if (auto argList = apply->getArgs()) {
auto arg = argList->get(paramIdx);
if (arg.getStartLoc().isValid())
argLoc = arg.getStartLoc();

// Determine the type of the argument, ignoring any implicit
// conversions that could have stripped sendability.
if (Expr *argExpr = arg.getExpr()) {
checkSendable = shouldCheckSendable(argExpr);
argType = argExpr->findOriginalType();

// If this is a default argument expression, don't check Sendability
// if the argument is evaluated in the callee's isolation domain.
if (auto *defaultExpr = dyn_cast<DefaultArgumentExpr>(argExpr)) {
auto argIsolation = defaultExpr->getRequiredIsolation();
auto calleeIsolation = isolationCrossing->getCalleeIsolation();
if (argIsolation == calleeIsolation) {
continue;
}
}
}
}

if (checkSendable &&
diagnoseNonSendableTypes(
argType ? argType : param.getParameterType(),
declContext, /*inDerivedConformance*/Type(),
argLoc, diag::non_sendable_call_argument,
isolationCrossing.value().exitsIsolation(),
isolationCrossing.value().getDiagnoseIsolation()))
return true;
}
return false;
}

namespace {
/// Check for adherence to the actor isolation rules, emitting errors
/// when actor-isolated declarations are used in an unsafe manner.
Expand Down Expand Up @@ -2844,29 +2765,17 @@ namespace {
auto *patternBindingDecl = getTopPatternBindingDecl();
if (patternBindingDecl && patternBindingDecl->isAsyncLet()) {
// Defer diagnosing checking of non-Sendable types that are passed
// into async let to SIL level region based isolation if we have
// region based isolation enabled.
//
// We purposely do not do this for SendingArgsAndResults since that
// just triggers the actual ability to represent
// 'sending'. RegionBasedIsolation is what determines if we get the
// differing semantics.
if (!ctx.LangOpts.hasFeature(Feature::RegionBasedIsolation)) {
diagnoseNonSendableTypes(
type, getDeclContext(),
/*inDerivedConformance*/Type(), capture.getLoc(),
diag::implicit_async_let_non_sendable_capture,
decl->getName());
}
} else {
// Fallback to a generic implicit capture missing sendable
// conformance diagnostic.
diagnoseNonSendableTypes(type, getDeclContext(),
/*inDerivedConformance*/Type(),
capture.getLoc(),
diag::implicit_non_sendable_capture,
decl->getName());
// into async let to SIL level region based isolation.
return;
}

// Fallback to a generic implicit capture missing sendable
// conformance diagnostic.
diagnoseNonSendableTypes(type, getDeclContext(),
/*inDerivedConformance*/Type(),
capture.getLoc(),
diag::implicit_non_sendable_capture,
decl->getName());
} else if (fnType->isSendable()) {
diagnoseNonSendableTypes(type, getDeclContext(),
/*inDerivedConformance*/Type(),
Expand Down Expand Up @@ -3903,12 +3812,11 @@ namespace {
unsatisfiedIsolation, setThrows, usesDistributedThunk);
}

// Check if language features ask us to defer sendable diagnostics if so,
// don't check for sendability of arguments here.
if (!ctx.LangOpts.hasFeature(Feature::RegionBasedIsolation)) {
diagnoseApplyArgSendability(apply, getDeclContext());
}
// Sendable checking for arguments is deferred to region isolation.

// FIXME: Defer sendable checking for result types to region isolation
// always.
//
// Check for sendability of the result type if we do not have a
// sending result.
if ((!ctx.LangOpts.hasFeature(Feature::RegionBasedIsolation) ||
Expand Down
5 changes: 0 additions & 5 deletions lib/Sema/TypeCheckConcurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -644,11 +644,6 @@ bool isPotentiallyIsolatedActor(
VarDecl *var, llvm::function_ref<bool(ParamDecl *)> isIsolated =
[](ParamDecl *P) { return P->isIsolated(); });

/// Check whether the given ApplyExpr makes an unsatisfied isolation jump
/// and if so, emit diagnostics for any nonsendable arguments to the apply
bool diagnoseApplyArgSendability(
swift::ApplyExpr *apply, const DeclContext *declContext);

/// If the enclosing function has @_unsafeInheritExecutorAttr, return it.
AbstractFunctionDecl *enclosingUnsafeInheritsExecutor(const DeclContext *dc);

Expand Down
32 changes: 7 additions & 25 deletions test/Concurrency/issue-57376.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=targeted %s -emit-sil -o /dev/null -verify -verify-additional-prefix targeted-and-complete-
// RUN: %target-swift-frontend -disable-availability-checking -strict-concurrency=targeted %s -emit-sil -o /dev/null -verify -verify-additional-prefix targeted-
// RUN: %target-swift-frontend -disable-availability-checking %s -emit-sil -o /dev/null -verify -strict-concurrency=complete -verify-additional-prefix tns- -verify-additional-prefix transferring-tns-

// REQUIRES: concurrency
Expand All @@ -24,55 +24,37 @@ func testAsyncSequence1Sendable<Seq: AsyncSequence>(_ seq: Seq) async throws whe
async let _ = seq.reduce(0) { $0 + $1 } // OK
}

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}}
func testAsyncSequenceTypedPattern<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int {
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}}
func testAsyncSequenceTypedPattern1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int {
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}}
func testAsyncSequence<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int {
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}}
func testAsyncSequence1<Seq: AsyncSequence>(_ seq: Seq) async throws where Seq.Element == Int {
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}}
func testAsyncSequence3<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int {
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}}
func testAsyncSequence4<Seq>(_ seq: Seq) async throws where Seq: AsyncSequence, Seq.Element == Int {
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
Loading