Fixes to exponent handling in dec2flt - #164331

wpegden · 2026-10-14T21:45:41Z

The first issue was ignoring later exponent digits even when the resulting correct parse value could still depend on them.

The second issue was doing a subtraction in i32's that could overflow; now the subtraction is done in i64's and is then safely clamped.

I have also added comments for "non-issues"; theoretical bugs that would only be reachable on exabyte-scale inputs.

These issues (and non-issues) were discovered in an LLM-based autonomous Lean formalization project which formalized mathematical claims about the actual behavior of the dec2flt library. The code changes were written and initially reviewed by agents in that process; I have also carefully reviewed the changes and wrote the comments (and e.g., this message) myself.

@tgross35 offered to serve as a reviewer for these changes under the experimental LLM contribution policy.

r ? @tgross35

rustbot · 2026-10-14T21:45:46Z

Some changes occurred in float parsing

cc @tgross35

rustbot · 2026-10-14T21:45:49Z

Thanks for the pull request, and welcome! The Rust Project has assigned @aapoalas (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks.

Please see the contribution instructions and our LLM policy for more information.

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: libs
libs expanded to 12 candidates
Random selection from JohnTitor, Mark-Simulacrum, aapoalas

tgross35 · 2026-10-14T21:47:33Z

Can you please make the title and commit message summary something meaningful? We shouldn't have to cross reference issues to know what this is doing when seen in a list.

tgross35 · 2026-10-14T21:49:05Z

To get a quick idea of perf changes, though I expect this should be a wash on 64-bit

@bors try @rust-timer queue

Fix for issues #163674 and #163676.

The first issue was ignoring later exponent digits even when the resulting correct parse value could still depend on them. The second issue was doing a subtraction in i32's that could overflow; now the subtraction is done in i64's and is then safely clamped. I have also added comments for "non-issues"; theoretical bugs that would only be reachable on exabyte-scale inputs.

rust-bors · 2026-10-14T22:50:05Z

☀️ Try build successful (CI)
Build commit: efd703d (efd703da50c6003fa15f2aa035d829fb8f9aaa0a)
Base parent: f6c1107 (f6c1107f8be13e48465e12c0fd26e6a39fe18e4f)

rust-timer · 2026-10-14T23:47:18Z

Finished benchmarking commit (efd703d): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=iffy rustc-perf
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.2%, 0.3%]	2
Improvements ✅ (primary)	-0.8%	[-0.8%, -0.8%]	1
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.2%]	3
All ❌✅ (primary)	-0.8%	[-0.8%, -0.8%]	1

Max RSS (memory usage)

