[Sema]Skip Sendable conformance check when sending are added to parameters or return types of an actor-isolated function

[stzn]

Resolves #76710

Previously, we encountered errors and notes when using a non-Sendable type with sending for parameters or return types of an actor-isolated function, as shown below:

class NonSendable { var value = 0 } // note: class 'NonSendable' does not conform to the 'Sendable' protocol

protocol P {
    func bar(a: sending NonSendable) async -> sending NonSendable
}

@MainActor
class PImpl: P {
    // error: non-sendable type 'NonSendable' cannot be returned from main actor-isolated implementation to caller of protocol requirement 'bar(a:)'
    // error: non-sendable parameter type 'NonSendable' cannot be sent from caller of protocol requirement 'bar(a:)' into main actor-isolated implementation
    func bar(a: sending NonSendable) async -> sending NonSendable { a }
}

Based on the issue described, this functionality should work without any errors. To resolve the issue, we need to bypass the Sendable conformance check for non-Sendable types when sending keywords is added to function's parameters or return types.

--

Even if the conformance check is skipped, invalid scenarios will still be validated at the SIL stage to ensure correctness.

For example:

When running -emit-sil -swift-version 6, a potential data race error is still detected.

func test() async {
    var nonSendable = NonSendable()
    let p = PImpl()

    // error: sending 'nonSendable' risks causing data races
    // note: 'nonSendable' used after being passed as a 'sending' parameter; Later uses could race 
    _ = await p.bar(a: nonSendable)

    // note: access can happen concurrently 
    nonSendable.value = 1 
}

[azarovalex]

nit

Suggested change

	func bar(_ a: sending NonSendableKlass1) async -> sending NonSendableKlass1
	func bar(_ a: sending NonSendableKlass1) async -> sending NonSendableKlass1

[azarovalex]

Here you are skipping sendability validation on all other parameters if at least one of them is marked as sending, which seems incorrect, we should check sendability of all arguments.
Also this function is supposed to return true if there are any errors being produced, which is not what happens in our case.
It seems like instead of bailing out early from this function we should keep checking other arguments but skip diagnoseNonSendableTypes check below if the argument is marked as sending.

Could you also consider adding a test for a case where first argument is marked with sending but a second argument isn't?

[stzn]

Thank you for the review.

we should check sendability of all arguments.

I think that's right. Let me check it again.

Also, I overlooked type->hasError() in diagnoseSingleNonSendableType. I think that we need to check type->hasError() as well (not sure point).

static bool diagnoseSingleNonSendableType(
    Type type, SendableCheckContext fromContext,
    Type inDerivedConformance, SourceLoc loc,
    llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
  if (type->hasError())
    return false;

Could you also consider adding a test for a case where first argument is marked with sending but a second argument isn't?

Sure, I'll add it.

Thank you!

[stzn]

I fixed it in f633505. Please check it when you have time. Thank you!

[omochi]

@swift-ci please smoke test

[azarovalex]

We probably shouldn't bail out early if the result is markin with sending, there are a lot of other checks further down in this function that should still pass.

Also the return value can't be true here, since based on the comment here true means that an error was emitted, in our case though there is no error.

[stzn]

Thank you.

Also the return value can't be true here,
It was my misunderstanding🤦🏻‍♂️ Let me fix it.

We probably shouldn't bail out early if the result is markin with sending, there are a lot of other checks further down in this function that should still pass.

How about checking diag.ID like the below?

if (diag.ID == diag::non_sendable_result_in_witness.ID &&
            func->hasSendingResult() && !resultType->hasError())
    return false;

non_sendable_result_in_witness.ID is “non-sendable type %0 cannot be returned from %2 implementation to caller of protocol requirement %1”.

[stzn]

@azarovalex

Hi, what do you think about #78601 (comment) ?

I changed it in this commit

[xedin]

subscript declarations can have sending on parameters/result:

struct S {
    subscript(_: sending Int) -> Bool { true }
}

I think it should be possible to unify some of the logic in this function because i.e. getParameters() and getIndices() both produce ParameterList * and for results the logic is pretty much the same except to exactly how the type has been retrieved.

[stzn]

@xedin

Thank you very much for the advice. I combined the function and subscript checks into one.

Sorry, It's my misunderstanding. the below subscirpt is not async.

I have a question. I tried reproducing the issue with the subscript, but I encountered a different error as shown below.!

class NonSendable {}
protocol P {
subscript(_: sending NonSendable) -> Bool { get }
}
@mainactor
class C: P {
// Main actor-isolated subscript 'subscript(_:)' cannot be used to satisfy nonisolated protocol requirement
subscript(_: sending NonSendable) -> Bool { true } // If add nonisolated, the error is solved
}

I tried more patterns, but all of them successfully built.
Is there a way to reproduce the issue with subscripts? I’d like to add tests for it.

[stzn]

That is probably incorrect. It looks that we need to use getElementInterfaceType in the case of subscript. I'll correct it (and the above if condition as well).

[stzn]

I’m currently looking into several potential issues related to subscripts that use the
sending keyword.

1

An assertion failure occurs during SIL generation (see https://github.com/swiftlang/swift/blob/main/lib/AST/ASTPrinter.cpp#L3816). It appears that
ParameterTypeFlags is not set the sending flag. The issue occurs when printing the subscript getter in SILWitnessTable.

sil_witness_table hidden S: P module main {
  method #P.subscript!getter: <Self where Self : P> (Self) -> (Assertion failed: (flags.isSending() && "Only valid when sending is enabled"), function printParameterFlags, file ASTPrinter.cpp, line 3759.

Steps to reproduce:

Run the following: swift -frontend -emit-sil -swift-version 6

protocol P {
    subscript(_: sending NonSendable) -> Bool { get }
}

struct S: P {
    subscript(_: sending NonSendable) -> Bool { true }
}

It seems that setSending need be called when subscriptParam includes the sending keyword in the case of a subscript getter.

https://github.com/swiftlang/swift/blob/main/lib/AST/Decl.cpp#L10701

if (subscriptParam->isSending())
        param->setSending();

Could this be a separate issue that would be better addressed in a different PR?

I fixed it in another PR.
#79042

2

The SubscriptDecl does not have a HasSending field, so retrieving sending information relies on the
SendingTypeExpr. I am uncertain whether this is the correct approach.

It appears ok.

3

The sending keyword is not printed because the ASTPrinter does not handle it in visitSubscriptDecl.

For example, given the following code:

struct S5 {
    subscript(_: Bool) -> sending NonSendable { NonSendable() }
}

The output is:

struct S5 {
  subscript(_: Bool) -> NonSendable { get }
  init()
}

In this case, the sending keyword is omitted. I am unsure if this behavior is intentional.

Is this related to #78612?

I fixed it in another PR.
#79560

[stzn]

I made a mistake with the branch. I corrected it.

[stzn]

I think I can find all of my answers in the comment.

So, I’d like to proceed with this PR again.

What do I need to modify more to fix the original issue?