flt2dec front of format_short & format_fixed by pascaldekloe · Pull Request #154505 · rust-lang/rust

pascaldekloe · 2026-03-28T13:13:05Z

The fallback to Dragon was placed in the Grisu module. As a result tests don't test what they appear to test, and some benchmarks from Grisu measured Dragon instead.

The corrected values in the tests haven been verified against the Go implementation.

rustbot · 2026-03-28T13:13:09Z

Some changes occurred in float parsing

cc @tgross35

rustbot · 2026-03-28T13:13:11Z

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: @scottmcm, libs
@scottmcm, libs expanded to 8 candidates
Random selection from Mark-Simulacrum, joboet, scottmcm

tgross35

Thank you for preparing this!

I only had time to go through the first commit here, which mostly looks pretty good to me with a few small requests. If you'd like to fix those then put only the first commit in a separate PR, I'd be happy to approve that bit and shrink the remaining todo here.

As a noncritical note, I'm having a bit of trouble understanding the commit summaries. The messages usually have details, but I'm not entirely sure what something like "write out of flt2dec/random.rs" or "test flt2dec front of format_short & format_fixed" means. We don't really have a guideline but sticking to the usual style with changes described in imperative mood would help a bit (e.g. something like flt2dec: test: Don't use `write!` in random tests or flt2dec: test: Test `format_{short,fixed}` via the main entrypoint).

I'll review the rest a little later.

View changes since this review

tgross35 · 2026-03-28T19:59:59Z

-# Problem statement
-
-We are given the floating-point number `v = f * 2^e` with an integer `f`,
-and its bounds `minus` and `plus` such that any number between `v - minus` and
-`v + plus` will be rounded to `v`. For the simplicity we assume that
-this range is exclusive. Then we would like to get the unique decimal
-representation `V = 0.d[0..n-1] * 10^k` such that:
-
- `d[0]` is non-zero.
-
- It's correctly rounded when parsed back: `v - minus < V < v + plus`.
-  Furthermore it is shortest such one, i.e., there is no representation
-  with less than `n` digits that is correctly rounded.
-
- It's closest to the original value: `abs(V - v) <= 10^(k-n) / 2`. Note that
-  there might be two representations satisfying this uniqueness requirement,
-  in which case some tie-breaking mechanism is used.
-
-We will call this mode of operation as to the *shortest* mode. This mode is used
-when there is no additional constraint, and can be thought as a "natural" mode
-as it matches the ordinary intuition (it at least prints `0.1f32` as "0.1").
-
-We have two more modes of operation closely related to each other. In these modes
-we are given either the number of significant digits `n` or the last-digit
-limitation `limit` (which determines the actual `n`), and we would like to get
-the representation `V = 0.d[0..n-1] * 10^k` such that:
-
- `d[0]` is non-zero, unless `n` was zero in which case only `k` is returned.
-
- It's closest to the original value: `abs(V - v) <= 10^(k-n) / 2`. Again,
-  there might be some tie-breaking mechanism.
-
-When `limit` is given but not `n`, we set `n` such that `k - n = limit`
-so that the last digit `d[n-1]` is scaled by `10^(k-n) = 10^limit`.
-If such `n` is negative, we clip it to zero so that we will only get `k`.
-We are also limited by the supplied buffer. This limitation is used to print
-the number up to given number of fractional digits without knowing
-the correct `k` beforehand.
-
-We will call the mode of operation requiring `n` as to the *exact* mode,
-and one requiring `limit` as to the *fixed* mode. The exact mode is a subset of
-the fixed mode: the sufficiently large last-digit limitation will eventually fill
-the supplied buffer and let the algorithm to return.
-
-# Implementation overview
-


Commit 1: It's fine to delete the short vs. fixed info here , but having an overview of how float formatting works is still nice. Also it's helpful to introduce notation, something like https://github.com/pascaldekloe/rust/blob/40811931b23b70db03103c851ca9ce3290a3201e/library/core/src/num/imp/dec2flt/mod.rs#L71-L81 would be better yet than what's preexisting.

It is documented on the two functions themselves now. We will lose the "helper" functions thus the remainder will be very readable, i.e., they no longer require overviews once organised.

I'll follow the Lemire notation, because why not.

Could you at least restore the removed portion of the module-level documentation, with changes to naming/notation as you see fit?

Organization isn't at odds with a module description, I don't want to lose the "readme" effect that can give somebody an understanding of the algorithms without needing to look at its API. There is also some helpful information here that we just don't gain anything by deleting.

