Feedback wanted: confusing concurrency diagnostic messages

Michael_Gottesman · January 2, 2025, 8:49pm

ole:

I find the compiler error "Task or actor isolated value cannot be sent" (discussed e.g. in this thread: Task or actor isolated value cannot be sent) confusing for grammar reasons.

I'm not a native English speaker, but I think this diagnostic should be as follows (note the added hyphens), to make clear that the task itself is not the problem:

"Task- or actor-isolated value cannot be sent"

or alternatively:

"Task-isolated or actor-isolated value cannot be sent"

It would be even better if the name of the variable that is being sent could be named in the diagnostic, e.g.:

"Task- or actor-isolated value 'myVar' cannot be sent"

It's sometimes not immediately clear to me which variable this is talking about. For example, this post from the thread mentioned above shows a screenshot where the diagnostic is attached rather generically to a Task closure rather than a specific expression in the closure body (perhaps because the isolated value in question in this case is the implicit 'self'?). If the closure body were longer than just a single expression, it would not be immediately apparent to which expression the diagnostic is referring.

This error is actually an example of what I call a "fallback error"... where the compiler detects the error but fails to infer a name or source location for the actual value that is being sent. We have to fail and we have to emit a message... but if we cannot determine the actual value that is being sent, this is the best that we can do...

That being said, the emission of this error is a form of a bug. The methodology that we use to discover the name/information about the variable that is causing the problem is failing... so a bug report would be appreciated especially if it does reproduce with a ToT compiler. = ).

Michael_Gottesman · January 2, 2025, 8:55pm

bjhomer:

I ran across this error message today:

Value of non-Sendable type '@isolated(any) @async @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for ' accessed after being transferred; later accesses could race
class NonSendable {}

func test() async {
    await withTaskGroup(of: Bool.self) { group in
        for ns in [NonSendable()] {
            // 🛑: Value of non-Sendable type '@isolated(any)
            // @async @callee_guaranteed @substituted <τ_0_0> () 
            // -> @out τ_0_0 for <Bool>' accessed after being 
            // transferred; later accesses could race.
            group.addTask {
                print(ns)
                return false
            }
        }
        await group.waitForAll()
    }
}
I suspect most developers (including myself) don't know what a @isolated(any) @async @callee_guaranteed @substituted <τ_0_0> () -> @out τ_0_0 for <Bool> is.

The actual problem here seems to be that I'm capturing a non-sendable type, but the diagnostic message didn't lead me in that direction!

This is a known issue that was already fixed on main. We now emit an error that has the source level type. On main, we now emit the following:

test.swift:11:19: error: sending value of non-Sendable type '() async -> Bool' risks causing data races
 9 |             // -> @out τ_0_0 for <Bool>' accessed after being 
10 |             // transferred; later accesses could race.
11 |             group.addTask {
   |                   |- error: sending value of non-Sendable type '() async -> Bool' risks causing data races
   |                   |- note: Passing value of non-Sendable type '() async -> Bool' as a 'sending' argument to instance method 'addTask(priority:operation:)' risks causing races in between local and caller code
   |                   `- note: access can happen concurrently
12 |                 print(ns)
13 |                 return false

NOTE: the actual error here is that from a control flow perspective, we are accessing the region of the array multiple times as we go around the loop potentially in different tasks. The compiler does not know that the array only has one element. That is why the "access can happen concurrently" note is on addTask. It is actually the use in the next iteration. This can be seen if one unrolls the loop as follows:

class NonSendable {}

func test() async {
  await withTaskGroup(of: Bool.self) { group in
    let x = [NonSendable()]
    group.addTask {
      print(x)
      return false
    }
    group.addTask {
      print(x)
      return false
    }
    await group.waitForAll()
  }
}

Error:

test.swift:7:11: error: sending value of non-Sendable type '() async -> Bool' risks causing data races
 5 |   await withTaskGroup(of: Bool.self) { group in
 6 |     let x = [NonSendable()]
 7 |     group.addTask {
   |           |- error: sending value of non-Sendable type '() async -> Bool' risks causing data races
   |           `- note: Passing value of non-Sendable type '() async -> Bool' as a 'sending' argument to instance method 'addTask(priority:operation:)' risks causing races in between local and caller code
 8 |       print(x)
 9 |       return false
10 |     }
11 |     group.addTask {
   |                   `- note: access can happen concurrently
12 |       print(x)
13 |       return false

ole · January 7, 2025, 11:49am

Thanks, this is helpful. It looks like the error message for this specific case is indeed much improved on a current main toolchain. I tested with this code snippet:

class Foo {
    let value: Int = 42

    func foo() {
        Task { @MainActor in
            print(value)
        }
    }
}

With the shipping Swift 6.0.3 compiler we get the fallback error message:

$ swift --version
swift-driver version: 1.115.1 Apple Swift version 6.0.3 (swiftlang-6.0.3.1.10 clang-1600.0.30.1)
Target: arm64-apple-macosx15.0

$ swiftc -swift-version 6 code.swift
code.swift:5:14: error: task or actor isolated value cannot be sent
 3 |
 4 |     func foo() {
 5 |         Task { @MainActor in
   |              `- error: task or actor isolated value cannot be sent
 6 |             print(value)
 7 |         }

With a semi-current main toolchain (I tested with 2024-12-13 because that was the one I had already installed) we get a much better diagnostic:

$ swift --version
Apple Swift version 6.2-dev (LLVM be8c96d78337932, Swift 0bbaa3519b62071)
Target: arm64-apple-macosx15.0

$ swiftc -swift-version 6 code.swift
code.swift:6:19: error: sending 'self' risks causing data races
 4 |     func foo() {
 5 |         Task { @MainActor in
 6 |             print(value)
   |                   |- error: sending 'self' risks causing data races
   |                   `- note: task-isolated 'self' is captured by a main actor-isolated closure. main actor-isolated uses in closure may race against later nonisolated uses
 7 |         }
 8 |     }

mattie · January 9, 2025, 6:11pm

This one that involves protocol conformances seems like really low-hanging fruit:

github.com/swiftlang/swift

"cannot be used to satisfy nonisolated protocol requirement" diagnostic should identify protocol

opened 03:40PM - 09 Jan 25 UTC

rnapier

feature triage needed

### Motivation ``` protocol P { func f() } @MainActor final class C: …P { func f() {} } ``` This code correctly generates the diagnostic `warning: main actor-isolated instance method 'f()' cannot be used to satisfy nonisolated protocol requirement`. However, the diagnostic does not indicate what protocol is impacted. In larger types where the protocol conformance may be defined in an extension or even another file, it can be frustrating to track down which protocol `f` applies to in order to consider the correct fix. ### Proposed solution The diagnostic should be extended to include the protocol, for example: ``` main actor-isolated instance method 'f()' cannot be used to satisfy nonisolated protocol requirement 'P'; this is an error in the Swift 6 language mode ``` ### Alternatives considered _No response_ ### Additional information _No response_