Skip to content

Commit 368a94d

Browse files
authored
Avoid eagerly serializing move-only attachments. (#1317)
This PR adjusts the interface of `Attachment` so that it always conforms to `Copyable` even if the value it wraps does not. This allows us to avoid eagerly serializing attachments that are not `Copyable` but _are_ `Sendable`. We must still eagerly serialize non-sendable attachments. This change will allow us to support "attachment lifetime" (API to be designed, see XCTest's API [here](https://developer.apple.com/documentation/xctest/xctattachment/lifetime-swift.enum)) more efficiently and for more attachable values. If you're paying the cost of serialization for attachments you don't end up keeping, that's a waste of\ resources. ### 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.
1 parent d875f17 commit 368a94d

File tree

2 files changed

+84
-104
lines changed

2 files changed

+84
-104
lines changed

Sources/Testing/Attachments/Attachment.swift

Lines changed: 83 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,23 @@ private import _TestingInternals
2323
/// @Available(Swift, introduced: 6.2)
2424
/// @Available(Xcode, introduced: 26.0)
2525
/// }
26-
public struct Attachment<AttachableValue>: ~Copyable where AttachableValue: Attachable & ~Copyable {
26+
public struct Attachment<AttachableValue> where AttachableValue: Attachable & ~Copyable {
27+
/// A class that stores an attachment's (potentially move-only) attachable
28+
/// value.
29+
///
30+
/// We use a class to store the attachable value so that ``Attachment`` can
31+
/// conform to `Copyable` even if `AttachableValue` doesn't.
32+
fileprivate final class Storage {
33+
/// Storage for ``Attachment/attachableValue-7dyjv``.
34+
let attachableValue: AttachableValue
35+
36+
init(_ attachableValue: consuming AttachableValue) {
37+
self.attachableValue = attachableValue
38+
}
39+
}
40+
2741
/// Storage for ``attachableValue-7dyjv``.
28-
fileprivate var _attachableValue: AttachableValue
42+
private var _storage: Storage
2943

3044
/// The path to which the this attachment was written, if any.
3145
///
@@ -80,12 +94,11 @@ public struct Attachment<AttachableValue>: ~Copyable where AttachableValue: Atta
8094
var sourceLocation: SourceLocation
8195
}
8296

83-
extension Attachment: Copyable where AttachableValue: Copyable {}
8497
extension Attachment: Sendable where AttachableValue: Sendable {}
98+
extension Attachment.Storage: Sendable where AttachableValue: Sendable {}
8599

86100
// MARK: - Initializing an attachment
87101

88-
#if !SWT_NO_LAZY_ATTACHMENTS
89102
extension Attachment where AttachableValue: ~Copyable {
90103
/// Initialize an instance of this type that encloses the given attachable
91104
/// value.
@@ -105,29 +118,12 @@ extension Attachment where AttachableValue: ~Copyable {
105118
/// @Available(Xcode, introduced: 26.0)
106119
/// }
107120
public init(_ attachableValue: consuming AttachableValue, named preferredName: String? = nil, sourceLocation: SourceLocation = #_sourceLocation) {
108-
self._attachableValue = attachableValue
121+
self._storage = Storage(attachableValue)
109122
self._preferredName = preferredName
110123
self.sourceLocation = sourceLocation
111124
}
112125
}
113126

114-
@_spi(ForToolsIntegrationOnly)
115-
extension Attachment where AttachableValue == AnyAttachable {
116-
/// Create a type-erased attachment from an instance of ``Attachment``.
117-
///
118-
/// - Parameters:
119-
/// - attachment: The attachment to type-erase.
120-
fileprivate init(_ attachment: Attachment<some Attachable & Sendable & Copyable>) {
121-
self.init(
122-
_attachableValue: AnyAttachable(wrappedValue: attachment.attachableValue),
123-
fileSystemPath: attachment.fileSystemPath,
124-
_preferredName: attachment._preferredName,
125-
sourceLocation: attachment.sourceLocation
126-
)
127-
}
128-
}
129-
#endif
130-
131127
/// A type-erased wrapper type that represents any attachable value.
132128
///
133129
/// This type is not generally visible to developers. It is used when posting
@@ -140,47 +136,45 @@ extension Attachment where AttachableValue == AnyAttachable {
140136
/// `Event.Kind.valueAttached(_:)`, otherwise it would be declared private.
141137
/// }
142138
@_spi(ForToolsIntegrationOnly)
143-
public struct AnyAttachable: AttachableWrapper, Copyable, Sendable {
144-
#if !SWT_NO_LAZY_ATTACHMENTS
145-
public typealias Wrapped = any Attachable & Sendable /* & Copyable rdar://137614425 */
146-
#else
147-
public typealias Wrapped = [UInt8]
148-
#endif
139+
public struct AnyAttachable: AttachableWrapper, Sendable, Copyable {
140+
public struct Wrapped: Sendable {}
149141

150-
public var wrappedValue: Wrapped
142+
public var wrappedValue: Wrapped {
143+
Wrapped()
144+
}
151145

152-
init(wrappedValue: Wrapped) {
153-
self.wrappedValue = wrappedValue
146+
init<A>(_ attachment: Attachment<A>) where A: Attachable & Sendable & ~Copyable {
147+
_estimatedAttachmentByteCount = { attachment.attachableValue.estimatedAttachmentByteCount }
148+
_withUnsafeBytes = { try attachment.withUnsafeBytes($0) }
149+
_preferredName = { attachment.attachableValue.preferredName(for: attachment, basedOn: $0) }
154150
}
155151

152+
/// The implementation of ``estimatedAttachmentByteCount`` borrowed from the
153+
/// original attachment.
154+
private var _estimatedAttachmentByteCount: @Sendable () -> Int?
155+
156156
public var estimatedAttachmentByteCount: Int? {
157-
wrappedValue.estimatedAttachmentByteCount
157+
_estimatedAttachmentByteCount()
158158
}
159159

160+
/// The implementation of ``withUnsafeBytes(for:_:)`` borrowed from the
161+
/// original attachment.
162+
private var _withUnsafeBytes: @Sendable ((UnsafeRawBufferPointer) throws -> Void) throws -> Void
163+
160164
public func withUnsafeBytes<R>(for attachment: borrowing Attachment<Self>, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
161-
func open<T>(_ wrappedValue: T, for attachment: borrowing Attachment<Self>) throws -> R where T: Attachable & Sendable & Copyable {
162-
let temporaryAttachment = Attachment<T>(
163-
_attachableValue: wrappedValue,
164-
fileSystemPath: attachment.fileSystemPath,
165-
_preferredName: attachment._preferredName,
166-
sourceLocation: attachment.sourceLocation
167-
)
168-
return try temporaryAttachment.withUnsafeBytes(body)
165+
var result: R!
166+
try _withUnsafeBytes { bytes in
167+
result = try body(bytes)
169168
}
170-
return try open(wrappedValue, for: attachment)
169+
return result
171170
}
172171

172+
/// The implementation of ``preferredName(for:basedOn:)`` borrowed from the
173+
/// original attachment.
174+
private var _preferredName: @Sendable (String) -> String
175+
173176
public borrowing func preferredName(for attachment: borrowing Attachment<Self>, basedOn suggestedName: String) -> String {
174-
func open<T>(_ wrappedValue: T, for attachment: borrowing Attachment<Self>) -> String where T: Attachable & Sendable & Copyable {
175-
let temporaryAttachment = Attachment<T>(
176-
_attachableValue: wrappedValue,
177-
fileSystemPath: attachment.fileSystemPath,
178-
_preferredName: attachment._preferredName,
179-
sourceLocation: attachment.sourceLocation
180-
)
181-
return temporaryAttachment.preferredName
182-
}
183-
return open(wrappedValue, for: attachment)
177+
_preferredName(suggestedName)
184178
}
185179
}
186180

@@ -215,7 +209,7 @@ extension Attachment where AttachableValue: ~Copyable {
215209
/// }
216210
@_disfavoredOverload public var attachableValue: AttachableValue {
217211
_read {
218-
yield _attachableValue
212+
yield _storage.attachableValue
219213
}
220214
}
221215
}
@@ -245,32 +239,33 @@ extension Attachment where AttachableValue: AttachableWrapper & ~Copyable {
245239

246240
// MARK: - Attaching an attachment to a test (etc.)
247241

248-
#if !SWT_NO_LAZY_ATTACHMENTS
249-
extension Attachment where AttachableValue: Sendable & Copyable {
242+
extension Attachment where AttachableValue: Sendable & ~Copyable {
250243
/// Attach an attachment to the current test.
251244
///
252245
/// - Parameters:
253246
/// - attachment: The attachment to attach.
254247
/// - sourceLocation: The source location of the call to this function.
255248
///
256-
/// When attaching a value of a type that does not conform to both
257-
/// [`Sendable`](https://developer.apple.com/documentation/swift/sendable) and
258-
/// [`Copyable`](https://developer.apple.com/documentation/swift/copyable),
259-
/// the testing library encodes it as data immediately. If the value cannot be
260-
/// encoded and an error is thrown, that error is recorded as an issue in the
261-
/// current test and the attachment is not written to the test report or to
262-
/// disk.
263-
///
264-
/// An attachment can only be attached once.
249+
/// When `attachableValue` is an instance of a type that does not conform to
250+
/// the [`Sendable`](https://developer.apple.com/documentation/swift/sendable)
251+
/// protocol, the testing library encodes it as data immediately. If
252+
/// `attachableValue` throws an error when the testing library attempts to
253+
/// encode it, the testing library records that error as an issue in the
254+
/// current test and does not write the attachment to the test report or to
255+
/// persistent storage.
265256
///
266257
/// @Metadata {
267258
/// @Available(Swift, introduced: 6.2)
268259
/// @Available(Xcode, introduced: 26.0)
269260
/// }
270261
@_documentation(visibility: private)
271262
public static func record(_ attachment: consuming Self, sourceLocation: SourceLocation = #_sourceLocation) {
272-
var attachmentCopy = Attachment<AnyAttachable>(attachment)
273-
attachmentCopy.sourceLocation = sourceLocation
263+
var attachmentCopy = Attachment<AnyAttachable>(
264+
AnyAttachable(copy attachment),
265+
named: attachment._preferredName,
266+
sourceLocation: sourceLocation
267+
)
268+
attachmentCopy.fileSystemPath = attachment.fileSystemPath
274269
Event.post(.valueAttached(attachmentCopy))
275270
}
276271

@@ -283,19 +278,17 @@ extension Attachment where AttachableValue: Sendable & Copyable {
283278
/// derive a reasonable filename for the attached value.
284279
/// - sourceLocation: The source location of the call to this function.
285280
///
286-
/// When attaching a value of a type that does not conform to both
287-
/// [`Sendable`](https://developer.apple.com/documentation/swift/sendable) and
288-
/// [`Copyable`](https://developer.apple.com/documentation/swift/copyable),
289-
/// the testing library encodes it as data immediately. If the value cannot be
290-
/// encoded and an error is thrown, that error is recorded as an issue in the
291-
/// current test and the attachment is not written to the test report or to
292-
/// disk.
281+
/// When `attachableValue` is an instance of a type that does not conform to
282+
/// the [`Sendable`](https://developer.apple.com/documentation/swift/sendable)
283+
/// protocol, the testing library encodes it as data immediately. If
284+
/// `attachableValue` throws an error when the testing library attempts to
285+
/// encode it, the testing library records that error as an issue in the
286+
/// current test and does not write the attachment to the test report or to
287+
/// persistent storage.
293288
///
294289
/// This function creates a new instance of ``Attachment`` and immediately
295290
/// attaches it to the current test.
296291
///
297-
/// An attachment can only be attached once.
298-
///
299292
/// @Metadata {
300293
/// @Available(Swift, introduced: 6.2)
301294
/// @Available(Xcode, introduced: 26.0)
@@ -305,7 +298,6 @@ extension Attachment where AttachableValue: Sendable & Copyable {
305298
record(Self(attachableValue, named: preferredName, sourceLocation: sourceLocation), sourceLocation: sourceLocation)
306299
}
307300
}
308-
#endif
309301

310302
extension Attachment where AttachableValue: ~Copyable {
311303
/// Attach an attachment to the current test.
@@ -314,32 +306,22 @@ extension Attachment where AttachableValue: ~Copyable {
314306
/// - attachment: The attachment to attach.
315307
/// - sourceLocation: The source location of the call to this function.
316308
///
317-
/// When attaching a value of a type that does not conform to both
318-
/// [`Sendable`](https://developer.apple.com/documentation/swift/sendable) and
319-
/// [`Copyable`](https://developer.apple.com/documentation/swift/copyable),
320-
/// the testing library encodes it as data immediately. If the value cannot be
321-
/// encoded and an error is thrown, that error is recorded as an issue in the
322-
/// current test and the attachment is not written to the test report or to
323-
/// disk.
324-
///
325-
/// An attachment can only be attached once.
309+
/// When `attachableValue` is an instance of a type that does not conform to
310+
/// the [`Sendable`](https://developer.apple.com/documentation/swift/sendable)
311+
/// protocol, the testing library encodes it as data immediately. If
312+
/// `attachableValue` throws an error when the testing library attempts to
313+
/// encode it, the testing library records that error as an issue in the
314+
/// current test and does not write the attachment to the test report or to
315+
/// persistent storage.
326316
///
327317
/// @Metadata {
328318
/// @Available(Swift, introduced: 6.2)
329319
/// @Available(Xcode, introduced: 26.0)
330320
/// }
331321
public static func record(_ attachment: consuming Self, sourceLocation: SourceLocation = #_sourceLocation) {
332322
do {
333-
let attachmentCopy = try attachment.withUnsafeBytes { buffer in
334-
let attachableWrapper = AnyAttachable(wrappedValue: Array(buffer))
335-
return Attachment<AnyAttachable>(
336-
_attachableValue: attachableWrapper,
337-
fileSystemPath: attachment.fileSystemPath,
338-
_preferredName: attachment.preferredName, // invokes preferredName(for:basedOn:)
339-
sourceLocation: sourceLocation
340-
)
341-
}
342-
Event.post(.valueAttached(attachmentCopy))
323+
let bufferCopy = try attachment.withUnsafeBytes { Array($0) }
324+
Attachment<Array>.record(bufferCopy, sourceLocation: sourceLocation)
343325
} catch {
344326
let sourceContext = SourceContext(backtrace: .current(), sourceLocation: sourceLocation)
345327
Issue(kind: .valueAttachmentFailed(error), comments: [], sourceContext: sourceContext).record()
@@ -355,19 +337,17 @@ extension Attachment where AttachableValue: ~Copyable {
355337
/// derive a reasonable filename for the attached value.
356338
/// - sourceLocation: The source location of the call to this function.
357339
///
358-
/// When attaching a value of a type that does not conform to both
359-
/// [`Sendable`](https://developer.apple.com/documentation/swift/sendable) and
360-
/// [`Copyable`](https://developer.apple.com/documentation/swift/copyable),
361-
/// the testing library encodes it as data immediately. If the value cannot be
362-
/// encoded and an error is thrown, that error is recorded as an issue in the
363-
/// current test and the attachment is not written to the test report or to
364-
/// disk.
340+
/// When `attachableValue` is an instance of a type that does not conform to
341+
/// the [`Sendable`](https://developer.apple.com/documentation/swift/sendable)
342+
/// protocol, the testing library encodes it as data immediately. If
343+
/// `attachableValue` throws an error when the testing library attempts to
344+
/// encode it, the testing library records that error as an issue in the
345+
/// current test and does not write the attachment to the test report or to
346+
/// persistent storage.
365347
///
366348
/// This function creates a new instance of ``Attachment`` and immediately
367349
/// attaches it to the current test.
368350
///
369-
/// An attachment can only be attached once.
370-
///
371351
/// @Metadata {
372352
/// @Available(Swift, introduced: 6.2)
373353
/// @Available(Xcode, introduced: 26.0)

Tests/TestingTests/AttachmentTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ struct AttachmentTests {
227227
return
228228
}
229229

230-
#expect(attachment.attachableValue is MySendableAttachable)
230+
#expect((attachment.attachableValue as Any) is AnyAttachable.Wrapped)
231231
#expect(attachment.sourceLocation.fileID == #fileID)
232232
valueAttached()
233233
}

0 commit comments

Comments
 (0)