From 0cd6e02b46344631eb146a608b5ecb427bae8ab1 Mon Sep 17 00:00:00 2001 From: Kyle Date: Wed, 20 Dec 2023 20:46:15 +0800 Subject: [PATCH] Update Subscribers.Demand (#4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update Subscribers.Demand Align the implementation with Xcode 15 SDK Combine’s swiftinterface * Update Demand.assertNonZero * [Optimize] Align with Combine interface * Fix WASM platform build issue * Fix flacky test case on Xcode 14.3.1 --- Sources/OpenCombine/AnySubscriber.swift | 6 +- Sources/OpenCombine/Helpers/Violations.swift | 10 +- .../Subscribers/Subscribers.Demand.swift | 137 ++++++++++-------- .../PublisherConcurrencyTests.swift | 4 +- .../HelpersTests/ViolationsTests.swift | 25 ++++ 5 files changed, 112 insertions(+), 70 deletions(-) create mode 100644 Tests/OpenCombineTests/HelpersTests/ViolationsTests.swift diff --git a/Sources/OpenCombine/AnySubscriber.swift b/Sources/OpenCombine/AnySubscriber.swift index e2ef8adb9..4513a118f 100644 --- a/Sources/OpenCombine/AnySubscriber.swift +++ b/Sources/OpenCombine/AnySubscriber.swift @@ -193,13 +193,13 @@ internal final class ClosureBasedAnySubscriber : AnySubscriberBase { @usableFromInline - internal let receiveSubscriptionThunk: (Subscription) -> Void + final internal let receiveSubscriptionThunk: (Subscription) -> Void @usableFromInline - internal let receiveValueThunk: (Input) -> Subscribers.Demand + final internal let receiveValueThunk: (Input) -> Subscribers.Demand @usableFromInline - internal let receiveCompletionThunk: (Subscribers.Completion) -> Void + final internal let receiveCompletionThunk: (Subscribers.Completion) -> Void @inlinable internal init(_ rcvSubscription: @escaping (Subscription) -> Void, diff --git a/Sources/OpenCombine/Helpers/Violations.swift b/Sources/OpenCombine/Helpers/Violations.swift index 1cf4121d1..78eb999d1 100644 --- a/Sources/OpenCombine/Helpers/Violations.swift +++ b/Sources/OpenCombine/Helpers/Violations.swift @@ -24,10 +24,10 @@ internal func abstractMethod(file: StaticString = #file, line: UInt = #line) -> } extension Subscribers.Demand { - internal func assertNonZero(file: StaticString = #file, - line: UInt = #line) { - if self == .none { - fatalError("API Violation: demand must not be zero", file: file, line: line) - } + @_transparent + @inline(__always) + internal func assertNonZero() { + precondition(rawValue <= Subscribers.Demand.unlimited.rawValue) + precondition(rawValue > Subscribers.Demand.none.rawValue) } } diff --git a/Sources/OpenCombine/Subscribers/Subscribers.Demand.swift b/Sources/OpenCombine/Subscribers/Subscribers.Demand.swift index 9641c635b..395c0c521 100644 --- a/Sources/OpenCombine/Subscribers/Subscribers.Demand.swift +++ b/Sources/OpenCombine/Subscribers/Subscribers.Demand.swift @@ -13,14 +13,9 @@ import _Concurrency extension Subscribers { - /// A requested number of items, sent to a publisher from a subscriber through - /// the subscription. - public struct Demand: Equatable, - Comparable, - Hashable, - Codable, - CustomStringConvertible - { + /// A requested number of items, sent to a publisher from a subscriber through the subscription. + @frozen + public struct Demand: Equatable, Comparable, Hashable, Codable, CustomStringConvertible { @usableFromInline internal let rawValue: UInt @@ -31,18 +26,12 @@ extension Subscribers { } /// A request for as many values as the publisher can produce. - @inline(__always) - @inlinable - public static var unlimited: Demand { - return Demand(rawValue: .max) - } + public static let unlimited = Demand(rawValue: .max) /// A request for no elements from the publisher. /// /// This is equivalent to `Demand.max(0)`. - @inline(__always) - @inlinable - public static var none: Demand { return .max(0) } + public static let none = Demand(rawValue: .zero) /// Creates a demand for the given maximum number of elements. /// @@ -54,7 +43,7 @@ extension Subscribers { @inline(__always) @inlinable public static func max(_ value: Int) -> Demand { - precondition(value >= 0, "demand cannot be negative") + precondition(value >= 0, "Demand cannot be negative") return Demand(rawValue: UInt(value)) } @@ -71,15 +60,21 @@ extension Subscribers { @inline(__always) @inlinable public static func + (lhs: Demand, rhs: Demand) -> Demand { - switch (lhs, rhs) { - case (.unlimited, _): - return .unlimited - case (_, .unlimited): + if lhs == .unlimited { return .unlimited - default: - let (sum, isOverflow) = Int(lhs.rawValue) - .addingReportingOverflow(Int(rhs.rawValue)) - return isOverflow ? .unlimited : .max(sum) + } else { + if rhs == .unlimited { + return .unlimited + } else { + let lhsValue: Int = numericCast(lhs.rawValue) + let rhsvalue: Int = numericCast(rhs.rawValue) + let r = lhsValue.addingReportingOverflow(rhsvalue) + if r.overflow { + return .unlimited + } else { + return .max(r.partialValue) + } + } } } @@ -99,9 +94,15 @@ extension Subscribers { public static func + (lhs: Demand, rhs: Int) -> Demand { if lhs == .unlimited { return .unlimited + } else { + let lhsValue: Int = numericCast(lhs.rawValue) + let r = lhsValue.addingReportingOverflow(rhs) + if r.overflow { + return .unlimited + } else { + return .max(r.partialValue) + } } - let (sum, isOverflow) = Int(lhs.rawValue).addingReportingOverflow(rhs) - return isOverflow ? .unlimited : .max(sum) } /// Adds an integer to a demand, and assigns the result to the demand. @@ -118,10 +119,15 @@ extension Subscribers { public static func * (lhs: Demand, rhs: Int) -> Demand { if lhs == .unlimited { return .unlimited + } else { + let lhsValue: Int = numericCast(lhs.rawValue) + let r = lhsValue.multipliedReportingOverflow(by: rhs) + if r.overflow { + return .unlimited + } else { + return .max(r.partialValue) + } } - let (product, isOverflow) = Int(lhs.rawValue) - .multipliedReportingOverflow(by: rhs) - return isOverflow ? .unlimited : .max(product) } /// Multiplies a demand by an integer, and assigns the result to the demand. @@ -141,15 +147,21 @@ extension Subscribers { @inline(__always) @inlinable public static func - (lhs: Demand, rhs: Demand) -> Demand { - switch (lhs, rhs) { - case (.unlimited, _): + if lhs == .unlimited { return .unlimited - case (_, .unlimited): - return .none - default: - let (difference, isOverflow) = Int(lhs.rawValue) - .subtractingReportingOverflow(Int(rhs.rawValue)) - return isOverflow ? .none : .max(difference) + } else { + if rhs == .unlimited { + return .none + } else { + let lhsValue: Int = numericCast(lhs.rawValue) + let rhsValue: Int = numericCast(rhs.rawValue) + let r = lhsValue.subtractingReportingOverflow(rhsValue) + if r.overflow { + return .max(0) + } else { + return .max(r.partialValue) + } + } } } @@ -175,11 +187,15 @@ extension Subscribers { public static func - (lhs: Demand, rhs: Int) -> Demand { if lhs == .unlimited { return .unlimited + } else { + let lhsValue: Int = numericCast(lhs.rawValue) + let r = lhsValue.subtractingReportingOverflow(rhs) + if r.overflow { + return .max(0) + } else { + return .max(r.partialValue) + } } - - let (difference, isOverflow) = Int(lhs.rawValue) - .subtractingReportingOverflow(rhs) - return isOverflow ? .none : .max(difference) } /// Subtracts an integer from a demand, and assigns the result to the demand. @@ -204,7 +220,7 @@ extension Subscribers { if lhs == .unlimited { return true } else { - return Int(lhs.rawValue) > rhs + return numericCast(lhs.rawValue) > rhs } } @@ -218,7 +234,7 @@ extension Subscribers { if lhs == .unlimited { return true } else { - return Int(lhs.rawValue) >= rhs + return numericCast(lhs.rawValue) >= rhs } } @@ -232,7 +248,7 @@ extension Subscribers { if rhs == .unlimited { return false } else { - return lhs > Int(rhs.rawValue) + return lhs > numericCast(rhs.rawValue) } } @@ -246,7 +262,7 @@ extension Subscribers { if rhs == .unlimited { return false } else { - return lhs >= Int(rhs.rawValue) + return lhs >= numericCast(rhs.rawValue) } } @@ -260,7 +276,7 @@ extension Subscribers { if lhs == .unlimited { return false } else { - return Int(lhs.rawValue) < rhs + return numericCast(lhs.rawValue) < rhs } } @@ -274,7 +290,7 @@ extension Subscribers { if rhs == .unlimited { return true } else { - return lhs < Int(rhs.rawValue) + return lhs < numericCast(rhs.rawValue) } } @@ -288,7 +304,7 @@ extension Subscribers { if lhs == .unlimited { return false } else { - return Int(lhs.rawValue) <= rhs + return numericCast(lhs.rawValue) <= rhs } } @@ -302,7 +318,7 @@ extension Subscribers { if rhs == .unlimited { return true } else { - return lhs <= Int(rhs.rawValue) + return lhs <= numericCast(rhs.rawValue) } } @@ -316,6 +332,8 @@ extension Subscribers { @inlinable public static func < (lhs: Demand, rhs: Demand) -> Bool { switch (lhs, rhs) { + case (.unlimited, .unlimited): + return false case (.unlimited, _): return false case (_, .unlimited): @@ -397,7 +415,7 @@ extension Subscribers { if lhs == .unlimited { return false } else { - return Int(lhs.rawValue) == rhs + return numericCast(lhs.rawValue) == rhs } } @@ -409,7 +427,7 @@ extension Subscribers { if lhs == .unlimited { return true } else { - return Int(lhs.rawValue) != rhs + return numericCast(lhs.rawValue) != rhs } } @@ -437,25 +455,20 @@ extension Subscribers { } } - @inlinable - public static func == (lhs: Demand, rhs: Demand) -> Bool { - return lhs.rawValue == rhs.rawValue - } - /// The number of requested values. /// - /// The value is `nil` if the demand is `Subscribers.Demand.unlimited`. + /// The value is `nil` if the demand is ``Subscribers/Demand/unlimited``. @inlinable public var max: Int? { if self == .unlimited { return nil } else { - return Int(rawValue) + return numericCast(rawValue) } } /// Creates a demand instance from a decoder. /// - /// - Parameter decoder: The decoder of a previously-encoded `Subscribers.Demand` + /// - Parameter decoder: The decoder of a previously-encoded ``Subscribers.Demand`` /// instance. public init(from decoder: Decoder) throws { try self.init(rawValue: decoder.singleValueContainer().decode(UInt.self)) @@ -468,6 +481,10 @@ extension Subscribers { var container = encoder.singleValueContainer() try container.encode(rawValue) } + + public static func == (lhs: Demand, rhs: Demand) -> Bool { + return lhs.rawValue == rhs.rawValue + } } } diff --git a/Tests/OpenCombineTests/ConcurrencyTests/PublisherConcurrencyTests.swift b/Tests/OpenCombineTests/ConcurrencyTests/PublisherConcurrencyTests.swift index 61596e16c..2397b6a5d 100644 --- a/Tests/OpenCombineTests/ConcurrencyTests/PublisherConcurrencyTests.swift +++ b/Tests/OpenCombineTests/ConcurrencyTests/PublisherConcurrencyTests.swift @@ -972,8 +972,8 @@ final class PublisherConcurrencyTests: XCTestCase { } XCTAssertEqual(numberOfTasksFinished, 3) - // FIXME: This test case will sometimes fail on macOS 13 + Xcode 15.0.1 - #if swift(>=5.9) && canImport(Darwin) + // FIXME: This test case will sometimes fail on macOS 13 + Xcode 15.0.1/Xcode 14.3.1 + #if swift(>=5.8) && canImport(Darwin) #else XCTAssertEqual(subscription.history, [.requested(.max(1)), .requested(.max(1))]) #endif diff --git a/Tests/OpenCombineTests/HelpersTests/ViolationsTests.swift b/Tests/OpenCombineTests/HelpersTests/ViolationsTests.swift new file mode 100644 index 000000000..29324f6f5 --- /dev/null +++ b/Tests/OpenCombineTests/HelpersTests/ViolationsTests.swift @@ -0,0 +1,25 @@ +// +// ViolationsTests.swift +// +// +// Created by Kyle on 2023/11/18. +// + +#if !OPENCOMBINE_COMPATIBILITY_TEST +@testable import OpenCombine +import XCTest + +final class ViolationsTests: XCTestCase { + func testDemandAssertNonZero() { + let d0 = Subscribers.Demand.none + let d1 = Subscribers.Demand.max(1) + let d2 = Subscribers.Demand.unlimited + + assertCrashes { + d0.assertNonZero() + } + d1.assertNonZero() + d2.assertNonZero() + } +} +#endif