Implement `int_format_into` feature #142098

GuillaumeGomez · 2025-06-05T21:33:04Z

I took over #138338 with @madhav-madhusoodanan's approval.

Since #136264, a lot of changes happened so I made use of them to reduce the number of changes.

ACP approval: rust-lang/libs-team#546 (comment)

Associated Issue

Tracking Issue for integer formatting into a fixed-size buffer #138215

r? @hanna-kruppe

rustbot · 2025-06-05T21:33:07Z

Failed to set assignee to hanna-kruppe: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

GuillaumeGomez · 2025-06-05T21:33:47Z

library/alloctests/tests/num.rs

+        assert_eq!(ret, value);
+
+        let mut buffer = NumBuffer::<$int>::new();
+        assert_eq!(value.format_into(&mut buffer), s.as_str());


Since format_into is not part of a trait, I needed to switch from a function to a macro to avoid the generics limitations.

You could write and implement a trait right here, in the test - right?

I could indeed, but would be better than the current code?

Less churn; other than that I don't have strong feelings.

If someone else agrees with you I'll make the change (lazyness++ 😆).

library/core/src/fmt/num_buffer.rs

GuillaumeGomez · 2025-06-05T21:38:08Z

r? @Amanieu

library/core/src/fmt/num.rs

GuillaumeGomez · 2025-06-06T12:31:13Z

Applied suggestions and opened another PR for the number of digits of u128 computation to not add even more changes to this PR.

library/core/src/fmt/num.rs

bors · 2025-06-07T04:19:14Z

☔ The latest upstream changes (presumably #142133) made this pull request unmergeable. Please resolve the merge conflicts.

GuillaumeGomez · 2025-06-10T14:29:34Z

Fixed merge conflict.

GuillaumeGomez · 2025-06-17T22:07:41Z

Maybe you want to take a look at this one too @tgross35 ? :)

bors · 2025-06-20T23:48:39Z

