From 3bc180c50063c16941408210f9f2f1e1d6681e59 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Tue, 21 Jul 2026 17:55:46 +0200 Subject: [PATCH] fix(python): detect radix prefix after whitespace and sign `preprocess_numeric_string` detected the radix on the raw string but stripped the prefix from the trimmed one, so anything before the `0x`/`0o`/`0b` specifier hid it. FSharp.Core consumes both leading whitespace and the sign before looking for the specifier, so two inputs that work on .NET failed: int " 0x11" // .NET 17, Fable ValueError int "-0x11" // .NET -17, Fable ValueError Trim and split a leading `-` first, detect the radix on what remains, then re-attach the sign. Only `-` is split, matching FSharp.Core: `int "+0x11"` is a format error on .NET while `int "+11"` is not. Fixing the ordering surfaced a latent bug in `parse_int64`, which routed every non-decimal radix through `u64::from_str_radix`. That cannot accept a minus sign, so `int64 "-0x11"` still failed; it had only been hidden because the prefix was never detected. Negative non-decimal values now take the i64 path, leaving the u64 two's complement path for unsigned values that set the high bit (`0xFFFFFFFFFFFFFFFF` is still -1). Expectations for every new case were verified against .NET first. The F# assertions are added to the JS suite too, where they already pass, so the two targets stay aligned. Cases where Fable is more permissive than .NET are left alone and reported separately in #4827. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/fable-library-py/src/ints.rs | 37 ++++++++++++++++-- src/fable-library-py/tests/test_ints.py | 52 +++++++++++++++++++++++++ tests/Js/Main/ConvertTests.fs | 7 ++++ tests/Python/TestConvert.fs | 7 ++++ 4 files changed, 99 insertions(+), 4 deletions(-) diff --git a/src/fable-library-py/src/ints.rs b/src/fable-library-py/src/ints.rs index eeb7b5a042..1c41dcce1e 100644 --- a/src/fable-library-py/src/ints.rs +++ b/src/fable-library-py/src/ints.rs @@ -1112,13 +1112,37 @@ fn create_parse_error(string: &str) -> PyErr { )) } +/// Splits an optional leading `-` from the rest of the string +/// +/// FSharp.Core consumes the sign before looking for a `0x`/`0o`/`0b` +/// specifier, so `-0x11` is -17. Only `-` is split: `+` is left in place for +/// the underlying parser, which matches `int "+0x11"` being a format error on +/// .NET while `int "+11"` is not. +#[inline] +fn split_sign(string: &str) -> (&str, &str) { + match string.strip_prefix('-') { + Some(digits) => ("-", digits), + None => ("", string), + } +} + /// Preprocesses a numeric string for parsing by handling radix detection, /// whitespace trimming, prefix removal, and underscore cleanup fn preprocess_numeric_string(string: &str, style: i32, default_radix: i32) -> (String, i32) { - let actual_radix = determine_radix(string, style, default_radix); + // Trim and split the sign *before* detecting the radix: the prefix follows + // both, so inspecting the raw string would miss " 0x11" and "-0x11", which + // are 17 and -17 on .NET let trimmed = trim_whitespace(string, style); - let without_prefix = remove_prefix(trimmed, actual_radix); - let final_string = without_prefix.replace('_', ""); + let (sign, unsigned) = split_sign(trimmed); + let actual_radix = determine_radix(unsigned, style, default_radix); + let digits = remove_prefix(unsigned, actual_radix).replace('_', ""); + + // Avoid re-allocating for the common unsigned case + let final_string = if sign.is_empty() { + digits + } else { + format!("{sign}{digits}") + }; (final_string, actual_radix) } @@ -1275,11 +1299,16 @@ pub fn parse_int64( let (final_string, actual_radix) = preprocess_numeric_string(string, style, radix); // Parse the integer - handle large hex values by parsing as u64 first for non-decimal - let v = if actual_radix != 10 { + let v = if actual_radix != 10 && !final_string.starts_with('-') { // For non-decimal, parse as u64 first to handle large hex values u64::from_str_radix(&final_string, actual_radix as u32) .map(|u_val| u_val as i64) // Cast performs automatic two's complement conversion .map_err(|_| create_parse_error(string))? + } else if actual_radix != 10 { + // A negative non-decimal literal such as "-0x11" is a plain signed + // value, not a two's complement bit pattern, so u64 cannot parse it + i64::from_str_radix(&final_string, actual_radix as u32) + .map_err(|_| create_parse_error(string))? } else { // For decimal, use standard i64 parsing i64::from_str_radix(&final_string, actual_radix as u32) diff --git a/src/fable-library-py/tests/test_ints.py b/src/fable-library-py/tests/test_ints.py index 1159cff3dd..2f14e93462 100644 --- a/src/fable-library-py/tests/test_ints.py +++ b/src/fable-library-py/tests/test_ints.py @@ -70,6 +70,58 @@ def test_int32_try_parse_multibyte_unicode_returns_false(style: int) -> None: assert result.contents == 0 +# NumberStyles.Any (511) is what the compiler emits for `int "..."`. Leading +# whitespace and the sign both precede the radix specifier, so they have to be +# stripped before the 0x/0o/0b prefix is looked for. Expectations verified +# against .NET. +@pytest.mark.parametrize( + ("text", "expected"), + [ + ("0x11", 17), + (" 0x11", 17), + ("0x11 ", 17), + (" 0o11 ", 9), + (" 0b11 ", 3), + ("-0x11", -17), + (" -0x11 ", -17), + (" -0b11 ", -3), + ("-1_1", -11), + ("+11", 11), + ("-11", -11), + ], +) +def test_int32_parse_prefix_after_whitespace_and_sign(text: str, expected: int) -> None: + assert int32.parse(text, 511) == expected + + +@pytest.mark.parametrize( + ("text", "expected"), + [ + ("-0x11", -17), + (" -0x11 ", -17), + ("-0b11", -3), + ("0x11", 17), + ], +) +def test_int64_parse_signed_non_decimal(text: str, expected: int) -> None: + # A negative non-decimal literal is a plain signed value, not a two's + # complement bit pattern, so it cannot go through the u64 fast path + assert int64.parse(text, 511) == expected + + +def test_int64_parse_large_hex_is_twos_complement() -> None: + # The u64 fast path must still apply to unsigned values that set the high bit + assert int64.parse("FFFFFFFFFFFFFFFF", 515) == -1 + assert int64.parse("7FFFFFFFFFFFFFFF", 515) == 9223372036854775807 + + +@pytest.mark.parametrize("text", ["+0x11", "0x", "-", "foo", "–"]) +def test_int32_parse_rejects_invalid(text: str) -> None: + # `int "+0x11"` is a format error on .NET: only `-` precedes the specifier + with pytest.raises(ValueError): + int32.parse(text, 511) + + def test_byte_add() -> None: assert byte(42) + byte(42) == 84 assert byte(255) + byte(1) == 0 diff --git a/tests/Js/Main/ConvertTests.fs b/tests/Js/Main/ConvertTests.fs index a1b368c55f..2b0bd9a640 100644 --- a/tests/Js/Main/ConvertTests.fs +++ b/tests/Js/Main/ConvertTests.fs @@ -195,6 +195,13 @@ let tests = equal 17 (int "0x11") equal 9 (int "0o11") equal 3 (int "0b11") + // Leading whitespace and the sign both precede the radix specifier + equal 17 (int " 0x11") + equal 9 (int " 0o11 ") + equal -17 (int "-0x11") + equal -3 (int " -0b11 ") + equal -17L (int64 "-0x11") + equal -11 (int "-1_1") testCase "System.Int32.TryParse works" <| fun () -> tryParse Int32.TryParse 0 "1" |> equal (true, 1) diff --git a/tests/Python/TestConvert.fs b/tests/Python/TestConvert.fs index 32b2283437..7c69a5e425 100644 --- a/tests/Python/TestConvert.fs +++ b/tests/Python/TestConvert.fs @@ -201,6 +201,13 @@ let ``test Parsing integers with different radices works`` () = equal 17 (int "0x11") equal 9 (int "0o11") equal 3 (int "0b11") + // Leading whitespace and the sign both precede the radix specifier + equal 17 (int " 0x11") + equal 9 (int " 0o11 ") + equal -17 (int "-0x11") + equal -3 (int " -0b11 ") + equal -17L (int64 "-0x11") + equal -11 (int "-1_1") [] let ``test System.Int32.TryParse works`` () =