Decimal(sign:exponent:significand:) causes unexpected sign if significand is negative

[EthanHumphrey]

With the release of iOS 18 Developer Beta 5 and Public Beta 3, my team received reports from customers that all negative values in our app were showing as positives, which led to major confusion.

We narrowed down the issue to a change to the initializer for Decimal: Decimal(sign:exponent:significand). Prior to Beta 5, the sign passed into the initializer would be the sign of the Decimal. Passing .plus would result in a positive decimal, and passing .minus would result in a negative Decimal. This is regardless of the sign of the significant. In Beta 5, it seems that the sign passed into the init, and the sign of the significand are now negated. This means that passing .minus for sign and a negative Decimal for significand results in an unexpectedly positive Decimal value in Beta 5 alone. This behavior does not seem to be explicitly documented.

I created a quick playground to illustrate the issue. Here's the code:

// Expected Value: 20
// Actual Value: 20
let positiveDecimal = Decimal(sign: .plus, exponent: 1, significand: 2)

// Expected Value: 20
// Actual Value (15.4.0, 16.0 Beta 4): 20
// Actual Value (16.0 Beta 5): -20
let positiveDecimalWithNegativeSignificand = Decimal(sign: .plus, exponent: 1, significand: -2)

// Expected Value: -20
// Actual Value: -20
let negativeDecimal = Decimal(sign: .minus, exponent: 1, significand: 2)

// Expected Value: -20
// Actual Value (15.4.0, 16.0 Beta 4): -20
// Actual Value (16.0 Beta 5): 20
let negativeDecimalWithNegativeSignificand = Decimal(sign: .minus, exponent: 1, significand: -2)

I've tracked down the issue to a PR in the swift-foundation repo from 3 weeks ago, which seems to pull a commit from the swift-corelibs-foundation repo.

[vitorll]

We are facing the same issue and would love to see this fixed by next beta release of iOS 18, is there a way to get this prioritised by the team?

[oscbyspro]

While Decimal doesn't conform to it, I believe this initializer is supposed to match the one required by FloatingPoint. In that case, the new behavior correctly matches the documented behavior of:

(sign == .minus ? -1 : 1) * significand * pow(radix, exponent)

The new behavior also matches the existing behavior of Double for example:

Double (sign: .minus, exponent: 1, significand: -21) //  42
Decimal(sign: .minus, exponent: 1, significand: -21) // 210 (was: -210)

I'm sure you'll get an authoritative answer at some point, but the code for it currently resides in an extension commented as such. So, I suspect that this behavior change is an intentional correction.

swift-foundation/Sources/FoundationEssentials/Decimal/Decimal+Conformances.swift

