Skip to content

Commit 17b7ce0

Browse files
committed
Avoid eagerly serializing move-only attachments.
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.
1 parent 1be6b35 commit 17b7ce0

File tree

2 files changed

+73
-82
lines changed

2 files changed

+73
-82
lines changed

Sources/Testing/Attachments/Attachment.swift

Lines changed: 72 additions & 81 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 {
27-
/// Storage for ``attachableValue-7dyjv``.
28-
fileprivate var _attachableValue: AttachableValue
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+
41+
/// Storage for ``attachableValue``.
42+
fileprivate 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,7 +118,7 @@ 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
}
@@ -117,16 +130,15 @@ extension Attachment where AttachableValue == AnyAttachable {
117130
///
118131
/// - Parameters:
119132
/// - attachment: The attachment to type-erase.
120-
fileprivate init(_ attachment: Attachment<some Attachable & Sendable & Copyable>) {
133+
fileprivate init(_ attachment: Attachment<some Attachable & Sendable & ~Copyable>) {
121134
self.init(
122-
_attachableValue: AnyAttachable(wrappedValue: attachment.attachableValue),
135+
storage: Storage(AnyAttachable(attachment)),
123136
fileSystemPath: attachment.fileSystemPath,
124137
_preferredName: attachment._preferredName,
125138
sourceLocation: attachment.sourceLocation
126139
)
127140
}
128141
}
129-
#endif
130142

131143
/// A type-erased wrapper type that represents any attachable value.
132144
///
@@ -140,47 +152,45 @@ extension Attachment where AttachableValue == AnyAttachable {
140152
/// `Event.Kind.valueAttached(_:)`, otherwise it would be declared private.
141153
/// }
142154
@_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
155+
public struct AnyAttachable: AttachableWrapper, Sendable, Copyable {
156+
public struct Wrapped: Sendable {}
149157

150-
public var wrappedValue: Wrapped
158+
public var wrappedValue: Wrapped {
159+
Wrapped()
160+
}
151161

152-
init(wrappedValue: Wrapped) {
153-
self.wrappedValue = wrappedValue
162+
init<A>(_ attachment: Attachment<A>) where A: Attachable & Sendable & ~Copyable {
163+
_estimatedAttachmentByteCount = { attachment.attachableValue.estimatedAttachmentByteCount }
164+
_withUnsafeBytes = { try attachment.withUnsafeBytes($0) }
165+
_preferredName = { attachment.attachableValue.preferredName(for: attachment, basedOn: $0) }
154166
}
155167

168+
/// The implementation of ``estimatedAttachmentByteCount`` borrowed from the
169+
/// original attachment.
170+
private var _estimatedAttachmentByteCount: @Sendable () -> Int?
171+
156172
public var estimatedAttachmentByteCount: Int? {
157-
wrappedValue.estimatedAttachmentByteCount
173+
_estimatedAttachmentByteCount()
158174
}
159175

176+
/// The implementation of ``withUnsafeBytes(for:_:)`` borrowed from the
177+
/// original attachment.
178+
private var _withUnsafeBytes: @Sendable ((UnsafeRawBufferPointer) throws -> Void) throws -> Void
179+
160180
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)
181+
var result: R!
182+
try _withUnsafeBytes { bytes in
183+
result = try body(bytes)
169184
}
170-
return try open(wrappedValue, for: attachment)
185+
return result
171186
}
172187

188+
/// The implementation of ``preferredName(for:basedOn:)`` borrowed from the
189+
/// original attachment.
190+
private var _preferredName: @Sendable (String) -> String
191+
173192
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)
193+
_preferredName(suggestedName)
184194
}
185195
}
186196