tgross35 · 2026-03-28T20:31:06Z

    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_small_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(3.141592f64);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_big_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(f64::MAX);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_one_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(1.0);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_trailing_zero_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(250.000000000000000000000000);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_halfway_point_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(1.00000000000000011102230246251565404236316680908203125);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
+        format_fixed(black_box(&decoded), &mut buf, i16::MIN).unwrap();


Commit 1: Please explain what's going on with the removals here in the commit message

That is what I ment in point 4 of the commit message "Drop of Grisu benchmarks which's value caused Dragon". The numeric value of the deleted benchmarks doesn't have a correct representation in Grisu, so they silently tested Dragon instead.

Rather than deleting them, could you move them to the dragon benchmarks if they aren't already covered there?

Might be worth a FIXME here or both files to just call the grisu::format* directly if that's what this is intended to bench.

They are already in dragon. No fixing needed.

Okay, makes sense. Please just make a note in the commit message for posterity that they are already tested in dragon.

* Fallback from Grisu to Dragon in flt2dec front * Together with name change to ensure all use checked * Drop of Grisu benchmarks which tested Dragon fallback * Documentation at functions instead of module level

pascaldekloe · 2026-03-29T12:39:43Z

Pushed the commit explainer plus distinct summary lines in Rust docs @tgross35. :-) I'll wait for the rest of the review. Some urgency would be nice because I have time to do this now. 🙏

The git messages ain't readable sections on their own indeed. You need the code in conjunction to comprehend the listings. I'll work on that.

tgross35

Next batch, mostly for the second commit

View changes since this review

tgross35 · 2026-03-29T23:09:23Z

    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_small_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(3.141592f64);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_big_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(f64::MAX);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_one_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(1.0);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_trailing_zero_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(250.000000000000000000000000);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
