-
Notifications
You must be signed in to change notification settings - Fork 210
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?
Changes from 1 commit
9760ccc
fd55250
6f4d2b9
d71aef0
892f17b
ec458d8
d6e88d2
f8b3fbe
d5e426f
708c11c
5fc7a2e
53c3b2e
b621212
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,20 @@ | |
|
|
||
| //===--- ContiguousBytes --------------------------------------------------===// | ||
|
|
||
| #if compiler(>=6.2) | ||
| /// 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 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just confirming, are there any ABI implications on this change?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| /// Calls the given closure with the contents of underlying storage. | ||
| /// | ||
| /// - note: Calling `withUnsafeBytes` multiple times does not guarantee that | ||
| /// 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| } | ||
| #else | ||
| /// 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, *) | ||
|
|
@@ -24,6 +38,7 @@ public protocol ContiguousBytes { | |
| /// outside of the lifetime of the call to the closure. | ||
| func withUnsafeBytes<R>(_ body: (UnsafeRawBufferPointer) throws -> R) rethrows -> R | ||
| } | ||
| #endif | ||
|
|
||
| //===--- Collection Conformances ------------------------------------------===// | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the array types implement
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's reasonable. For Array and ArraySlice, it's just a matter of completeness right now, because we still have to go through |
||
|
|
||
|
|
@@ -109,3 +124,34 @@ extension Slice : ContiguousBytes where Base : ContiguousBytes { | |
| } | ||
| } | ||
| } | ||
|
|
||
| #if compiler(>=6.2) | ||
|
|
||
| //===--- Span Conformances -----------------------------------------===// | ||
|
|
||
| @available(FoundationPreview 6.3, *) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a public conformance, right? We would need an API review.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Here's the proposal PR: #1582 |
||
| extension RawSpan: ContiguousBytes { | ||
| } | ||
|
|
||
| @available(FoundationPreview 6.3, *) | ||
| extension MutableRawSpan: ContiguousBytes { | ||
| } | ||
|
|
||
| @available(FoundationPreview 6.3, *) | ||
| extension Span: ContiguousBytes where Element == UInt8 { | ||
| } | ||
|
|
||
| @available(FoundationPreview 6.3, *) | ||
| extension MutableSpan: ContiguousBytes where Element == UInt8 { | ||
| } | ||
|
|
||
| @available(FoundationPreview 6.3, *) | ||
| @available(macOS 26.0, iOS 26.0, tvOS 26.0, watchOS 26.0, visionOS 26.0, *) | ||
|
||
| extension InlineArray: ContiguousBytes where Element == UInt8 { | ||
| @_alwaysEmitIntoClient | ||
| public func withUnsafeBytes<R, E>(_ body: (UnsafeRawBufferPointer) throws(E) -> R) throws(E) -> R { | ||
| return try span.withUnsafeBytes(body) | ||
| } | ||
| } | ||
|
|
||
| #endif | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||
| //===----------------------------------------------------------------------===// | ||||||
| // | ||||||
| // This source file is part of the Swift.org open source project | ||||||
| // | ||||||
| // Copyright (c) 2025 Apple Inc. and the Swift project authors | ||||||
| // Licensed under Apache License v2.0 with Runtime Library Exception | ||||||
| // | ||||||
| // See https://swift.org/LICENSE.txt for license information | ||||||
| // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||||||
| // | ||||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
||||||
| import Testing | ||||||
|
|
||||||
| #if canImport(FoundationEssentials) | ||||||
| @testable import FoundationEssentials | ||||||
| #else | ||||||
| @testable import Foundation | ||||||
| #endif | ||||||
|
|
||||||
| #if compiler(>=6.2) | ||||||
|
|
||||||
| func acceptContiguousBytes(_ bytes: borrowing some ContiguousBytes & ~Escapable & ~Copyable) { } | ||||||
|
|
||||||
| @Suite("ContiguousBytesTests") | ||||||
| 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, *) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was able to clean this up with your prior suggestion |
||||||
| var bytes: [UInt8] = [1, 2, 3] | ||||||
| acceptContiguousBytes(bytes.span) | ||||||
| acceptContiguousBytes(bytes.mutableSpan) | ||||||
| acceptContiguousBytes(bytes.span.bytes) | ||||||
|
|
||||||
| let ms = bytes.mutableSpan | ||||||
| acceptContiguousBytes(ms.bytes) | ||||||
|
||||||
| 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!
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.