☔ The latest upstream changes (presumably #142795) made this pull request unmergeable. Please resolve the merge conflicts.

…egers, r=tgross35 Use a distinct `ToString` implementation for `u128` and `i128` Part of rust-lang/rust#135543. Follow-up of rust-lang/rust#136264. When working on rust-lang/rust#142098, I realized that `i128` and `u128` could also benefit from a distinct `ToString` implementation so here it. The last commit is just me realizing that I forgot to add the format tests for `usize` and `isize`. Here is the bench comparison: | bench name | last nightly | with this PR | diff | |-|-|-|-| | bench_i128 | 29.25 ns/iter (+/- 0.66) | 17.52 ns/iter (+/- 0.7) | -40.1% | | bench_u128 | 34.06 ns/iter (+/- 0.21) | 16.1 ns/iter (+/- 0.6) | -52.7% | I used this code to test: ```rust #![feature(test)] extern crate test; use test::{Bencher, black_box}; #[inline(always)] fn convert_to_string<T: ToString>(n: T) -> String { n.to_string() } macro_rules! decl_benches { ($($name:ident: $ty:ident,)+) => { $( #[bench] fn $name(c: &mut Bencher) { c.iter(|| convert_to_string(black_box({ let nb: $ty = 20; nb }))); } )+ } } decl_benches! { bench_u128: u128, bench_i128: i128, } ```

library/core/src/fmt/num_buffer.rs

ChayimFriedman2 · 2025-06-22T01:12:43Z

library/core/src/fmt/num_buffer.rs

+    /// Returns the length of the buffer.
+    #[unstable(feature = "int_format_into", issue = "138215")]
+    pub const fn len(&self) -> usize {
+        self.buf.len()


This will always return 40, is this intended?

For now it's always 40 but I do hope in the future it will depend on the integer size when associated const support will be more advanced.

Maybe return T::BUF_SIZE here for forward-compatiblity.

That would be problematic since we always rely on buf.len(). It would create a gap, so we cannot do that.

…egers, r=tgross35 Use a distinct `ToString` implementation for `u128` and `i128` Part of rust-lang/rust#135543. Follow-up of rust-lang/rust#136264. When working on rust-lang/rust#142098, I realized that `i128` and `u128` could also benefit from a distinct `ToString` implementation so here it. The last commit is just me realizing that I forgot to add the format tests for `usize` and `isize`. Here is the bench comparison: | bench name | last nightly | with this PR | diff | |-|-|-|-| | bench_i128 | 29.25 ns/iter (+/- 0.66) | 17.52 ns/iter (+/- 0.7) | -40.1% | | bench_u128 | 34.06 ns/iter (+/- 0.21) | 16.1 ns/iter (+/- 0.6) | -52.7% | I used this code to test: ```rust #![feature(test)] extern crate test; use test::{Bencher, black_box}; #[inline(always)] fn convert_to_string<T: ToString>(n: T) -> String { n.to_string() } macro_rules! decl_benches { ($($name:ident: $ty:ident,)+) => { $( #[bench] fn $name(c: &mut Bencher) { c.iter(|| convert_to_string(black_box({ let nb: $ty = 20; nb }))); } )+ } } decl_benches! { bench_u128: u128, bench_i128: i128, } ```

library/core/src/fmt/num.rs

library/core/src/fmt/num_buffer.rs

GuillaumeGomez · 2025-06-30T15:43:02Z

Thanks for the review @Amanieu! Applied all suggestions.

Also, gonna send a separate PR to use get_unchecked in this file because we could make things much better.

GuillaumeGomez · 2025-06-30T18:15:31Z

Safety annotation needs to be right on the unsafe block, not on the start of the expression. Well, noted.

library/core/src/fmt/num.rs

GuillaumeGomez · 2025-07-01T12:00:46Z

@Amanieu I went to the end of the usage of get_unchecked everywhere in #143277 and basically, there is absolutely no difference, the compiler is already optimizing all the code correctly.

hanna-kruppe · 2025-07-01T12:14:10Z

Note that there were some recent PRs removing unsafe code from int Display impls that didn’t make a difference (#135265, #136594).

GuillaumeGomez · 2025-07-01T12:16:02Z

Yeah I will just revert the get_unchecked calls I added here as well. No point in keeping them.

GuillaumeGomez · 2025-07-01T12:25:30Z

And done.

Amanieu · 2025-07-02T06:13:33Z

library/core/src/fmt/num_buffer.rs

+    /// Initializes internal buffer.
+    #[unstable(feature = "int_format_into", issue = "138215")]
+    pub const fn new() -> Self {
+        // FIXME: Once const generics feature is working, use `T::BUF_SIZE` instead of 40.


Can you add a const { assert!(T::BUF_SIZE <= 40) } here.

Amanieu · 2025-07-02T06:14:17Z

library/core/src/fmt/num_buffer.rs

+    /// Returns the length of the buffer.
+    #[unstable(feature = "int_format_into", issue = "138215")]
+    pub const fn len(&self) -> usize {
+        self.buf.len()


Maybe return T::BUF_SIZE here for forward-compatiblity.

Amanieu · 2025-07-02T06:14:58Z

library/core/src/fmt/num.rs

@@ -248,6 +253,13 @@ macro_rules! impl_Display {
                issue = "none"
            )]
            pub fn _fmt<'a>(self, buf: &'a mut [MaybeUninit::<u8>]) -> &'a str {


This needs to be an unsafe function because it makes assumptions on the size of buf.

Amanieu · 2025-07-02T06:17:17Z

library/core/src/fmt/num.rs

                    n /= 10;

                    if n == 0 {
                        break;
                    }
                }
            }
+            cur


Should this be curr? How does this even compile?

Oh, that's because of optimize_for_size. I'm surprised it's not tested in CI.

Amanieu · 2025-07-02T06:17:42Z

library/core/src/fmt/num.rs

@@ -336,20 +421,25 @@ macro_rules! impl_Display {
            unsafe {


Unsafe is not needed here since get_unchecked is not used.

Amanieu · 2025-07-02T06:18:51Z

library/core/src/fmt/num.rs

@@ -598,12 +688,20 @@ impl u128 {
        issue = "none"
    )]
    pub fn _fmt<'a>(self, buf: &'a mut [MaybeUninit<u8>]) -> &'a str {


Same here, this needs to be unsafe because it assumes the slice size. Or _fmt_inner should be made safe.

…n-128-integers, r=tgross35 Use a distinct `ToString` implementation for `u128` and `i128` Part of rust-lang#135543. Follow-up of rust-lang#136264. When working on rust-lang#142098, I realized that `i128` and `u128` could also benefit from a distinct `ToString` implementation so here it. The last commit is just me realizing that I forgot to add the format tests for `usize` and `isize`. Here is the bench comparison: | bench name | last nightly | with this PR | diff | |-|-|-|-| | bench_i128 | 29.25 ns/iter (+/- 0.66) | 17.52 ns/iter (+/- 0.7) | -40.1% | | bench_u128 | 34.06 ns/iter (+/- 0.21) | 16.1 ns/iter (+/- 0.6) | -52.7% | I used this code to test: ```rust #![feature(test)] extern crate test; use test::{Bencher, black_box}; #[inline(always)] fn convert_to_string<T: ToString>(n: T) -> String { n.to_string() } macro_rules! decl_benches { ($($name:ident: $ty:ident,)+) => { $( #[bench] fn $name(c: &mut Bencher) { c.iter(|| convert_to_string(black_box({ let nb: $ty = 20; nb }))); } )+ } } decl_benches! { bench_u128: u128, bench_i128: i128, } ```

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 Jun 5, 2025

GuillaumeGomez commented Jun 5, 2025

View reviewed changes

library/core/src/fmt/num_buffer.rs Outdated Show resolved Hide resolved

GuillaumeGomez mentioned this pull request Jun 5, 2025

Functionality for int_format_into for integer types #138338

Closed

rustbot assigned Amanieu Jun 5, 2025

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the int_format_into branch from 1d9a06e to 2589402 Compare June 5, 2025 21:45

tamird reviewed Jun 5, 2025

View reviewed changes

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the int_format_into branch 2 times, most recently from d11cb44 to 70ab4e0 Compare June 6, 2025 09:44

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the int_format_into branch from 70ab4e0 to 3eecb24 Compare June 6, 2025 10:00

GuillaumeGomez changed the title ~~Int format into~~ Jun 6, 2025

tamird reviewed Jun 6, 2025

View reviewed changes

library/core/src/fmt/num.rs Show resolved Hide resolved

tamird reviewed Jun 6, 2025

View reviewed changes

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

tamird reviewed Jun 6, 2025

View reviewed changes

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the int_format_into branch from 3eecb24 to 2f869c5 Compare June 6, 2025 12:27

tamird reviewed Jun 6, 2025

View reviewed changes

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the int_format_into branch from 2f869c5 to 9324432 Compare June 6, 2025 12:47

GuillaumeGomez force-pushed the int_format_into branch from 9324432 to 4733100 Compare June 10, 2025 09:16

GuillaumeGomez mentioned this pull request Jun 10, 2025

Use a distinct ToString implementation for u128 and i128 #142294

Merged

GuillaumeGomez force-pushed the int_format_into branch from 4733100 to 61cc5f3 Compare June 17, 2025 21:49

GuillaumeGomez force-pushed the int_format_into branch from 61cc5f3 to e5c6e02 Compare June 19, 2025 11:11

yotamofek reviewed Jun 21, 2025

View reviewed changes

library/core/src/fmt/num_buffer.rs Outdated Show resolved Hide resolved

ChayimFriedman2 reviewed Jun 22, 2025

View reviewed changes

GuillaumeGomez force-pushed the int_format_into branch from 1b59ede to e8eefc7 Compare June 23, 2025 12:00

rust-lang deleted a comment from rustbot Jun 28, 2025

Amanieu reviewed Jun 29, 2025

View reviewed changes

GuillaumeGomez force-pushed the int_format_into branch from e8eefc7 to 0c98949 Compare June 30, 2025 15:40

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the int_format_into branch from 0c98949 to 1ce8780 Compare June 30, 2025 18:15

This comment has been minimized.

Sign in to view

tamird reviewed Jun 30, 2025

View reviewed changes

library/core/src/fmt/num.rs Outdated Show resolved Hide resolved

GuillaumeGomez force-pushed the int_format_into branch 3 times, most recently from a8b169e to 9e76d15 Compare July 1, 2025 09:44

GuillaumeGomez force-pushed the int_format_into branch from 9e76d15 to fb17077 Compare July 1, 2025 12:25

Amanieu reviewed Jul 2, 2025

View reviewed changes

GuillaumeGomez added 3 commits July 3, 2025 18:57

Implement int_format_into feature

82c6037

Add test for int_format_into feature

7e23144

Use slice_buffer_to_str in GenericRadix::fmt_int

9943c19

GuillaumeGomez force-pushed the int_format_into branch from fb17077 to 9943c19 Compare July 3, 2025 16:57

Implement int_format_into feature #142098

Are you sure you want to change the base?

Implement int_format_into feature #142098

Conversation

GuillaumeGomez commented Jun 5, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Associated Issue

rustbot commented Jun 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Jun 5, 2025

This comment has been minimized.

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GuillaumeGomez commented Jun 6, 2025

Uh oh!

Uh oh!

Uh oh!

bors commented Jun 7, 2025

GuillaumeGomez commented Jun 10, 2025

GuillaumeGomez commented Jun 17, 2025

bors commented Jun 20, 2025

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GuillaumeGomez commented Jun 30, 2025

This comment has been minimized.

GuillaumeGomez commented Jun 30, 2025

This comment has been minimized.

Uh oh!

GuillaumeGomez commented Jul 1, 2025

hanna-kruppe commented Jul 1, 2025

GuillaumeGomez commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GuillaumeGomez commented Jul 1, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Implement `int_format_into` feature #142098

Implement `int_format_into` feature #142098

GuillaumeGomez commented Jun 5, 2025 •

edited by rustbot

Loading

GuillaumeGomez commented Jul 1, 2025 •

edited

Loading