-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Reduce formatting width
and precision
to 16 bits
#136932
base: master
Are you sure you want to change the base?
Conversation
@bors try |
…try> Reduce formatting `width` and `precision` to 16 bits This is reduces the `width` and `precision` fields in format strings to 16 bits. They are currently full `usize`s, but it's a bit nonsensical that we need to support the case where someone wants to pad their value to eighteen quintillion spaces and/or have eighteen quintillion digits of precision. This is a breaking change, but probably affects virtually no code. Let's do a crater run to find out. Marking this as experiment for now. --- There are three ways to set a width or precision today: 1. Directly a formatting string, e.g. `println!("{a:1234}")` 2. Indirectly in a formatting string, e.g. `println!("{a:width$}", width=1234)` 3. Through the unstable `FormattingOptions::width` method. This PR: - Adds a compiler error for 1. (`println!("{a:9999999}")` no longer compiles and gives a clear error.) - Adds a runtime check for 2. (`println!("{a:width$}, width=9999999)` will panic.) - Changes the signature of `FormattingOptions::width` to take a `u16` instead. --- Additional context: All the formatting flags and options are currently: - The `+` flag (1 bit) - The `-` flag (1 bit) - The `#` flag (1 bit) - The `0` flag (1 bit) - The `x?` flag (1 bit) - The `X?` flag (1 bit) - The alignment (2 bits) - The fill character (21 bits) - Whether a width is specified (1 bit) - Whether a precision is specified (1 bit) - If used, the width (a full usize) - If used, the precision (a full usize) Everything except the last two can simply fit in a `u32` (those add up to 31 bits in total). If we can accept a max width and precision of u16::MAX, we can make a `FormattingOptions` that is exactly 64 bits in size; the same size as a thin reference on most platforms. If, additionally, we also limit the number of formatting arguments to u16::MAX, we can also reduce the size of `fmt::Arguments` (that is, of a `format_args!()` expression).
tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-abort.diff
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
☀️ Try build successful - checks-actions |
@craterbot run mode=build-and-test |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
7355738
to
ccb9429
Compare
As a data point, I do use write!(self.w, "\n{:1$}]", "", self.cur_indent)?;
write!(f, "{:len$}", "", len = (stride - 1) * 4)?; |
The type will stay a |
Reduce FormattingOptions to 64 bits This reduces FormattingOptions from 6-7 machine words (384 bits on 64-bit platforms, 224 bits on 32-bit platforms) to just 64 bits (a single register on 64-bit platforms). This PR includes rust-lang#136932, which reduces the width and precision options to 16 bits, to make it all fit. Before: ```rust pub struct FormattingOptions { flags: u32, // only 6 bits used fill: char, align: Option<Alignment>, width: Option<usize>, precision: Option<usize>, } ``` After: ```rust pub struct FormattingOptions { /// Bits: /// - 0: `+` flag [rt::Flag::SignPlus] /// - 1: `-` flag [rt::Flag::SignMinus] /// - 2: `#` flag [rt::Flag::Alternate] /// - 3: `0` flag [rt::Flag::SignAwareZeroPad] /// - 4: `x?` flag [rt::Flag::DebugLowerHex] /// - 5: `X?` flag [rt::Flag::DebugUpperHex] /// - 6-7: Alignment (0: Left, 1: Right, 2: Center, 3: Unknown) /// - 8: Width flag (if set, the width field below is used) /// - 9: Precision flag (if set, the precision field below is used) /// - 10: unused /// - 11-31: fill character (21 bits, a full `char`) flags: u32, /// Width if width flag above is set. Otherwise, always 0. width: u16, /// Precision if precision flag above is set. Otherwise, always 0. precision: u16, } ```
🎉 Experiment
|
Crater results: Only two regressions are caused by an error or panic from this change.
|
This reduces the This is technically a breaking change, in two ways:
This is expected to have virtually no impact on real world code. See crater run results above: #136932 (comment) @rfcbot merge |
Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Fun fact: This also fixes a previously unknown bug that can occur when you run rustc on a 64-bit platform but target a 32-bit platform.
A width of 4294967306 was accepted because it fits in the compiler's |
@m-ou-se I updated the release notes based on that bug, which I think makes this change even more well-justified. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
ccb9429
to
b61458a
Compare
Has any thought been given to reducing the maximum width/precision to In terms of breakage, I'd expect it'd be mostly similar: 65534 or 65535 is about one and the same. On the other hand, from a "compaction" point of view, this allows using Even if not changing to |
Yes, I've considered that. I agree that it'd also work fine, but here are the reasons I didn't go for that option:
All of these arguments are quite weak. But unless there are clear benefits for |
While printing primitives doesn't need large width and precision, they can sometimes be used when dealing with arbitrary precision. Would this code to get 100,000 digits of pi break? let pi = rug::Float::with_val(332200, rug::float::Constant::Pi);
let pi_s = format!("{pi:.100000}"); Edit: for what it's worth, Rug does have an alternative method, so that code could be changed to |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
That code will give a compiler error:
|
This is part of #99012
This is reduces the
width
andprecision
fields in format strings to 16 bits. They are currently fullusize
s, but it's a bit nonsensical that we need to support the case where someone wants to pad their value to eighteen quintillion spaces and/or have eighteen quintillion digits of precision.By reducing these fields to 16 bit, we can reduce
FormattingOptions
to 64 bits (see #136974) and improve the in memory representation offormat_args!()
. (See additional context below.)This also fixes a bug where the width or precision is silently truncated when cross-compiling to a target with a smaller
usize
. By reducing the width and precision fields to the minimum guaranteed size ofusize
, 16 bits, this bug is eliminated.This is a breaking change, but affects almost no existing code.
Details of this change:
There are three ways to set a width or precision today:
println!("{a:1234}")
println!("{a:width$}", width=1234)
FormattingOptions::width
method.This PR:
println!("{a:9999999}")
no longer compiles and gives a clear error.)println!("{a:width$}, width=9999999)
will panic.)FormattingOptions::[get_]width
methods to use au16
instead.Additional context for improving
FormattingOptions
andfmt::Arguments
:All the formatting flags and options are currently:
+
flag (1 bit)-
flag (1 bit)#
flag (1 bit)0
flag (1 bit)x?
flag (1 bit)X?
flag (1 bit)Everything except the last two can simply fit in a
u32
(those add up to 31 bits in total).If we can accept a max width and precision of u16::MAX, we can make a
FormattingOptions
that is exactly 64 bits in size; the same size as a thin reference on most platforms.If, additionally, we also limit the number of formatting arguments, we can also reduce the size of
fmt::Arguments
(that is, of aformat_args!()
expression).