-    });
-}
-
-#[bench]
-fn bench_halfway_point_exact_inf(b: &mut Bencher) {
-    let decoded = decode_finite(1.00000000000000011102230246251565404236316680908203125);
-    let mut buf = [MaybeUninit::new(0); 1024];
-    b.iter(|| {
-        format_exact(black_box(&decoded), &mut buf, i16::MIN);
+        format_fixed(black_box(&decoded), &mut buf, i16::MIN).unwrap();


Rather than deleting them, could you move them to the dragon benchmarks if they aren't already covered there?

Might be worth a FIXME here or both files to just call the grisu::format* directly if that's what this is intended to bench.

tgross35 · 2026-03-29T23:20:51Z

Commit "rounding of leading digit tested in fmt": would it be possible to cover f16 and f32 as well?

There's a ton of work pending. I'm trying to make progress rather than picking up every possible thing on the way. 😅

This is a new test, it should at least cover f32. Dropping this commit to a later PR is also fine if that's easier.

"A little copying is better than a little dependency." — Rob Pike * Equivalence comparison explicit instead of generic * Direct test failure instead of counting with summary * Bit patterns specified as ops::Range constants * No panic on invalid UTF-8 in equivalence test

* Test functionality as provided instead of inners * No generics abstraction of Grisu and Dragon * The stringify! of input in assertion message * TestableFloat abstraction omitted with macro * MSVC omission central, direct on respective test Unified signature plus description on assertion tools: * check_shortest! -> check_short! * try_exact! -> check_match! * try_fixed! -> check_resolution! * check_exact & check_exact! -> check_fixed_mix! * check_exact_one -> check_coef_pow2!

Fix the number of infinite zeroes tested (to seven). It looks like the spaces were used as text alignment anyway.

tgross35 · 2026-04-27T07:28:54Z

Commit 2 "write out of flt2dec/random.rs": Please reword the commit summary line, I'm not sure what it's intending to say. Otherwise LGTM at 2f7b015.

View changes since the review

tgross35 · 2026-04-27T07:32:52Z

+#[cfg(not(target_env = "msvc"))]
+#[cfg_attr(miri, ignore)] // Miri is too slow


Commit 3: Use cfg_attr(..., ignore) for MSVC as well rather than configuring it out, to make it easier to opt in if somebody wants to check this locally.

View changes since the review

tgross35 · 2026-04-27T08:05:46Z

-# Problem statement
-
-We are given the floating-point number `v = f * 2^e` with an integer `f`,
-and its bounds `minus` and `plus` such that any number between `v - minus` and
-`v + plus` will be rounded to `v`. For the simplicity we assume that
-this range is exclusive. Then we would like to get the unique decimal
-representation `V = 0.d[0..n-1] * 10^k` such that:
-
- `d[0]` is non-zero.
-
- It's correctly rounded when parsed back: `v - minus < V < v + plus`.
-  Furthermore it is shortest such one, i.e., there is no representation
-  with less than `n` digits that is correctly rounded.
-
- It's closest to the original value: `abs(V - v) <= 10^(k-n) / 2`. Note that
-  there might be two representations satisfying this uniqueness requirement,
-  in which case some tie-breaking mechanism is used.
-
-We will call this mode of operation as to the *shortest* mode. This mode is used
-when there is no additional constraint, and can be thought as a "natural" mode
-as it matches the ordinary intuition (it at least prints `0.1f32` as "0.1").
-
-We have two more modes of operation closely related to each other. In these modes
-we are given either the number of significant digits `n` or the last-digit
-limitation `limit` (which determines the actual `n`), and we would like to get
-the representation `V = 0.d[0..n-1] * 10^k` such that:
-
- `d[0]` is non-zero, unless `n` was zero in which case only `k` is returned.
-
- It's closest to the original value: `abs(V - v) <= 10^(k-n) / 2`. Again,
-  there might be some tie-breaking mechanism.
-
-When `limit` is given but not `n`, we set `n` such that `k - n = limit`
-so that the last digit `d[n-1]` is scaled by `10^(k-n) = 10^limit`.
-If such `n` is negative, we clip it to zero so that we will only get `k`.
-We are also limited by the supplied buffer. This limitation is used to print
-the number up to given number of fractional digits without knowing
-the correct `k` beforehand.
-
-We will call the mode of operation requiring `n` as to the *exact* mode,
-and one requiring `limit` as to the *fixed* mode. The exact mode is a subset of
-the fixed mode: the sufficiently large last-digit limitation will eventually fill
-the supplied buffer and let the algorithm to return.
-
-# Implementation overview
-


Could you at least restore the removed portion of the module-level documentation, with changes to naming/notation as you see fit?

Organization isn't at odds with a module description, I don't want to lose the "readme" effect that can give somebody an understanding of the algorithms without needing to look at its API. There is also some helpful information here that we just don't gain anything by deleting.

tgross35 · 2026-04-27T08:40:21Z

+    ($coef:expr, $pow2:expr => $digits:expr, $pow10:expr) => (
+         check_match!($coef * $pow2.exp2() => $digits, $pow10);
+         check_resolution!($coef * $pow2.exp2(), $pow10 - $digits.len() as i16 => $digits, $pow10);
+    )


C3: Nit: store $coef * $pow2.exp2() to a local to save a (potentially slow) exp2 in debug mode

View changes since the review

tgross35 · 2026-04-27T08:47:47Z

Commit 3 "test flt2dec front of format_short & format_fixed": the message says:

No generics abstraction of Grisu and Dragon

How are the individual algorithms being tested now?

Other than that, the changes here look great to me aside from a few requests.

View changes since the review

tgross35 · 2026-04-27T09:06:44Z

+        let resolution: i16 = $resolution;
+        let want: (&[u8], i16) = ($digits, $pow10);
+
+        let mut large_buf = [MaybeUninit::new(b'_'); 1024];


C3: Could [MaybeUninit::new(b'_'); 1024] be pulled out to a constant? Since it's used in a few places.

View changes since the review

tgross35 · 2026-04-27T09:22:06Z

-    check_fixed_mix!(f32::MIN_POSITIVE => b"1175494350822287507968736537222245677818", -37);
+    check_fixed_mix!(f32::MIN_POSITIVE => b"1175494350822287507968736537222245677819", -37);


C4 "FIX faulty test values in flt2dec; last digit was not tested": These values weren't correct before right? Just add a note in the commit message that it's the values that are wrong, not our implementation, and how you validated if applicable.

Otherwise this commit LGTM.

View changes since the review

tgross35 · 2026-04-27T09:31:22Z

C6 "typo in test value from round-to-even in fmt": Add a note in the commit message that the values are equivalent when rounded to three decimals, then LGTM.

View changes since the review

tgross35 · 2026-04-27T09:36:33Z

-            start = 0;
        }
-        for i in start..-10 {
+        for i in (skip_first as i16)..-10 {


C8 "elaborate on digit loss due rounding in flt2dec": Nit: i16::from(skip_first) (avoiding potentially lossy as casts)

View changes since the review

tgross35 · 2026-04-27T09:38:44Z

-            if exp <= limit {
-                // the restriction couldn't been met, so this should render like zero no matter
-                // `exp` was. this does not include the case that the restriction has been met
-                // only after the final rounding-up; it's a regular case with `exp = limit + 1`.
-                debug_assert_eq!(buf.len(), 0);
+            if buf.len() == 0 {
+                // The number rounds down to zero at the given resolution.
+                debug_assert!(exp <= limit);


C8: Since these should be equivalent, what's the motivation to swap?

View changes since the review

tgross35 · 2026-04-27T09:53:56Z

C1: Please clarify "Together with name change to ensure all use checked" in the commit message

View changes since the review

tgross35 · 2026-04-27T09:54:49Z

Let's make sure there's no perf effect.

@bors try @rust-timer queue

@rustbot author

rustbot · 2026-04-27T09:54:54Z

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

flt2dec front of format_short & format_fixed

rust-bors · 2026-04-27T12:05:01Z

☀️ Try build successful (CI)
Build commit: cb7a7f2 (cb7a7f2a841fd88e9e55ffb325dfabdc4476a5b2, parent: 4933094f25ba1edf2de0a203665976832fc85bc9)

rust-timer · 2026-04-27T13:13:11Z

Finished benchmarking commit (cb7a7f2): comparison URL.

Overall result: no relevant changes - no action needed

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

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (secondary 3.9%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.9%	[1.8%, 6.0%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: 488.817s -> 487.773s (-0.21%)
Artifact size: 393.48 MiB -> 393.50 MiB (0.01%)

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 Mar 28, 2026

rustbot assigned scottmcm Mar 28, 2026

pascaldekloe mentioned this pull request Mar 28, 2026

fmt of floating points defragmented #150278

Open

tgross35 assigned tgross35 and unassigned scottmcm Mar 28, 2026

tgross35 reviewed Mar 28, 2026

View reviewed changes

flt2dec front with format_short and format_fixed

bf6de9e

* Fallback from Grisu to Dragon in flt2dec front * Together with name change to ensure all use checked * Drop of Grisu benchmarks which tested Dragon fallback * Documentation at functions instead of module level

pascaldekloe force-pushed the flt2dec-uni branch from 20351fd to aaf4362 Compare March 29, 2026 12:40

tgross35 reviewed Mar 29, 2026

View reviewed changes

pascaldekloe added 7 commits March 30, 2026 16:24

FIX faulty test values in flt2dec; last digit was not tested

d700a0d

equals suffix instead of spaces for test of infinite zeroes from flt2dec

750a2d6

Fix the number of infinite zeroes tested (to seven). It looks like the spaces were used as text alignment anyway.

typo in test value from round-to-even in fmt

4d7a977

rounding of leading digit tested in fmt

be361e5

elaborate on digit loss due rounding in flt2dec

894fa1e

pascaldekloe force-pushed the flt2dec-uni branch from aaf4362 to 894fa1e Compare March 30, 2026 14:51

tgross35 reviewed Apr 27, 2026

View reviewed changes

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 27, 2026

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 27, 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 Apr 27, 2026

rust-bors Bot pushed a commit that referenced this pull request Apr 27, 2026

Auto merge of #154505 - pascaldekloe:flt2dec-uni, r=<try>

cb7a7f2

flt2dec front of format_short & format_fixed

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 27, 2026

		#[cfg(not(target_env = "msvc"))]
		#[cfg_attr(miri, ignore)] // Miri is too slow

		check_fixed_mix!(f32::MIN_POSITIVE => b"1175494350822287507968736537222245677818", -37);
		check_fixed_mix!(f32::MIN_POSITIVE => b"1175494350822287507968736537222245677819", -37);

Uh oh!

Conversation

pascaldekloe commented Mar 28, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 28, 2026

Uh oh!

rustbot commented Mar 28, 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!

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!

Uh oh!

Uh oh!

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!

pascaldekloe commented Mar 29, 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!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

tgross35 Apr 27, 2026 • 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!

tgross35 Apr 27, 2026 • 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!

tgross35 Apr 27, 2026 • 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!

tgross35 Apr 27, 2026 • 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!

tgross35 Apr 27, 2026 • 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!

tgross35 Apr 27, 2026 • 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!

tgross35 Apr 27, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

pascaldekloe commented Mar 28, 2026 •

edited by rustbot

Loading

tgross35 left a comment •

edited by rustbot

Loading

tgross35 left a comment •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading

tgross35 Apr 27, 2026 •

edited by rustbot

Loading