Issues with SE-0470: Global-actor isolated conformances

This proposal has only just been brought to my attention.

The requirement for meta types crossing isolation boundaries conforming to SendableMetatype impacts a number of server side swift libraries, including swift-nio, vapor, hummingbird, postgres-nio and soto.

A number of these libraries are going to have to release semver breaking changes (by adding the SendableMetatype requirement to generic parameters) if they are to compile without warnings. Admittedly not a huge issue given everything up to this point has had SendableMetatype conformance inferred. But it creates churn, causes messy code full of preprocessor directives, as we support before and after SE-0470. In general non-major version updates of the compiler and toolchain shouldn't break existing code. I know it is only an additional warning, but when releasing a version of a library I prefer to release one that doesn't have any warnings in it.

I fully understand the need for this new protocol but for something so impactful I was surprised to see the proposal discussion SE-0470: Global-actor isolated conformances had so little response.

FYI this is a proposed PR to fix this up for Hummingbird SendableMetatype fixups by adam-fowler · Pull Request #712 · hummingbird-project/hummingbird · GitHub

1 Like

Just for posterity, there was a fairly detailed discussion in the pitch thread as well.

I think an acceptable compatibility shim for compilers before 6.2 would be typealias SendableMetatype = Any, no? What am I missing that requires the more elaborate workaround?

The typealias has to be public which is a bit unfortunate if each package/module is going to declare such a typealias but we can phase it out over time again.

#if compiler(<6.2)
public typealias SendableMetatype = Any
#endif

public protocol Foo: SendableMetatype {}

One thing that isn't yet clear to me though is if we need to mark types and methods that add this constraint or conformance with 6.2 as @preconcurrency. As an example does the below change need a @preconcurrency annotation?

// Current API
func foo<T>(t: T.Type) {}

// API from 6.2 onwards
func foo<T: SendableMetatype>(t: T.Type)

From the proposals text:

Therefore, this proposal suggests to accept this as a source-breaking change with strict concurrency (as a warning in Swift 5, error in Swift 6) rather than staging the change through an upcoming feature or alternative language mode.

Which makes me assume we need to make methods and protocols that add a SendableMetatype constraint as @preconcurrency which rules out the typealias based approach as far as I can see.

I can't vouch for this 100% but it appears that @preconcurrency is ignored by older compilers if its a no-op, so this compiles without errors in 6.1 as far as I can tell. There might be some other case where it complains though:

#if compiler(<6.2)
typealias SendableMetatype = Any
#endif

@preconcurrency
protocol P: SendableMetatype {}

@preconcurrency
func f<T: SendableMetatype>(_: T) {}

EDIT: However, it appears that @preconcurrency doesn't actually have the intended effect here on 6.2:

swift % xcrun swiftc x.swift -emit-silgen | grep '^sil '
sil [ossa] @main : $@convention(c) (Int32, UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>) -> Int32 {
sil [ossa] @$s1x1fyyxlF : $@convention(thin) <T> (@in_guaranteed T) -> () {

swift % ../build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swiftc x.swift -emit-silgen | grep '^sil '
sil [ossa] @main : $@convention(c) (Int32, UnsafeMutablePointer<Optional<UnsafeMutablePointer<Int8>>>) -> Int32 {
sil [ossa] @$s1x1fyyxs16SendableMetatypeRzlF : $@convention(thin) <T where T : SendableMetatype> (@in_guaranteed T) -> () {

@Douglas_Gregor you might want to take a look.

3 Likes

I just confirmed that introducing a new SendableMetatype requirement on a generic method parameter is indeed a breaking change for adopters that results in an error.

// Module A
public protocol GlobalLookup {
  static func lookupByName(_ name: String) -> Self?
}

public func hasNamed<T: GlobalLookup>(_: T.Type, name: String) async -> Bool {
   return await Task.detached {
     return T.lookupByName(name) != nil // warning: Capture of non-sendable type 'T.Type' in an isolated closure
   }.value
}

// Module B
func bar<T: GlobalLookup>(t: T.Type) async {
    _ = await hasNamed(t.self, name: "p")
}

This is the example taken from the proposal. The above code produces the expected warning. If we now apply the fix from the proposal we get an error in the consuming module

// Module A
public func hasNamed<T: GlobalLookup & SendableMetatype>(_: T.Type, name: String) async -> Bool {
   return await Task.detached {
     return T.lookupByName(name) != nil
   }.value
}

// Module B
func bar<T: GlobalLookup>(t: T.Type) async {
    _ = await hasNamed(t.self, name: "p") // error: Global function 'hasNamed(_:name:)' requires that 'T' conform to 'SendableMetatype'
}

So to avoid breaking existing clients we need to apply @preconcurrency on the hasNamed method in Module A to downgrade this to a warning. However, as @Slava_Pestov rightly points out @preconcurrency currently has no effect on this so there is no way to downgrade this to a warning right now.

2 Likes

I believe fixing this is a matter of teaching @preconcurrency to strip away SendableMetatype the way it does with plain old Sendable. It sounds like an oversight. Do you mind filing an issue?

4 Likes

Created `@preconcurrnecy` doesn't downgrade `SendableMetatype` error · Issue #81739 · swiftlang/swift · GitHub to track this.