swift-foundation icon indicating copy to clipboard operation
swift-foundation copied to clipboard

Inconsistent formatting behavior when BinaryInteger cannot be converted into Int64

Open joey-gm opened this issue 2 years ago • 2 comments

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?

  • IntegerFormatStyle: Lines 221-222
  • IntegerFormatStyle.Percent: Lines 253-254
  • IntegerFormatStyle.Currency: Lines 285-286
  • AttributedString: Lines 512-513

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

joey-gm avatar Jun 25 '23 17:06 joey-gm

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.

jmschonfeld avatar Jun 27 '23 18:06 jmschonfeld

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

itingliu avatar Nov 07 '23 22:11 itingliu