Lines 36 to 39 in 41cbea8

	// The methods in this extension exist to match the protocol requirements of
	// FloatingPoint, even if we can't conform directly.
	@available(macOS 10.10, iOS 8.0, watchOS 2.0, tvOS 9.0, *)
	extension Decimal /* : FloatingPoint */ {

[EthanHumphrey]

I did track down the original PR on the swift-corelibs-foundation repo, and I believe you're right that this new behavior is intentional. However, it seems that this is a breaking change to all apps running on iOS 18 Beta 5+ and other platforms bundled with the latest version of Foundation, not just apps that are built with the latest toolchain. As such, I am hoping this change can be reverted for the time being until a better way to roll out this fix can be achieved.

Additionally, the documentation for this specific initializer is vague, which is pointed out in that original PR. I'd argue it implies that the sign given to the initializer will be the sign of the Decimal. Unless you have knowledge of the similar Double or FloatingPoint initializers, you'd have no clue that this isn't the case, especially since Decimal does not conform to FloatingPoint.

[iCharlesHu]

Hi @EthanHumphrey, thank you for the report. As @oscbyspro mentioned, this is indeed a longstanding bug in both Darwin and the open-source implementation of NSDecimal. The implementation of init(sign: FloatingPointSign, exponent: Int, significand: Decimal) was initially designed to implement the FloatingPoint protocol. The primary reason Decimal cannot fully conform to FloatingPoint is that it lacks a representation for infinity, it should otherwise follow the requirements of other methods.

Despite the behavioral changes, we decided to pull in this fix because it addresses a significant issue, and the new implementation is more accurate.

As a workaround, you might consider adding your own initializer to restore the previous behavior:

public init(sign: FloatingPointSign, exponent: Int, unsignedSignificand: Decimal) {
    self.init(
        _exponent: Int32(exponent) + unsignedSignificand._exponent,
        _length: unsignedSignificand._length,
        _isNegative: sign == .plus ? 0 : 1,
        _isCompact: unsignedSignificand._isCompact,
        _reserved: 0,
        _mantissa: unsignedSignificand._mantissa)
}

[KeithBauerANZ]

Breaking change in a major OS version to be more correct: good
Breaking change in any OS version that causes existing financial apps to show incorrect monetary amounts to customers: Bad.

This kind of thing really needs a linkedOnOrAfter check.

[EthanHumphrey]

Hi @iCharlesHu. Thank you for looking into this issue, the team really appreciates it. I understand that this has been a longstanding bug, and I am indeed in favor of fixing it so that it behaves correctly. However, while my team has been able to push a patch for this quickly, we know that not all teams will have the luxury of being able to do so, or may struggle to find the root cause of this issue.

As @KeithBauerANZ mentioned, this is not simply a breaking change in apps built with Xcode 16, but rather a breaking change for any app that uses this initializer and is installed on a device with iOS 18 (and other platforms). Unless all apps using this initializer are able to identify this issue and patch it on day 1, there are going to be a lot of users experiencing unexpected behavior in apps come September, some more obvious than others. The onus should be on the language and platform to ensure that existing apps' fundamental business logic is not broken by an OS update.

Like @KeithBauerANZ suggested, is there some check that we could utilize here to ensure backwards compatibility with apps that have not been built against the iOS 18 SDK, while fixing the issue going forward? What about checking for compiling in the Swift 6 language mode? If so, I'd love to update my open PR for the experience.

[KeithBauerANZ]

Apple has internal infrastructure to do a runtime check whether the running app was "linked on or after" a particular SDK version. This is normally how they stage breaking changes to existing API. It's probably necessary that this be made available to swift-foundation, but I don't know if it's available already…

However, if we could control the C headers as well as the Swift source, we could use #if compiler(>=6) and symbol renaming to achieve something similar at compile-time?

[pappalar]

Breaking change was introduced here

While it makes sense to fix a long-standing bug.
It would make more sense to announce it as a breaking change

[pappalar]

Isn't the real problem here that the API allows for this kind of assumption regarding the sign?

public init(sign: FloatingPointSign, exponent: Int, significand: Decimal) allows to provide both a sign and a significand that could both be positive or negative.

Wouldn't it make more sense to do a abs of the significand and honor the caller sign?

[xwu]

This API is Swift's spelling of the IEEE scaleB operation, which is specified for both binary and decimal floating-point types, so the new behavior is the correct one. Of note, swift-corelibs-foundation (and therefore any Linux clients) has had this correct behavior for something like 4 years.

Unfortunately, as with all things, rolling out a fix many years later in an ABI-stable world has its own complications. As others have said, documenting the change would be important, but also breaking existing clients that depend on the prior behavior isn't great. A solution employed in the standard library is defining a custom mangling for the new API and preserving the existing behavior in an ABI-stable but API-unavailable entrypoint--hopefully that can be done by the team here for Swift Foundation (or Apple's internal ingested version of it).

[alanzeino]

I don't think anyone is saying that the change isn't needed or shouldn't have been implemented.

I'd say the contention is:

• According to the ANZ engineers only showed up late in the annual beta cycle in beta 5
• There is no mention of this in the RC Release Notes right now
• There's no way for any developer who doesn't have a blocking force upgrade flow in their software to resolve this for old binaries built with older Base SDKs (because of ABI stability and because this change is default)

At the very least with four days before you ship major releases of your OSes, you would think the Release Notes would state this.

[xwu]

I don't think anyone is saying that the change isn't needed or shouldn't have been implemented.

My read is that some of the comments are saying exactly that. But I certainly agree with the pain points you mention and hope there is a way to address them adequately.

[parkera]

We're seeing what options we have for mitigating the impact to customers.

[EthanHumphrey]

With iOS 18 available as of yesterday and it still not resolving this issue, I guess we will see quite soon how widespread the impact here is.