Skip to content
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.

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

Converting negative bigint numbers to decimal does not work #3500

Closed
ThisFunctionalTom opened this issue Jul 30, 2023 · 4 comments
Closed

Converting negative bigint numbers to decimal does not work #3500

ThisFunctionalTom opened this issue Jul 30, 2023 · 4 comments

Comments

@ThisFunctionalTom
Copy link
Contributor

ThisFunctionalTom commented Jul 30, 2023

Description

Converting Decimal.MinValue to bigint and back to decimal returns -1 instead of Decimal.MinValue.

Actually, I just found out that all negative bigint numbers cannot be converted to decimal.

Repro code

Added two tests for the edge cases in this PR

Expected and actual results

As defined in the tests in PR.

Related information

  • Fable version: latest main branch
  • Operating system: Windows
@ThisFunctionalTom ThisFunctionalTom changed the title Convert Decimal.MinValue to bigint and back to decimal does not work Convertin negative bigint numbers to decimal does not work Jul 30, 2023
@ThisFunctionalTom ThisFunctionalTom changed the title Convertin negative bigint numbers to decimal does not work Converting negative bigint numbers to decimal does not work Jul 30, 2023
ThisFunctionalTom pushed a commit to ThisFunctionalTom/fsharp-hedgehog that referenced this issue Jul 30, 2023
@ThisFunctionalTom
Copy link
Contributor Author

I created the smallest repro in fable REPL:

open System

let actual = decimal -1I
let expected = -1M
printfn $"Actual: {actual} <> Expected: {expected}"

https://fable.io/repl/#?code=PTAEHUCcEsBcFNQGMD2ATRLKgDYoIZqj6gDO+AtgA46IBmkKFZ0GARvpALABQKV8AHagAygE9SCCr161YxJLACu+HKAC8oDEmgVVoALQBGAJKz48+AA8Bi+EU3GAsryoxBsOsIAkAIgCCiio4AFygAN74QaoAvqAAPAB8oACiNvB2aGHh1rYIaDG+MjxAA&html=Q&css=Q

@ThisFunctionalTom
Copy link
Contributor Author

I followed the rabbit into the hole and I believe that the problem is in toDecimal function in the BigInt.js file in fable-library.

export function toDecimal(x) {
    const low = Number(BigInt.asUintN(32, x));
    const mid = Number(BigInt.asUintN(32, x >> 32n));
    const high = Number(BigInt.asUintN(32, x >> 64n));
    const isNegative = x < zero;
    const scale = 0;
    return fromParts(low, mid, high, isNegative, scale);
}

Executing this test code:

const x = fromString("-79228162514264337593543950335");
const low = Number(BigInt.asUintN(32, x));
const mid = Number(BigInt.asUintN(32, x >> 32n));
const high = Number(BigInt.asUintN(32, x >> 64n));
const isNegative = x < 0n;
const scale = 0;
const actual = fromParts(low, mid, high, isNegative, scale);
const expected = fromParts(-1, -1, -1, true, 0);
toConsole(interpolate("x: %A%P()", [x]));
toConsole(interpolate("low: %A%P()", [low]));
toConsole(interpolate("mid: %A%P()", [mid]));
toConsole(interpolate("high: %A%P()", [high]));
toConsole(interpolate("isNegative: %A%P()", [isNegative]));
toConsole(interpolate("Actual: %A%P()", [actual]));
toConsole(interpolate("Expected: %A%P()", [expected]));
Expect_isTrue(true)("");

the low, mid, high values are 1, 0, 0 respectively and the code generated from fable for the same number is actually -1, -1, -1.

ncave added a commit to ncave/Fable that referenced this issue Jun 13, 2024
@ncave ncave mentioned this issue Jun 13, 2024
ncave added a commit to ncave/Fable that referenced this issue Jun 13, 2024
@ncave ncave closed this as completed in 6ae8390 Jun 13, 2024
@ncave
Copy link
Collaborator

ncave commented Jun 13, 2024

@ThisFunctionalTom Sorry it took so long, somehow I didn't notice this and it fell through the cracks.
Should be fixed in the next release.

@MangelMaxime
Copy link
Member

This has been released with Fable 4.19.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants