search

swiftlang / swift-foundation

The Foundation project
Apache License 2.0
2.4k stars 159 forks source link

Inconsistent formatting behavior when BinaryInteger cannot be converted into Int64 #186

Open joey-gm opened 1 year ago

joey-gm commented 1 year ago

ICUNumberFormatter doesn't support UInt64 number formatting. Integers not representable by Int64 are clamped as per documentation.

Under current implementation, however, if a BinaryInteger cannot be converted into Int64, IntegerFormatStyle will first attempt to convert the integer into Decimal (represented by Double) before falling back to clamping. This may result in inconsistent formatting behavior.

One edge case is 9223372036854775808 (Int64.max + 1). This integer can be converted to a floating-point value (9.223372036854776E+18), which will then be formatted into "9" instead of the the clamped number of 9,223,372,036,854,775,807 (Int64.max).

Should we drop the Decimal conversion in IntegerFormatStyle?

Personally, I would prefer to drop the Int64 clamping as well and fall back to the Swift Standard Library's LosslessStringConvertible: String(value). This will allow us to preserve the correct numeric value albeit incorrect format. Ideally, we should have a proper implementation for UInt64 number formatting.

https://github.com/apple/swift-foundation/blob/fa61cd8aeb316727f9c7d740caa675e8c6f75abf/Sources/FoundationInternationalization/Formatting/Number/IntegerFormatStyle.swift#L211-L225

https://github.com/apple/swift-foundation/blob/fa61cd8aeb316727f9c7d740caa675e8c6f75abf/Sources/FoundationInternationalization/Formatting/Number/IntegerFormatStyle.swift#L253-L254

https://github.com/apple/swift-foundation/blob/fa61cd8aeb316727f9c7d740caa675e8c6f75abf/Sources/FoundationInternationalization/Formatting/Number/IntegerFormatStyle.swift#L285-L286

https://github.com/apple/swift-foundation/blob/fa61cd8aeb316727f9c7d740caa675e8c6f75abf/Sources/FoundationInternationalization/Formatting/Number/IntegerFormatStyle.swift#L512-L513

jmschonfeld commented 1 year ago

I believe we added the Decimal conversion a little while back to support formatting values between Int64.max and UInt64.max so that numbers like UInt.max (equivalent to UInt64.max on some machines) were formatted correctly. We actually have some tests to ensure that these numbers are formatted correctly:

https://github.com/apple/swift-foundation/blob/b1f7b252df0ddbb7fa857284b1d1584940b6d477/Tests/FoundationInternationalizationTests/Formatting/NumberFormatStyleTests.swift#L1869-L1872

However, those tests are guarded by an #if FOUNDATION_FRAMEWORK because we haven't ported Decimal. Are you seeing the behavior incorrect for Int64.max + 1 somewhere? In practice I don't believe the conversion to decimal here actually produces a formatted "9" and the assertions in the above code pass, but @itingliu you might have more context here.

itingliu commented 1 year ago

Is this resolved by https://github.com/apple/swift-foundation/pull/262?