Should this be a Swift 6 error? (Actors + Sendable closures)

Using Swift
1

I am working toward a Swift 6 migration. I found the following code produces a warning in Swift 5, which makes sense. I would have thought that it would have been a hard error in Swift 6, but at least in Swift 6.3, this code is only an error even in the Swift 6 language mode:

@preconcurrency func runLater(_ work: @Sendable @escaping ()->Void) {
    DispatchQueue.global().async(execute: work)
}

public actor DBActor {
    private var count: Int = 0

    public func caller() {
        runLater {
            // ⚠️: Call to actor-isolated instance method 'syncFunction()'
            // in a synchronous nonisolated context
            // 
            // ??? I would expect this to be an error in Swift 6
            self.syncFunction()
        }
    }

    func syncFunction() {
        // 🛑 Assertion fails at runtime, as expected
        self.assertIsolated()
        count += 1
    }
}

Is there a plan to upgrade this warning into an error at some future time? Or is there an Upcoming Feature flag that would make this an error?

1 Like
2

I guess I assume it's because of the @preconcurrency on runLater. (I put that here just to mimic what we'd get from using DispatchQueue.async, which is also @preconcurrency.) So really, we're probably waiting for DispatchQueue and friends to be built with Swift 6, so they lose the @preconcurrency annotation.

3

Yes. @preconcurrency downgrades the error to a warning.

3 Likes
4

I'm a little bit worried by this just being a warning. According to the proposal which introduced @preconcurrency on decls:

  • Minimal concurrency checking: ... on nominal declarations, @preconcurrency (defined below) has special effects in this mode which suppress many diagnostics.

As I understand it, the motivations of supporting @preconcurrency on decls it to ease concurrency adoption of legacy modules. It should not affect the compiling behavior of modules built with -swift-version 6. For those modules, errors should always be errors.

5

SE-0337 is outdated in this regard and needs to be updated. There is currently an open PR to address this: Update SE-0337 to reflect @preconcurrency working on decls in Swift 6. CC @aviva

2 Likes
6

Thanks a lot for this information! Though I still think that's a real miss, rather than just a documentation problem.

In my opinion, continue downgrading errors to warning for -swift-version 6 modules will leave security holes in the codebase.
My reasoning is as follows. Once a certain API is marked @preconcurrency, the author cannot remove the attribute almost forever, because that will break the ABI. A client that uses this API may benefit from the downgrading behavior at first, but it will eventually migrate to -swift-version 6. If the compiler keeps holding back errors, the client can then still make some mistakes easily, that just negates the benefit of transitioning to strict concurrency in the first place...

7

I agree with you. Perhaps you saw my pitch for modernizing Async{Throwing}Stream. One of the things I hope we can fix with the new APIs is getting rid of @preconcurrency in Swift 6. When the unfolding initializer was retrospectively annotated with @Sendable in Swift 6.0 we also ended up requiring @preconcurrency, which just doesn’t feel right for a primary API from the _Concurrency module, in my opinion.

3 Likes
8

Yeah, it's definitely unfortunate. I think we may need a good design for opting into @preconcurrency as error as a consumer of libraries; but this needs to be balanced with wanting existing libraries to continue to adopt concurrency attributes. Before switching to SE-0478, I was working on a PR to track when preconcurrency was the source of a downgrade and add a note + userdoc (draft here) for this case--we absolutely need more visibility into why this is happening. I'd like to track the reason behind downgrades in more detail, but it's very involved.

The rationale behind supporting preconcurrency in Swift 6 is that without this support, a library with any Swift 6 consumers can no longer confidently add or fix mistaken concurrency attributes. For example, one of the recommendations for [Sema] Fix failing to diagnose unused attrs, improve alias attribute diagnostic, preserve alias sugar by a-viv-a · Pull Request #87838 · swiftlang/swift · GitHub was that existing APIs making this mistake should use @preconcurrency to not change their ABI. If Swift 6 didn't support @preconcurrency, what do you do? I could also imagine preconcurrency being used to rollout something FnOnce shaped (at most once can be very useful for concurrency) although I think that would be a bad idea.

I suspect a good design could look like allowing all existing @preconcurrency annotations in a dep to be ignored, but still respecting non-suppressed @preconcurrency, so you can get "full" checking and still allow your deps to evolve with new annotations (without breaking your build on an update). I think this would be more of a package manager feature to track it + some kind of CLI feature specify which @preconcurrency to ignore? You could argue that adding concurrency annotations after the initial 'concurrency audit' should be a breaking change, but those are hard to distinguish (especially done incrementally).

2 Likes