@@ -215,7 +225,7 @@ extension Attachment where AttachableValue: ~Copyable {
215225
/// }
216226
@_disfavoredOverload public var attachableValue: AttachableValue {
217227
_read {
218-
yield _attachableValue
228+
yield storage.attachableValue
219229
}
220230
}
221231
}
@@ -245,23 +255,20 @@ extension Attachment where AttachableValue: AttachableWrapper & ~Copyable {
245255

246256
// MARK: - Attaching an attachment to a test (etc.)
247257

248-
#if !SWT_NO_LAZY_ATTACHMENTS
249-
extension Attachment where AttachableValue: Sendable & Copyable {
258+
extension Attachment where AttachableValue: Sendable & ~Copyable {
250259
/// Attach an attachment to the current test.
251260
///
252261
/// - Parameters:
253262
/// - attachment: The attachment to attach.
254263
/// - sourceLocation: The source location of the call to this function.
255264
///
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.
265+
/// When `attachableValue` is an instance of a type that does not conform to
266+
/// the [`Sendable`](https://developer.apple.com/documentation/swift/sendable)
267+
/// protocol, the testing library encodes it as data immediately. If
268+
/// `attachableValue` throws an error when the testing library attempts to
269+
/// encode it, the testing library records that error as an issue in the
270+
/// current test and does not write the attachment to the test report or to
271+
/// persistent storage.
265272
///
266273
/// @Metadata {
267274
/// @Available(Swift, introduced: 6.2)
@@ -294,8 +301,6 @@ extension Attachment where AttachableValue: Sendable & Copyable {
294301
/// This function creates a new instance of ``Attachment`` and immediately
295302
/// attaches it to the current test.
296303
///
297-
/// An attachment can only be attached once.
298-
///
299304
/// @Metadata {
300305
/// @Available(Swift, introduced: 6.2)
301306
/// @Available(Xcode, introduced: 26.0)
@@ -305,7 +310,6 @@ extension Attachment where AttachableValue: Sendable & Copyable {
305310
record(Self(attachableValue, named: preferredName, sourceLocation: sourceLocation), sourceLocation: sourceLocation)
306311
}
307312
}
308-
#endif
309313

310314
extension Attachment where AttachableValue: ~Copyable {
311315
/// Attach an attachment to the current test.
@@ -314,32 +318,21 @@ extension Attachment where AttachableValue: ~Copyable {
314318
/// - attachment: The attachment to attach.
315319
/// - sourceLocation: The source location of the call to this function.
316320
///
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.
321+
/// When attaching a value of a type that does not conform to the
322+
/// [`Sendable`](https://developer.apple.com/documentation/swift/sendable)
323+
/// protocol, the testing library encodes it as data immediately. If the value
324+
/// cannot be encoded and an error is thrown, that error is recorded as an
325+
/// issue in the current test and the attachment is not written to the test
326+
/// report or to disk.
326327
///
327328
/// @Metadata {
328329
/// @Available(Swift, introduced: 6.2)
329330
/// @Available(Xcode, introduced: 26.0)
330331
/// }
331332
public static func record(_ attachment: consuming Self, sourceLocation: SourceLocation = #_sourceLocation) {
332333
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))
334+
let bufferCopy = try attachment.withUnsafeBytes { Array($0) }
335+
Attachment<Array>.record(bufferCopy, sourceLocation: sourceLocation)
343336
} catch {
344337
let sourceContext = SourceContext(backtrace: .current(), sourceLocation: sourceLocation)
345338
Issue(kind: .valueAttachmentFailed(error), comments: [], sourceContext: sourceContext).record()
@@ -355,19 +348,17 @@ extension Attachment where AttachableValue: ~Copyable {
355348
/// derive a reasonable filename for the attached value.
356349
/// - sourceLocation: The source location of the call to this function.
357350
///
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.
351+
/// When `attachableValue` is an instance of a type that does not conform to
352+
/// the [`Sendable`](https://developer.apple.com/documentation/swift/sendable)
353+
/// protocol, the testing library encodes it as data immediately. If
354+
/// `attachableValue` throws an error when the testing library attempts to
355+
/// encode it, the testing library records that error as an issue in the
356+
/// current test and does not write the attachment to the test report or to
357+
/// persistent storage.
365358
///
366359
/// This function creates a new instance of ``Attachment`` and immediately
367360
/// attaches it to the current test.
368361
///
369-
/// An attachment can only be attached once.
370-
///
371362
/// @Metadata {
372363
/// @Available(Swift, introduced: 6.2)
373364
/// @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)