-
Notifications
You must be signed in to change notification settings - Fork 207
Generalize ContiguousBytes to be noncopyable and nonescapable for spans #1565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Generalize ContiguousBytes to be noncopyable and nonescapable for spans #1565
Conversation
The ContiguousBytes protocol covers various types that can provide unsafe access to their stored bytes. Generalize the ContiguousBytes protocol by making it non-copyable and non-escapable, so that the Span family of types and InlineArray can conform to it. Fixes rdar://163716671.
|
@swift-ci please test |
|
|
||
| //===--- ContiguousBytes --------------------------------------------------===// | ||
|
|
||
| #if compiler(>=6.2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Foundation only compiles with the most recent released compiler + main, so I don't think this check is required.
|
|
||
| //===--- Span Conformances -----------------------------------------===// | ||
|
|
||
| @available(FoundationPreview 6.3, *) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a public conformance, right? We would need an API review.
| acceptContiguousBytes(bytes.span.bytes) | ||
|
|
||
| let ms = bytes.mutableSpan | ||
| acceptContiguousBytes(ms.bytes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| acceptContiguousBytes(ms.bytes) | |
| acceptContiguousBytes(ms.mutableBytes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually want to do both, thanks!
| /// Indicates that the conforming type is a contiguous collection of raw bytes | ||
| /// whose underlying storage is directly accessible by withUnsafeBytes. | ||
| @available(macOS 10.10, iOS 8.0, watchOS 2.0, tvOS 9.0, *) | ||
| public protocol ContiguousBytes: ~Escapable, ~Copyable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirming, are there any ABI implications on this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is not ABI-breaking (or source-breaking) to make this change.
| } | ||
|
|
||
| @available(FoundationPreview 6.3, *) | ||
| @available(macOS 26.0, iOS 26.0, tvOS 26.0, watchOS 26.0, visionOS 26.0, *) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The double availability attribute seems a bit suspicious here - each declaration should only have a single availability. Is the problem here the macOS back deployment build of the package? If so, we'll need to create a new availability macro (something like FoundationInlineArray) that maps to macOS 26-aligned in the package manifest / CMake files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This worked out, thank you for the suggestion
| private struct ContiguousBytesTests { | ||
| @available(macOS 26.0, iOS 26.0, tvOS 26.0, watchOS 26.0, visionOS 26.0, *) | ||
| @Test func span() throws { | ||
| if #available(FoundationPreview 6.3, *) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the function is already annotated as only available on macOS 26+, I don't think we need this #available check here. In the end what we probably want is for this function to be annotated as @available(FoundationInlineArray 6.3, *)
| /// the same buffer pointer will be passed in every time. | ||
| /// - warning: The buffer argument to the body should not be stored or used | ||
| /// outside of the lifetime of the call to the closure. | ||
| func withUnsafeBytes<R>(_ body: (UnsafeRawBufferPointer) throws -> R) rethrows -> R |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's probably still work to be done on this protocol to update it (it doesn't allow for producing a span, it hasn't adopted typed throws, etc.) but this seems like a reasonable step in a good direction that we can do without fully fleshing out those changes at the moment if we want to update this incrementally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. The best we can do with the protocol as-is (without breaking existing clients) would be to add a withSpan, because that's implementable by going through withUnsafeBytes.
…ntiguousBytes Introduce a new availability macro, FoundationInlineArray, which relies on features that are part of the Swift 6.2 standard library, and use that for InlineArray's conformance to ContiguousBytes. Also remove the #if compiler checks I added to this code. We can assume a Swift 6.2 (or newer) compiler.
For each of the conformances to ContiguousBytes, adopt typed throws in the withUnsafeBytes functions. When not building for Embedded Swift, leave in the old implementation as a @usableFromInline internal function to continue to expose the untyped (rethrowing) version in the binary. In the ContiguousBytes protocol itself, we don't have a good way to introduce the typed-throws variant without breaking the ABI, so settle for adopting typed throws only in Embedded Swift until we can sort that out.
The `withBytes` operation is the safe counterpart to `withUnsafeBytes`. It provides a RawSpan to its closure, rather than an unsafe raw buffer pointer, to ensure that the pointer does not escape the closure. There is a default implementation of `withBytes` built on top of `withUnsafeBytes` in the obvious way. It also papers over the Embedded-vs-Desktop differences by always providing a typed-throws API.
|
@swift-ci please test |
The ContiguousBytes protocol covers various types that can provide unsafe access to their stored bytes. Generalize the ContiguousBytes protocol by making it non-copyable and non-escapable, so that the Span family of types and InlineArray can conform to it.
This pull request also generalizes the ContiguousBytes APIs for typed throws and introduces a new
withBytesoperation as a safe counterpart towithUnsafeBytesthat provides aRawSpan.Fixes rdar://163716671.