swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.22k stars 10.32k forks source link

if/else works but ?? causes data race warning #74843

Closed jasonbobier closed 1 month ago

jasonbobier commented 2 months ago

Description

I have a case where I can use an if/else block, but can't use ?? without a data race warning

Reproduction

// This works
cbCentralManager.retrieveConnectedPeripherals(withServices: services.map { $0.uuid })
    .map {
        if let peripheral = ($0.delegate as? Peripheral) {
            return peripheral
        } else {
            return Peripheral(cbPeripheral: $0, queue: self.queue)
        }
    }

// This causes: 'self'-isolated '$0' is captured by a actor-isolated closure. actor-isolated uses in closure may race against later nonisolated uses
cbCentralManager.retrieveConnectedPeripherals(withServices: services.map { $0.uuid })
    .map { ($0.delegate as? Peripheral) ?? Peripheral(cbPeripheral: $0, queue: self.queue) }

Expected behavior

The ?? case would not cause a data race warning.

Environment

Xcode Version 16.0 beta 2 (16A5171r) in Swift 6 language mode

Additional information

No response

xedin commented 2 months ago

I think this is because ?? operator is using @autoclosure for it’s right-hand side parameter.

jasonbobier commented 2 months ago

Interesting. So @autoclosure probably needs some more smarts in a strict concurrency world.

AnthonyLatsis commented 2 months ago

As reduced as I could get it:

class Class {}

func takeClosure(_: () -> Void) {}

actor Actor {
  var x: Int = 0

  func test() {
    let _: (Class) -> Void = { c in
      takeClosure {
        _ = c
        _ = self.x
      }

      // OK:
      // let closure: () -> Void = {
      //   _ = c
      //   _ = self.x
      // }
      // takeClosure(closure)
    }
  }
}
326 |     let _: (Class) -> Void = { c in
327 |       takeClosure {
328 |         _ = c
    |             |- error: sending 'c' risks causing data races [regionbasedisolation_named_transfer_yields_race]
    |             `- note: 'self'-isolated 'c' is captured by a actor-isolated closure. actor-isolated uses in closure may race against later nonisolated uses [regionbasedisolation_named_isolated_closure_yields_race]
329 |         _ = self.x
330 |       }

This appears to be about closures in general, not specifically autoclosures. I am not familiar with region-based isolation semantics yet, but I at least do not see any isolation crossings to diagnose or watch out for here. Both the method and the closures are isolated to the parent actor.

xedin commented 2 months ago

I am bringing back ‘type checker’ label because that’s where the isolation is determined. This might not be a SIL issue because it’s a consumer of that information. cc @gottesmm

gottesmm commented 1 month ago

I fixed this some time ago I think. It doesn't reproduce with swift-DEVELOPMENT-SNAPSHOT-2024-07-27-a. Thank you for the test case!