https://github.com/apple/swift-foundation/issues/833 Skip to content Navigation Menu Toggle navigation Sign in * Product + Actions Automate any workflow + Packages Host and manage packages + Security Find and fix vulnerabilities + Codespaces Instant dev environments + GitHub Copilot Write better code with AI + Code review Manage code changes + Issues Plan and track work + Discussions Collaborate outside of code Explore + All features + Documentation + GitHub Skills + Blog * Solutions By size + Enterprise + Teams + Startups By industry + Healthcare + Financial services + Manufacturing By use case + CI/CD & Automation + DevOps + DevSecOps * Resources Topics + AI + DevOps + Security + Software Development + View all Explore + Learning Pathways + White papers, Ebooks, Webinars + Customer Stories + Partners * Open Source + GitHub Sponsors Fund open source developers + The ReadME Project GitHub community articles Repositories + Topics + Trending + Collections * Enterprise + Enterprise platform AI-powered developer platform Available add-ons + Advanced Security Enterprise-grade security features + GitHub Copilot Enterprise-grade AI features + Premium Support Enterprise-grade 24/7 support * Pricing Search or jump to... Search code, repositories, users, issues, pull requests... Search [ ] Clear Search syntax tips Provide feedback We read every piece of feedback, and take your input very seriously. [ ] [ ] Include my email address so I can be contacted Cancel Submit feedback Saved searches Use saved searches to filter your results more quickly Name [ ] Query [ ] To see all available qualifiers, see our documentation. Cancel Create saved search Sign in Sign up Reseting focus You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session. You switched accounts on another tab or window. Reload to refresh your session. Dismiss alert {{ message }} apple / swift-foundation Public * Notifications You must be signed in to change notification settings * Fork 150 * Star 2.4k * Code * Issues 75 * Pull requests 29 * Security * Insights Additional navigation options * Code * Issues * Pull requests * Security * Insights New issue Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community. Sign up for GitHub By clicking "Sign up for GitHub", you agree to our terms of service and privacy statement. We'll occasionally send you account related emails. Already on GitHub? Sign in to your account Jump to bottom Decimal(sign:exponent:significand:) causes unexpected sign if significand is negative #833 Open EthanHumphrey opened this issue Aug 8, 2024 * 12 comments * May be fixed by #834 Open Decimal(sign:exponent:significand:) causes unexpected sign if significand is negative #833 EthanHumphrey opened this issue Aug 8, 2024 * 12 comments * May be fixed by #834 Assignees @iCharlesHu Comments @EthanHumphrey Copy link EthanHumphrey commented Aug 8, 2024 * edited Loading 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. The text was updated successfully, but these errors were encountered: 15 vitorll, petergoldsmith-anzx, adebarge, rpassis, stanchito, The-MM, adamjcampbell, eddiekerr, JCNalder, timsutton, and 5 more reacted with thumbs up emoji 4 KiraFleming, eddiekerr, yaroslavyaroslav, and bartekpacia reacted with laugh emoji 2 KiraFleming and eddiekerr reacted with rocket emoji All reactions * 15 reactions * 4 reactions * 2 reactions @EthanHumphrey EthanHumphrey linked a pull request Aug 8, 2024 that will close this issue Fix unexpected Decimal sign behavior from Decimal (sign:exponent:significand:) #834 Open @vitorll Copy link vitorll commented Aug 9, 2024 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? 1 adebarge reacted with thumbs up emoji All reactions * 1 reaction Sorry, something went wrong. @oscbyspro Copy link Contributor oscbyspro commented Aug 9, 2024 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 */ { All reactions Sorry, something went wrong. @EthanHumphrey Copy link Author EthanHumphrey commented Aug 9, 2024 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. 8 mitchell-anz, petergoldsmith-anzx, vitorll, luispadron, mormahr, Jon889, frozendevil, and NiklasBr reacted with thumbs up emoji All reactions * 8 reactions Sorry, something went wrong. @EthanHumphrey EthanHumphrey changed the title [DEL:Decimal (sign:exponent:significand:) causes incorrect sign if significand is negative:DEL] [INS:Decimal(sign:exponent:significand:) causes unexpected sign if significand is negative:INS] Aug 9, 2024 @iCharlesHu iCharlesHu self-assigned this Aug 9, 2024 @iCharlesHu Copy link Contributor iCharlesHu commented Aug 14, 2024 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) } All reactions Sorry, something went wrong. @KeithBauerANZ Copy link KeithBauerANZ commented Aug 15, 2024 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. 6 KiraFleming, alanzeino, jozefizso, NiklasBr, CrazySqueak, and Lillecarl reacted with thumbs up emoji [?] 3 EthanHumphrey, vitorll, and mormahr reacted with heart emoji All reactions * 6 reactions * [?] 3 reactions Sorry, something went wrong. @EthanHumphrey Copy link Author EthanHumphrey commented Aug 15, 2024 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. All reactions Sorry, something went wrong. @KeithBauerANZ Copy link KeithBauerANZ commented Aug 18, 2024 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? All reactions Sorry, something went wrong. @pappalar Copy link pappalar commented Sep 10, 2024 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 All reactions Sorry, something went wrong. @pappalar Copy link pappalar commented Sep 10, 2024 * edited Loading 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? All reactions Sorry, something went wrong. @xwu Copy link xwu commented Sep 11, 2024 * edited Loading 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). 1 conradev reacted with thumbs up emoji All reactions * 1 reaction Sorry, something went wrong. @alanzeino Copy link alanzeino commented Sep 11, 2024 * edited Loading 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. All reactions Sorry, something went wrong. @xwu Copy link xwu commented Sep 11, 2024 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. All reactions Sorry, something went wrong. Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment Assignees @iCharlesHu iCharlesHu Labels None yet Projects None yet Milestone No milestone Development Successfully merging a pull request may close this issue. Fix unexpected Decimal sign behavior from Decimal (sign:exponent:significand:) EthanHumphrey/swift-foundation 8 participants @xwu @vitorll @alanzeino @pappalar @iCharlesHu @EthanHumphrey @oscbyspro @KeithBauerANZ Footer (c) 2024 GitHub, Inc. Footer navigation * Terms * Privacy * Security * Status * Docs * Contact * Manage cookies * Do not share my personal information You can't perform that action at this time.