Restrict withUnsafeBytes to swift 5.0 or greater availability to avoid ambiguity

[phausler]

The recent refactor for Data added a new method of withUnsafeBytes that vends a UnsafeRawBufferPointer in its closure. However there were external projects that added this as a category to Data which unfortunately lead to un-reconcilable ambiguity. Marking this method as available in swift 5.0 it prevents the ambiguity at the cost of preventing the usage of the API that avoids undefined behavior in 4.2 mode.

[phausler]

@swift-ci please smoke test

[phausler]

@swift-ci please test source compatibility

[itaiferber]

LGTM, pending this working, of course.

[phausler]

@jrose-apple do you know what these interface files are failing for?

[itaiferber]

@jrose-apple To clarify what we're doing here: SwiftNIO 1 added Data.withUnsafeBytes in an extension — now that we've added our own, we were causing source-breakage for SwiftNIO 1 users. SwiftNIO 2 removes this method (in favor of ours), and it's what Swift 5 users will need to use (SwiftNIO 1 is strictly pre-Swift-5, SwiftNIO 2 is strictly Swift 5 and later).

We're trying to avoid breaking Swift 4.2-mode users of Swift 5 by declaring our method as Swift 5 only. However, my gut feeling is that this is complaining that we wouldn't be able to build our overlay in Swift 4? (Which is true, but also, we build our overlay strictly in Swift 5)

[itaiferber]

The verification is indeed happening with -swift-version 4:

: 'RUN: at line 5';   for x in /Users/buildnode/jenkins/workspace/swift-PR-osx-smoke-test/branch-master/buildbot_incremental/swift-macosx-x86_64/lib/swift/macosx/x86_64/*.swiftinterface; do [[ $(basename "$x") = Swift.swiftinterface || $(basename "$x") = simd.swiftinterface || $(basename "$x") = SwiftLang.swiftinterface || $(basename "$x") = '*.swiftinterface' ]] && continue; /Users/buildnode/jenkins/workspace/swift-PR-osx-smoke-test/branch-master/buildbot_incremental/swift-macosx-x86_64/bin/swiftc -frontend -target x86_64-apple-macosx10.9  -module-cache-path '/Users/buildnode/jenkins/workspace/swift-PR-osx-smoke-test/branch-master/buildbot_incremental/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.9/clang-module-cache' -sdk '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk' -swift-version 4  "$x" -emit-module -o /Users/buildnode/jenkins/workspace/swift-PR-osx-smoke-test/branch-master/buildbot_incremental/swift-macosx-x86_64/validation-test-macosx-x86_64/ParseableInterface/Output/verify_all_overlays.swift.tmp/$(basename "$x" .swiftinterface).swiftmodule -disable-objc-attr-requires-foundation-module -enable-resilience -Fsystem "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk"/System/Library/PrivateFrameworks/ -swift-version 4 || echo 'macosx:' $(basename "$x") >> /Users/buildnode/jenkins/workspace/swift-PR-osx-smoke-test/branch-master/buildbot_incremental/swift-macosx-x86_64/validation-test-macosx-x86_64/ParseableInterface/Output/verify_all_overlays.swift.tmp/failures.txt; done

Upon further thought, this is a real issue. ContiguousBytes relies on withUnsafeBytes, and so this could never work in Swift 4.x.

I think the solution will be to annotate every new API with @available for Swift 5.

[itaiferber]

@jrose-apple @slavapestov This is worse than we thought, since it doesn't look like we can conditionally extend Data with ContiguousBytes only for Swift 5, unless I'm missing the syntax for it. Is there any way to express the following?

import Foundation

@available(swift, introduced: 5)
public protocol Foo {}

@available(swift, introduced: 5)
extension Data : Foo {}

Untitled 4.swift:7:18: error: 'Foo' is unavailable
extension Data : Foo {}
                 ^~~
Untitled 4.swift:4:17: note: 'Foo' was introduced in Swift 5
public protocol Foo {}
                ^

[jrose-apple]

No, you can't do that. Swift versions are a compile-time concept, but conformances exist at run time.

[jrose-apple]

I'll fix the interfaces test soon to use whatever Swift version they were originally built with, but yeah, unless ContiguousBytes itself is limited to Swift 5 mode you're going to be in trouble here.

[jrose-apple]

(feel free to XFAIL the test if you need to make progress)

[itaiferber]

@jrose-apple Thanks for confirming — if this is the case, we can't fix this by making ContiguousBytes Swift-5-only, nor can we change the availability of this method. We're going to need to figure out an alternative here.

[itaiferber]

@jrose-apple Any @_implements trickery you think we can pull to make this work? Unfortunately, the following does not compile, but is the essence of what we really want:

protocol Foo {
    func foo()
}

struct Bar : Foo {
    @_implements(Foo, foo())
    @available(swift, introduced: 4, obsoleted: 5)
    func bar() {
        print("Bar!")
    }
    
    @available(swift, introduced: 5)
    func foo() {
        print("Foo!")
    }
}

let f: Foo = Bar()
f.foo()

[phausler]

@swift-ci please smoke test

[jrose-apple]

I hate to add more uses of _implements, but if you drop the @available for bar it compiles and does what you want. I'm not 100% sure that fixes the original problem, though—you might then just have ambiguity with the protocol method.

EDIT: Or maybe that will work because the people who added the new capability will have added it to Data, making it more specific (i.e. a better overload) than the one on the protocol.

[phausler]

@swift-ci please smoke test

[itaiferber]

@jrose-apple Hmm, from the perspective of the protocol conformance, I'm not sure what would happen. If Module A has an implementation for protocol P's methods on struct S, and Module B adds identically-named implementations in an extension to S, does Module C see ambiguity, or does it "just work" (for some definition of "just work")? [This is the issue we're hitting here — Data implements withUnsafeBytes in the overlay, SwiftNIO adds Data.withUnsafeBytes in an extension, and then downstream clients of both see ambiguity.] If we rename the method on the protocol to something else but use @_implements, SwiftNIO's extension wouldn't override the method, but would clients still see ambiguity between what is required by the protocol, and what SwiftNIO is offering?

In either case, it appears that just removing the @available annotation from bar() doesn't compile for me in Swift 4 mode:

error: type 'Bar' does not conform to protocol 'Foo'
struct Bar : Foo {
       ^
error: unavailable instance method 'foo()' was used to satisfy a requirement of protocol 'Foo'
    func foo() { print("Foo!") }
         ^
note: requirement 'foo()' declared here
protocol Foo { func foo() }
                    ^

[jrose-apple]

Fixing the interface tests in #21687.

[phausler]

After review we have decided to take a different direction here.