Results (primary 2.1%, secondary 4.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.1%	[2.1%, 2.1%]	1
Regressions ❌ (secondary)	4.8%	[4.8%, 4.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.1%	[2.1%, 2.1%]	1

Cycles

Results (primary 2.2%, secondary -0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.2%	[2.1%, 2.3%]	2
Regressions ❌ (secondary)	4.9%	[1.8%, 8.3%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-7.9%	[-9.5%, -6.2%]	2
All ❌✅ (primary)	2.2%	[2.1%, 2.3%]	2

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 494.567s -> 490.54s (-0.81%)
Artifact size: 406.85 MiB -> 407.16 MiB (0.08%)

aapoalas · 2026-10-15T15:50:48Z

r? @tgross35

rustbot · 2026-10-15T15:50:53Z

tgross35 is currently at their maximum review capacity.
They may take a while to respond.

tgross35

Could you run the benchmarks at library/coretests/benches/num/{flt2dec,dec2flt} before and after this PR and post the outputs? I'm thinking this may come with a small regression, which is why I'm wondering if it may be better to error on excessively long inputs.

View changes since this review

tgross35 · 2026-10-19T05:24:59Z

Can you cover f16 in these tests too? With the target_has_reliable_f16 gate like other existing tests.

yes will do. it was excluded just because of the examples rounded to zero in f16. But I think examples may exist that work across types.

tgross35 · 2026-10-19T05:29:32Z

+    assert_eq!(long.parse::<f64>().map(f64::to_bits), Ok(0x3d719799812dea11));
+    assert_eq!(long.parse::<f32>(), short.parse::<f32>());
+
+    #[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))]


Could you note why 16-bit is excluded?

tgross35 · 2026-10-19T05:31:06Z

+    let long = format!("{short}{}", "0".repeat(100_000));
+    assert_eq!(short.parse::<f64>().map(f64::to_bits), Ok(0x3d719799812dea11));
+    assert_eq!(long.parse::<f64>().map(f64::to_bits), Ok(0x3d719799812dea11));
+    assert_eq!(long.parse::<f32>(), short.parse::<f32>());


This test fails when current std is built with overflow checks, right? It passes on the playground.

Any idea if it's possible to write a test that fails without overflow checks enabled due to bad wrapping behavior?

As far as I know, the previous overflows always worked out to round the right way so that overflow-panic was the only problem; you couldn't show it with actual values.

tgross35 · 2026-10-19T05:48:05Z

    if matches!(s.first(), Some(&x) if x.is_ascii_digit()) {
        *s_ref = s.parse_digits(|digit| {
-            // no overflows here, saturate well before overflow
-            if exponent < 0x10000 {
+            // The exponent is at most 10 * 2^59 - 1, which is small enough for i64.
+            // We stop accumulating the exponent once it's larger than 2^59 - 1, knowing that
+            // a fractional part small enough for an even larger exponent to matter requires
+            // exabyte-scale input size.
+            if exponent < (1_i64 << 59) {
                exponent = 10 * exponent + digit as i64;
            }
        });
        if negative { Some(-exponent) } else { Some(exponent) }
    } else {
        *s_ref = s;
        None
    }


Compared to the current version, this adds a check for s.first(). Does that matter?

Hm, I think this may actually be possible to simplify to:

let mut parsed_any = false; *s_ref = s.parse_digits(|digit| { parsed_any = true; // rest of this block that currently exists }); if parsed_any { if negative { Some(-exponent) } else { Some(exponent) } } else { None }

since parse_digits does the check right?

tgross35 · 2026-10-19T06:01:06Z

-            // no overflows here, saturate well before overflow
-            if exponent < 0x10000 {
+            // The exponent is at most 10 * 2^59 - 1, which is small enough for i64.
+            // We stop accumulating the exponent once it's larger than 2^59 - 1, knowing that
+            // a fractional part small enough for an even larger exponent to matter requires
+            // exabyte-scale input size.
+            if exponent < (1_i64 << 59) {
                exponent = 10 * exponent + digit as i64;
            }


Should we just return an error on massive inputs, like 1 << 40, rather than trying to parse? We'd have to ask libs for input, but if this makes it easier to reason about overflow then I imagine it could be worth it. Nobody sane is using a full terabyte to encode a single float.

tgross35 · 2026-10-19T06:22:34Z

        }
        s = s.parse_digits(|digit| d.try_add_digit(digit));
-        d.decimal_point = s.len() as i32 - first.len() as i32;
+        fractional_count = first.len() - s.len();


I think the name parsed_fractional_digits may be more clear

tgross35 · 2026-10-19T06:25:57Z

+    let significant_count = d.num_digits;
    if d.num_digits != 0 {
        // Ignore the trailing zeros if there are any
        let mut n_trailing_zeros = 0;
        for &c in start[..(start.len() - s.len())].iter().rev() {
            if c == b'0' {
                n_trailing_zeros += 1;
            } else if c != b'.' {
                break;
            }
        }
-        d.decimal_point += n_trailing_zeros as i32;
        d.num_digits -= n_trailing_zeros;
-        d.decimal_point += d.num_digits as i32;
        if d.num_digits > DecimalSeq::MAX_DIGITS {
            d.truncated = true;
            d.num_digits = DecimalSeq::MAX_DIGITS;
        }
    }


significant_count doesn't seem to be the actual count of significant digits, since trailing zeros are still counted.

tgross35 · 2026-10-19T06:27:00Z

-            });
-
-            d.decimal_point += if neg_exp { -exp_num } else { exp_num };
+            explicit = parse_scientific(&mut s).unwrap_or(0);


explicit_exponent perhaps?

tgross35 · 2026-10-19T06:39:24Z

        s = s.parse_digits(|digit| d.try_add_digit(digit));
-        d.decimal_point = s.len() as i32 - first.len() as i32;
+        fractional_count = first.len() - s.len();
    }

+    let significant_count = d.num_digits;
    if d.num_digits != 0 {
        // Ignore the trailing zeros if there are any
        let mut n_trailing_zeros = 0;
        for &c in start[..(start.len() - s.len())].iter().rev() {
            if c == b'0' {
                n_trailing_zeros += 1;
            } else if c != b'.' {
                break;
            }
        }
-        d.decimal_point += n_trailing_zeros as i32;
        d.num_digits -= n_trailing_zeros;
-        d.decimal_point += d.num_digits as i32;
        if d.num_digits > DecimalSeq::MAX_DIGITS {
            d.truncated = true;
            d.num_digits = DecimalSeq::MAX_DIGITS;
        }
    }

+    let mut explicit = 0;
    if let Some((&ch, s_next)) = s.split_first() {
        if ch == b'e' || ch == b'E' {
            s = s_next;
-            let mut neg_exp = false;
-            if let Some((&ch, s_next)) = s.split_first() {
-                neg_exp = ch == b'-';
-                if ch == b'-' || ch == b'+' {
-                    s = s_next;
-                }
-            }
-            let mut exp_num = 0_i32;
-
-            s.parse_digits(|digit| {
-                if exp_num < 0x10000 {
-                    exp_num = 10 * exp_num + digit as i32;
-                }
-            });
-
-            d.decimal_point += if neg_exp { -exp_num } else { exp_num };
+            explicit = parse_scientific(&mut s).unwrap_or(0);
        }
    }
+    d.decimal_point = final_decimal_point(significant_count, fractional_count, explicit);

    for i in d.num_digits..DecimalSeq::MAX_DIGITS_WITHOUT_OVERFLOW {
        d.digits[i] = 0;
    }

    d
 }


Imho the existing version is a bit easier to follow. Could the same behavior be accomplished by replacing d.decimal_point with a decimal_point: i64 and doing the same math inline? Rather than stashing the values and computing with them in final_decimal_point. Then clamp right before assigning to d.decimal_point.

rustbot · 2026-10-19T06:47:31Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

wpegden · 2026-10-19T17:55:05Z

I'm attaching benchmarks (attachment was LLM prepared).
Geometric mean change:
dec2flt: 2.6% faster
flt2dec: 0.23% slower

t-tests show the small improvement as statistically significant (.001) and the small regression not significant (.4). So I think on performance this seems ok?

I think erroring on huge inputs would be reasonable; the current code is correct even for inputs up to 2^58. But there are inputs that make the previous code wrong even of size 65KiB or so, which I suspect is too small to want to refuse, even if clearly pathological.

I will prepare a version incorporating your other suggestions; should I go ahead and include an error on huge inputs (and is there a preferred way for how/where to do that handling?)
rust-pr-164331-statistics.txt

tgross35 · 2026-10-19T20:07:17Z

Interesting, that's a nice little win! Wonder where exactly it came from. flt2dec changes have to be noise.

If an error on huge inputs makes sense, could you do that in a separate PR? Then we can nominate it for discussion separately from the fix here since, with the limit in mind, it may be worth reworking the rest a bit.

Please paste output in the message so we don't need to download anything, unless it's huge (then consider a gist). You can hide it in <details> https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/organizing-information-with-collapsed-sections

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 14, 2026

rustbot assigned aapoalas Oct 14, 2026

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 14, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Oct 14, 2026

Auto merge of #164331 - wpegden:dec2flt-exponent-fix, r=<try>

efd703d

Fix for issues #163674 and #163676.

wpegden changed the title ~~Fix for issues #163674 and #163676.~~ Fixes to exponent handling in dec2flt Oct 14, 2026

wpegden force-pushed the dec2flt-exponent-fix branch from 14a8d75 to 1732f6e Compare October 14, 2026 21:58

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Oct 14, 2026

rustbot assigned tgross35 and unassigned aapoalas Oct 15, 2026

tgross35 requested changes Oct 19, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 19, 2026

theemathas added the llm-assisted An LLM-assisted PR as defined by the LLM policy. Requires ahead-of-time consent by assignee. label Oct 20, 2026

wpegden mentioned this pull request Oct 29, 2026

Error when parsing implausibly large float strings #165163

Open

Conversation

wpegden commented Oct 14, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 14, 2026

Uh oh!

rustbot commented Oct 14, 2026

Uh oh!

This comment has been minimized.

tgross35 commented Oct 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgross35 commented Oct 14, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Oct 14, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Oct 14, 2026

Overall result: ❌✅ regressions and improvements - please read:

Uh oh!

aapoalas commented Oct 15, 2026

Uh oh!

rustbot commented Oct 15, 2026

Uh oh!

tgross35 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Oct 19, 2026

Uh oh!

wpegden commented Oct 19, 2026

Uh oh!

tgross35 commented Oct 19, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wpegden commented Oct 14, 2026 •

edited by rustbot

Loading

tgross35 commented Oct 14, 2026 •

edited

Loading

tgross35 left a comment •

edited by rustbot

Loading