Add support for attachments to the Foundation cross-import overlay. #799
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grynspan
added
enhancement
grynspan
requested review from
stmontgomery,
dennisweissmann,
briancroom,
SeanROlszewski and
suzannaratcliff
as code owners
|
@swift-ci test |
grynspan
force-pushed
the
jgrynspan/foundation-attachment-overlay
branch
from
81016f9 to
4ebf0f7
Compare
|
@swift-ci test |
grynspan
force-pushed
the
jgrynspan/foundation-attachment-overlay
branch
from
52ec53d to
992d1a8
Compare
|
@swift-ci test |
grynspan
force-pushed
the
jgrynspan/foundation-attachment-overlay
branch
from
f9b545e to
3b1b340
Compare
briancroom
reviewed
Nov 6, 2024
| /// | ||
| /// The specific format this case corresponds to depends on if we are encoding | ||
| /// an `Encodable` value or an `NSSecureCoding` value. | ||
| case `default` |
There was a problem hiding this comment.
Should this be the first case in the list?
There was a problem hiding this comment.
I can move it if it bothers you—it's an internal type so it's not going to affect client code.
grynspan
force-pushed
the
jgrynspan/foundation-attachment-overlay
branch
from
3b1b340 to
def5bbf
Compare
This PR makes `Test.Attachment` generic over its attachable value type. It gains conditional conformance to `Copyable` and `Sendable` depending on the attachable value and if you call `attach()` on a move-only or non-sendable attachment, will eagerly serialize the attachable value at that point (rather than during initialization.) There are a few benefits here: 1. Callers can statically know the type of the attachable value in an attachment rather than needing to always deal with an existential box; 2. We can add associated types to `Test.Attachable` that will be readily accessible in `withUnsafeBufferPointer(for:_:)` again without needing an existential; and 3. When we eventually add support for image attachments, we won't need a bunch of additional initializers or intermediate box types or what-have-you; and 4. For Embedded Swift or other environments where existentials are problematic, we can eagerly serialize all attachments and pass a consistent type (`Test.Attachment<[UInt8]>`) to the event handler. There are also some drawbacks: 1. Because conformance to `Copyable` and `Sendable` is conditional, we lose a bit of flexibility if you have a non-sendable `Test.Attachment` instance or whatnot; 2. We still need a lazy, type-erased attachment type that can be passed to the event handler. I played around with `Test.Attachment<Any>` but that causes as many problems as it solves. We end up with `Test.Attachment<any Test.Attachable & Sendable & Copyable>` but, because that's an existential type that doesn't conform to itself, the generic parameter `AttachableValue` is not constrained to `Test.Attachable`. We only provide initializers for types that do conform though (plus the existential one internally) so in practice it's not a huge issue. 3. There is some code duplication necessary (i.e. multiple implementations of `attach()` and `write()`.)
…ttachment is a pain to use with it
grynspan
force-pushed
the
jgrynspan/foundation-attachment-overlay
branch
from
def5bbf to
a7b3030
Compare
grynspan
force-pushed
the
jgrynspan/foundation-attachment-overlay
branch
from
a7b3030 to
7a9b6da
Compare
…es since we'll need them for image attachments
…le, don't limit our options more than we have to
This PR adds experimental support for attachments to some types in Foundation via the (non-functional) cross-import overlay. @stmontgomery is working on setting up said overlay so that it can actually be used; until then, the changes here are speculative only.
grynspan
force-pushed
the
jgrynspan/foundation-attachment-overlay
branch
from
7a9b6da to
df272e7
Compare
|
Closing for now: I want to sort out generic attachments first. |
2 tasks
grynspan
added a commit
that referenced
this pull request
Nov 13, 2024
…819) This PR adds experimental support for attachments to some types in Foundation via the (non-functional) cross-import overlay. @stmontgomery is working on setting up said overlay so that it can actually be used; until then, the changes here are speculative only. Replaces #799. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
grynspan
added
the
cross-import-overlays
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds experimental support for attachments to some types in Foundation via the (non-functional) cross-import overlay. @stmontgomery is working on setting up said overlay so that it can actually be used; until then, the changes here are speculative only.
Checklist: