Skip to content

Commit f4736a6

Browse files
authored
Remove optionality from _AttachableImageWrapper.wrappedValue on Windows. (#1258)
This PR makes `_AttachableImageWrapper.wrappedValue` non-optional on Windows (as it is on Darwin.) As currently implemented, it allows for failures when calling `CopyImage()` and `CopyIcon()`, but in practice the only way these can fail is due to heap exhaustion[^msDocs]. Swift treats allocation failures as almost universally fatal, so we should do the same. [^msDocs]: Microsoft's documentation for these functions does not list any other failure modes. ### 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 cb090d3 commit f4736a6

7 files changed

+23
-32
lines changed

Sources/Overlays/_Testing_WinSDK/Attachments/AttachableAsIWICBitmap.swift

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,14 @@ public protocol _AttachableByAddressAsIWICBitmap {
6262
/// - Returns: A copy of `imageAddress`, or `imageAddress` if this type does
6363
/// not support a copying operation.
6464
///
65-
/// - Throws: Any error that prevented copying the value at `imageAddress`.
66-
///
6765
/// The testing library uses this function to take ownership of image
6866
/// resources that test authors pass to it. If possible, make a copy of or add
6967
/// a reference to the value at `imageAddress`. If this type does not support
7068
/// making copies, return `imageAddress` verbatim.
7169
///
7270
/// This function is not part of the public interface of the testing library.
7371
/// It may be removed in a future update.
74-
static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) throws -> UnsafeMutablePointer<Self>
72+
static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) -> UnsafeMutablePointer<Self>
7573

7674
/// Manually deinitialize any resources at the given address.
7775
///
@@ -135,16 +133,14 @@ public protocol AttachableAsIWICBitmap {
135133
/// - Returns: A copy of `self`, or `self` if this type does not support a
136134
/// copying operation.
137135
///
138-
/// - Throws: Any error that prevented copying this value.
139-
///
140136
/// The testing library uses this function to take ownership of image
141137
/// resources that test authors pass to it. If possible, make a copy of or add
142138
/// a reference to `self`. If this type does not support making copies, return
143139
/// `self` verbatim.
144140
///
145141
/// This function is not part of the public interface of the testing library.
146142
/// It may be removed in a future update.
147-
func _copyAttachableValue() throws -> Self
143+
func _copyAttachableValue() -> Self
148144

149145
/// Manually deinitialize any resources associated with this image.
150146
///

Sources/Overlays/_Testing_WinSDK/Attachments/HBITMAP+AttachableAsIWICBitmap.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ extension HBITMAP__: _AttachableByAddressAsIWICBitmap {
2727
return bitmap
2828
}
2929

30-
public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) throws -> UnsafeMutablePointer<Self> {
31-
guard let result = CopyImage(imageAddress, UINT(IMAGE_BITMAP), 0, 0, 0)?.assumingMemoryBound(to: Self.self) else {
32-
throw Win32Error(rawValue: GetLastError())
33-
}
34-
return result
30+
public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) -> UnsafeMutablePointer<Self> {
31+
// The only reasonable failure mode for `CopyImage()` is allocation failure,
32+
// and Swift treats allocation failures as fatal. Hence, we do not check for
33+
// `nil` on return.
34+
CopyImage(imageAddress, UINT(IMAGE_BITMAP), 0, 0, 0).assumingMemoryBound(to: Self.self)
3535
}
3636

3737
public static func _deinitializeAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) {

Sources/Overlays/_Testing_WinSDK/Attachments/HICON+AttachableAsIWICBitmap.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ extension HICON__: _AttachableByAddressAsIWICBitmap {
2727
return bitmap
2828
}
2929

30-
public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) throws -> UnsafeMutablePointer<Self> {
31-
guard let result = CopyIcon(imageAddress) else {
32-
throw Win32Error(rawValue: GetLastError())
33-
}
34-
return result
30+
public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) -> UnsafeMutablePointer<Self> {
31+
// The only reasonable failure mode for `CopyIcon()` is allocation failure,
32+
// and Swift treats allocation failures as fatal. Hence, we do not check for
33+
// `nil` on return.
34+
CopyIcon(imageAddress)
3535
}
3636

3737
public static func _deinitializeAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) {

Sources/Overlays/_Testing_WinSDK/Attachments/IWICBitmap+AttachableAsIWICBitmap.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ extension IWICBitmap: _AttachableByAddressAsIWICBitmap {
2323
return imageAddress
2424
}
2525

26-
public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) throws -> UnsafeMutablePointer<Self> {
26+
public static func _copyAttachableValue(at imageAddress: UnsafeMutablePointer<Self>) -> UnsafeMutablePointer<Self> {
2727
_ = imageAddress.pointee.lpVtbl.pointee.AddRef(imageAddress)
2828
return imageAddress
2929
}

Sources/Overlays/_Testing_WinSDK/Attachments/ImageAttachmentError.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,12 @@
1313

1414
internal import WinSDK
1515

16-
/// A type describing errors that can be thrown by WIC or COM while attaching an
17-
/// image.
16+
/// A type representing an error that can occur when attaching an image.
1817
enum ImageAttachmentError: Error {
1918
/// A call to `QueryInterface()` failed.
2019
case queryInterfaceFailed(Any.Type, HRESULT)
2120

22-
/// The testing library failed to create a WIC object.
21+
/// The testing library failed to create a COM object.
2322
case comObjectCreationFailed(Any.Type, HRESULT)
2423

2524
/// An image could not be written.

Sources/Overlays/_Testing_WinSDK/Attachments/UnsafeMutablePointer+AttachableAsIWICBitmap.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ extension UnsafeMutablePointer: AttachableAsIWICBitmap where Pointee: _Attachabl
1919
try Pointee._copyAttachableIWICBitmap(from: self, using: factory)
2020
}
2121

22-
public func _copyAttachableValue() throws -> Self {
23-
try Pointee._copyAttachableValue(at: self)
22+
public func _copyAttachableValue() -> Self {
23+
Pointee._copyAttachableValue(at: self)
2424
}
2525

2626
public consuming func _deinitializeAttachableValue() {

Sources/Overlays/_Testing_WinSDK/Attachments/_AttachableImageWrapper.swift

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,30 +28,26 @@ internal import WinSDK
2828
@_spi(Experimental)
2929
public final class _AttachableImageWrapper<Image>: Sendable where Image: AttachableAsIWICBitmap {
3030
/// The underlying image.
31-
nonisolated(unsafe) let image: Result<Image, any Error>
31+
nonisolated(unsafe) let image: Image
3232

3333
/// The image format to use when encoding the represented image.
3434
let imageFormat: AttachableImageFormat?
3535

3636
init(image: borrowing Image, imageFormat: AttachableImageFormat?) {
37-
self.image = Result { [image = copy image] in
38-
try image._copyAttachableValue()
39-
}
37+
self.image = image._copyAttachableValue()
4038
self.imageFormat = imageFormat
4139
}
4240

4341
deinit {
44-
if let image = try? image.get() {
45-
image._deinitializeAttachableValue()
46-
}
42+
image._deinitializeAttachableValue()
4743
}
4844
}
4945

5046
// MARK: -
5147

5248
extension _AttachableImageWrapper: AttachableWrapper {
53-
public var wrappedValue: Image? {
54-
try? image.get()
49+
public var wrappedValue: Image {
50+
image
5551
}
5652

5753
public func withUnsafeBytes<R>(for attachment: borrowing Attachment<_AttachableImageWrapper>, _ body: (UnsafeRawBufferPointer) throws -> R) throws -> R {
@@ -74,7 +70,7 @@ extension _AttachableImageWrapper: AttachableWrapper {
7470
}
7571

7672
// Create the bitmap and downcast it to an IWICBitmapSource for later use.
77-
let bitmap = try image.get().copyAttachableIWICBitmapSource(using: factory)
73+
let bitmap = try image.copyAttachableIWICBitmapSource(using: factory)
7874
defer {
7975
_ = bitmap.pointee.lpVtbl.pointee.Release(bitmap)
8076
}

0 commit comments

Comments
 (0)