From 8c96149ddf596457ab880277f091f6b3374942fa Mon Sep 17 00:00:00 2001 From: Elias Holzmann <9659253+EliasHolzmann@users.noreply.github.com> Date: Wed, 22 Nov 2023 01:14:41 +0100 Subject: [PATCH 1/6] Formatter: Access members via getter methods wherever possible The idea behind this is to make implementing `fmt::FormattingOptions` (as well as any future changes to `std::Formatter`) easier. In theory, this might have a negative performance impact because of the additional function calls. However, I strongly believe that those will be inlined anyway, thereby producing assembly code that has comparable performance. --- library/core/src/fmt/float.rs | 6 +++--- library/core/src/fmt/mod.rs | 26 +++++++++++++------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/library/core/src/fmt/float.rs b/library/core/src/fmt/float.rs index 3bbf5d8770bd2..6915edc813045 100644 --- a/library/core/src/fmt/float.rs +++ b/library/core/src/fmt/float.rs @@ -86,7 +86,7 @@ where true => flt2dec::Sign::MinusPlus, }; - if let Some(precision) = fmt.precision { + if let Some(precision) = fmt.precision() { float_to_decimal_common_exact(fmt, num, sign, precision) } else { let min_precision = 0; @@ -161,7 +161,7 @@ where true => flt2dec::Sign::MinusPlus, }; - if let Some(precision) = fmt.precision { + if let Some(precision) = fmt.precision() { // 1 integral digit + `precision` fractional digits = `precision + 1` total digits float_to_exponential_common_exact(fmt, num, sign, precision + 1, upper) } else { @@ -179,7 +179,7 @@ where true => flt2dec::Sign::MinusPlus, }; - if let Some(precision) = fmt.precision { + if let Some(precision) = fmt.precision() { // this behavior of {:.PREC?} predates exponential formatting for {:?} float_to_decimal_common_exact(fmt, num, sign, precision) } else { diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index e1b7b46a1ed2f..74265e488eb78 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -1298,7 +1298,7 @@ impl<'a> Formatter<'a> { } // The `width` field is more of a `min-width` parameter at this point. - match self.width { + match self.width() { // If there's no minimum length requirements then we can just // write the bytes. None => { @@ -1365,12 +1365,12 @@ impl<'a> Formatter<'a> { #[stable(feature = "rust1", since = "1.0.0")] pub fn pad(&mut self, s: &str) -> Result { // Make sure there's a fast path up front - if self.width.is_none() && self.precision.is_none() { + if self.width().is_none() && self.precision().is_none() { return self.buf.write_str(s); } // The `precision` field can be interpreted as a `max-width` for the // string being formatted. - let s = if let Some(max) = self.precision { + let s = if let Some(max) = self.precision() { // If our string is longer that the precision, then we must have // truncation. However other flags like `fill`, `width` and `align` // must act as always. @@ -1387,7 +1387,7 @@ impl<'a> Formatter<'a> { &s }; // The `width` field is more of a `min-width` parameter at this point. - match self.width { + match self.width() { // If we're under the maximum length, and there's no minimum length // requirements, then we can just emit the string None => self.buf.write_str(s), @@ -1432,10 +1432,10 @@ impl<'a> Formatter<'a> { }; for _ in 0..pre_pad { - self.buf.write_char(self.fill)?; + self.buf.write_char(self.fill())?; } - Ok(PostPadding::new(self.fill, post_pad)) + Ok(PostPadding::new(self.fill(), post_pad)) } /// Takes the formatted parts and applies the padding. @@ -1446,12 +1446,12 @@ impl<'a> Formatter<'a> { /// /// Any `numfmt::Part::Copy` parts in `formatted` must contain valid UTF-8. unsafe fn pad_formatted_parts(&mut self, formatted: &numfmt::Formatted<'_>) -> Result { - if let Some(mut width) = self.width { + if let Some(mut width) = self.width() { // for the sign-aware zero padding, we render the sign first and // behave as if we had no sign from the beginning. let mut formatted = formatted.clone(); - let old_fill = self.fill; - let old_align = self.align; + let old_fill = self.fill(); + let old_align = self.align(); if self.sign_aware_zero_pad() { // a sign always goes first let sign = formatted.sign; @@ -2384,7 +2384,7 @@ impl Debug for char { #[stable(feature = "rust1", since = "1.0.0")] impl Display for char { fn fmt(&self, f: &mut Formatter<'_>) -> Result { - if f.width.is_none() && f.precision.is_none() { + if f.width().is_none() && f.precision().is_none() { f.write_char(*self) } else { f.pad(self.encode_utf8(&mut [0; 4])) @@ -2408,8 +2408,8 @@ impl Pointer for *const T { /// /// [problematic]: https://github.com/rust-lang/rust/issues/95489 pub(crate) fn pointer_fmt_inner(ptr_addr: usize, f: &mut Formatter<'_>) -> Result { - let old_width = f.width; - let old_flags = f.flags; + let old_width = f.width(); + let old_flags = f.flags(); // The alternate flag is already treated by LowerHex as being special- // it denotes whether to prefix with 0x. We use it to work out whether @@ -2418,7 +2418,7 @@ pub(crate) fn pointer_fmt_inner(ptr_addr: usize, f: &mut Formatter<'_>) -> Resul if f.alternate() { f.flags |= 1 << (rt::Flag::SignAwareZeroPad as u32); - if f.width.is_none() { + if f.width().is_none() { f.width = Some((usize::BITS / 4) as usize + 2); } } From 973ce00ea2fdd1d8f61da5b00149504d47734cc1 Mon Sep 17 00:00:00 2001 From: Elias Holzmann <9659253+EliasHolzmann@users.noreply.github.com> Date: Wed, 22 Nov 2023 01:52:13 +0100 Subject: [PATCH 2/6] Added struct `fmt::FormattingOptions` This allows to build custom `std::Formatter`s at runtime. Also added some related enums and two related methods on `std::Formatter`. --- library/alloc/src/fmt.rs | 2 + library/alloc/src/lib.rs | 1 + library/alloc/src/string.rs | 3 +- library/core/src/fmt/mod.rs | 381 ++++++++++++++++++++++++++++------ library/core/tests/fmt/mod.rs | 36 +++- library/core/tests/lib.rs | 1 + 6 files changed, 351 insertions(+), 73 deletions(-) diff --git a/library/alloc/src/fmt.rs b/library/alloc/src/fmt.rs index 5b50ef7bf6c21..66b04ebe55c89 100644 --- a/library/alloc/src/fmt.rs +++ b/library/alloc/src/fmt.rs @@ -563,6 +563,8 @@ pub use core::fmt::{write, Arguments}; pub use core::fmt::{Binary, Octal}; #[stable(feature = "rust1", since = "1.0.0")] pub use core::fmt::{Debug, Display}; +#[unstable(feature = "formatting_options", issue = "118117")] +pub use core::fmt::{DebugAsHex, FormattingOptions, Sign}; #[stable(feature = "rust1", since = "1.0.0")] pub use core::fmt::{DebugList, DebugMap, DebugSet, DebugStruct, DebugTuple}; #[stable(feature = "rust1", since = "1.0.0")] diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 0af3ac38ee534..9c03c3b59dd0c 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -129,6 +129,7 @@ #![feature(extend_one)] #![feature(fmt_internals)] #![feature(fn_traits)] +#![feature(formatting_options)] #![feature(hasher_prefixfree_extras)] #![feature(inline_const)] #![feature(inplace_iteration)] diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index 4d6968157dedd..884fbf2c7cc3b 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -44,6 +44,7 @@ use core::error::Error; use core::fmt; +use core::fmt::FormattingOptions; use core::hash; #[cfg(not(no_global_oom_handling))] use core::iter::from_fn; @@ -2605,7 +2606,7 @@ impl ToString for T { #[inline] default fn to_string(&self) -> String { let mut buf = String::new(); - let mut formatter = core::fmt::Formatter::new(&mut buf); + let mut formatter = core::fmt::Formatter::new(&mut buf, FormattingOptions::new()); // Bypass format_args!() to avoid write_str with zero-length strs fmt::Display::fmt(self, &mut formatter) .expect("a Display implementation returned an error unexpectedly"); diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 74265e488eb78..5dc37d1e5227c 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -36,6 +36,19 @@ pub enum Alignment { Center, } +#[doc(hidden)] +#[unstable(feature = "fmt_internals", reason = "internal to standard library", issue = "none")] +impl From for Option { + fn from(value: rt::Alignment) -> Self { + match value { + rt::Alignment::Left => Some(Alignment::Left), + rt::Alignment::Right => Some(Alignment::Right), + rt::Alignment::Center => Some(Alignment::Center), + rt::Alignment::Unknown => None, + } + } +} + #[stable(feature = "debug_builders", since = "1.2.0")] pub use self::builders::{DebugList, DebugMap, DebugSet, DebugStruct, DebugTuple}; @@ -231,6 +244,243 @@ impl Write for &mut W { } } +/// The signedness of a [`Formatter`] (or of a [`FormattingOptions`]). +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[unstable(feature = "formatting_options", issue = "118117")] +pub enum Sign { + /// Represents the `+` flag. + Plus, + /// Represents the `-` flag. + Minus, +} + +/// Specifies whether the [`Debug`] trait should use lower-/upper-case +/// hexadecimal or normal integers. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[unstable(feature = "formatting_options", issue = "118117")] +pub enum DebugAsHex { + /// Use lower-case hexadecimal integers for the `Debug` trait (like [the `x?` type](../../std/fmt/index.html#formatting-traits)). + Lower, + /// Use upper-case hexadecimal integers for the `Debug` trait (like [the `x?` type](../../std/fmt/index.html#formatting-traits)). + Upper, +} + +/// Options for formatting. +/// +/// `FormattingOptions` is a [`Formatter`] without an attached [`Write`] trait. +/// It is mainly used to construct `Formatter` instances. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[unstable(feature = "formatting_options", issue = "118117")] +pub struct FormattingOptions { + sign: Option, + sign_aware_zero_pad: bool, + alternate: bool, + fill: char, + alignment: Option, + width: Option, + precision: Option, + debug_as_hex: Option, +} + +impl FormattingOptions { + /// Construct a new `FormatterBuilder` with the supplied `Write` trait + /// object for output that is equivalent to the `{}` formatting + /// specifier: + /// + /// - no flags, + /// - filled with spaces, + /// - no alignment, + /// - no width, + /// - no precision, and + /// - no [`DebugAsHex`] output mode. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn new() -> Self { + Self { + sign: None, + sign_aware_zero_pad: false, + alternate: false, + fill: ' ', + alignment: None, + width: None, + precision: None, + debug_as_hex: None, + } + } + + /// Sets or removes the sign (the `+` or the `-` flag). + /// + /// - `+`: This is intended for numeric types and indicates that the sign + /// should always be printed. By default only the negative sign of signed + /// values is printed, and the sign of positive or unsigned values is + /// omitted. This flag indicates that the correct sign (+ or -) should + /// always be printed. + /// - `-`: Currently not used + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn sign(&mut self, sign: Option) -> &mut Self { + self.sign = sign; + self + } + /// Sets or unsets the `0` flag. + /// + /// This is used to indicate for integer formats that the padding to width should both be done with a 0 character as well as be sign-aware + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn sign_aware_zero_pad(&mut self, sign_aware_zero_pad: bool) -> &mut Self { + self.sign_aware_zero_pad = sign_aware_zero_pad; + self + } + /// Sets or unsets the `#` flag. + /// + /// This flag indicates that the "alternate" form of printing should be + /// used. The alternate forms are: + /// - [`Debug`] : pretty-print the [`Debug`] formatting (adds linebreaks and indentation) + /// - [`LowerHex`] as well as [`UpperHex`] - precedes the argument with a `0x` + /// - [`Octal`] - precedes the argument with a `0b` + /// - [`Binary`] - precedes the argument with a `0o` + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn alternate(&mut self, alternate: bool) -> &mut Self { + self.alternate = alternate; + self + } + /// Sets the fill character. + /// + /// The optional fill character and alignment is provided normally in + /// conjunction with the width parameter. This indicates that if the value + /// being formatted is smaller than width some extra characters will be + /// printed around it. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn fill(&mut self, fill: char) -> &mut Self { + self.fill = fill; + self + } + /// Sets or removes the alignment. + /// + /// The alignment specifies how the value being formatted should be + /// positioned if it is smaller than the width of the formatter. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn alignment(&mut self, alignment: Option) -> &mut Self { + self.alignment = alignment; + self + } + /// Sets or removes the width. + /// + /// This is a parameter for the “minimum width” that the format should take + /// up. If the value’s string does not fill up this many characters, then + /// the padding specified by [`FormattingOptions::fill`]/[`FormattingOptions::alignment`] + /// will be used to take up the required space. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn width(&mut self, width: Option) -> &mut Self { + self.width = width; + self + } + /// Sets or removes the precision. + /// + /// - For non-numeric types, this can be considered a “maximum width”. If + /// the resulting string is longer than this width, then it is truncated + /// down to this many characters and that truncated value is emitted with + /// proper fill, alignment and width if those parameters are set. + /// - For integral types, this is ignored. + /// - For floating-point types, this indicates how many digits after the + /// decimal point should be printed. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn precision(&mut self, precision: Option) -> &mut Self { + self.precision = precision; + self + } + /// Specifies whether the [`Debug`] trait should use lower-/upper-case + /// hexadecimal or normal integers + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn debug_as_hex(&mut self, debug_as_hex: Option) -> &mut Self { + self.debug_as_hex = debug_as_hex; + self + } + + /// Returns the current sign (the `+` or the `-` flag). + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn get_sign(&self) -> Option { + self.sign + } + /// Returns the current `0` flag. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn get_sign_aware_zero_pad(&self) -> bool { + self.sign_aware_zero_pad + } + /// Returns the current `#` flag. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn get_alternate(&self) -> bool { + self.alternate + } + /// Returns the current fill character. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn get_fill(&self) -> char { + self.fill + } + /// Returns the current alignment. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn get_alignment(&self) -> Option { + self.alignment + } + /// Returns the current width. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn get_width(&self) -> Option { + self.width + } + /// Returns the current precision. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn get_precision(&self) -> Option { + self.precision + } + /// Returns the current precision. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn get_debug_as_hex(&self) -> Option { + self.debug_as_hex + } + + /// Creates a [`Formatter`] that writes its output to the given [`Write`] trait. + /// + /// You may alternatively use [`Formatter::new()`]. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn create_formatter<'a>(self, write: &'a mut (dyn Write + 'a)) -> Formatter<'a> { + Formatter { options: self, buf: write } + } + + #[doc(hidden)] + #[unstable(feature = "fmt_internals", reason = "internal to standard library", issue = "none")] + /// Flags for formatting + pub fn flags(&mut self, flags: u32) { + self.sign = if flags & (1 << rt::Flag::SignPlus as u32) != 0 { + Some(Sign::Plus) + } else if flags & (1 << rt::Flag::SignMinus as u32) != 0 { + Some(Sign::Minus) + } else { + None + }; + self.alternate = (flags & (1 << rt::Flag::Alternate as u32)) != 0; + self.sign_aware_zero_pad = (flags & (1 << rt::Flag::SignAwareZeroPad as u32)) != 0; + self.debug_as_hex = if flags & (1 << rt::Flag::DebugLowerHex as u32) != 0 { + Some(DebugAsHex::Lower) + } else if flags & (1 << rt::Flag::DebugUpperHex as u32) != 0 { + Some(DebugAsHex::Upper) + } else { + None + }; + } + #[doc(hidden)] + #[unstable(feature = "fmt_internals", reason = "internal to standard library", issue = "none")] + /// Flags for formatting + pub fn get_flags(&self) -> u32 { + >::into(self.get_sign() == Some(Sign::Plus)) << rt::Flag::SignPlus as u32 + | >::into(self.get_sign() == Some(Sign::Minus)) + << rt::Flag::SignMinus as u32 + | >::into(self.get_alternate()) << rt::Flag::Alternate as u32 + | >::into(self.get_sign_aware_zero_pad()) + << rt::Flag::SignAwareZeroPad as u32 + | >::into(self.debug_as_hex == Some(DebugAsHex::Lower)) + << rt::Flag::DebugLowerHex as u32 + | >::into(self.debug_as_hex == Some(DebugAsHex::Upper)) + << rt::Flag::DebugUpperHex as u32 + } +} + /// Configuration for formatting. /// /// A `Formatter` represents various options related to formatting. Users do not @@ -244,34 +494,28 @@ impl Write for &mut W { #[stable(feature = "rust1", since = "1.0.0")] #[rustc_diagnostic_item = "Formatter"] pub struct Formatter<'a> { - flags: u32, - fill: char, - align: rt::Alignment, - width: Option, - precision: Option, + options: FormattingOptions, buf: &'a mut (dyn Write + 'a), } impl<'a> Formatter<'a> { - /// Creates a new formatter with default settings. + /// Creates a new formatter with given [`FormattingOptions`]. /// - /// This can be used as a micro-optimization in cases where a full `Arguments` - /// structure (as created by `format_args!`) is not necessary; `Arguments` - /// is a little more expensive to use in simple formatting scenarios. + /// If `write` is a reference to a formatter, it is recommended to use + /// [`Formatter::with_options`] instead as this can borrow the underlying + /// `write`, thereby bypassing one layer of indirection. /// - /// Currently not intended for use outside of the standard library. - #[unstable(feature = "fmt_internals", reason = "internal to standard library", issue = "none")] - #[doc(hidden)] - pub fn new(buf: &'a mut (dyn Write + 'a)) -> Formatter<'a> { - Formatter { - flags: 0, - fill: ' ', - align: rt::Alignment::Unknown, - width: None, - precision: None, - buf, - } + /// You may alternatively use [`FormattingOptions::create_formatter()`]. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn new(write: &'a mut (dyn Write + 'a), options: FormattingOptions) -> Self { + Formatter { options, buf: write } + } + + /// Creates a new formatter based on this one with given [`FormattingOptions`]. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn with_options(&'a mut self, options: FormattingOptions) -> Self { + Formatter { options, buf: self.buf } } } @@ -1104,7 +1348,7 @@ pub trait UpperExp { /// [`write!`]: crate::write! #[stable(feature = "rust1", since = "1.0.0")] pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { - let mut formatter = Formatter::new(output); + let mut formatter = Formatter::new(output, FormattingOptions::new()); let mut idx = 0; match args.fmt { @@ -1148,14 +1392,14 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { } unsafe fn run(fmt: &mut Formatter<'_>, arg: &rt::Placeholder, args: &[rt::Argument<'_>]) -> Result { - fmt.fill = arg.fill; - fmt.align = arg.align; - fmt.flags = arg.flags; + fmt.options.fill(arg.fill); + fmt.options.alignment(arg.align.into()); + fmt.options.flags(arg.flags); // SAFETY: arg and args come from the same Arguments, // which guarantees the indexes are always within bounds. unsafe { - fmt.width = getcount(args, &arg.width); - fmt.precision = getcount(args, &arg.precision); + fmt.options.width(getcount(args, &arg.width)); + fmt.options.precision(getcount(args, &arg.precision)); } // Extract the correct argument @@ -1213,11 +1457,7 @@ impl<'a> Formatter<'a> { buf: wrap(self.buf), // And preserve these - flags: self.flags, - fill: self.fill, - align: self.align, - width: self.width, - precision: self.precision, + options: self.options, } } @@ -1314,14 +1554,15 @@ impl<'a> Formatter<'a> { // The sign and prefix goes before the padding if the fill character // is zero Some(min) if self.sign_aware_zero_pad() => { - let old_fill = crate::mem::replace(&mut self.fill, '0'); - let old_align = crate::mem::replace(&mut self.align, rt::Alignment::Right); + let old_fill = crate::mem::replace(&mut self.options.fill, '0'); + let old_align = + crate::mem::replace(&mut self.options.alignment, Some(Alignment::Right)); write_prefix(self, sign, prefix)?; let post_padding = self.padding(min - width, Alignment::Right)?; self.buf.write_str(buf)?; post_padding.write(self)?; - self.fill = old_fill; - self.align = old_align; + self.options.fill = old_fill; + self.options.alignment = old_align; Ok(()) } // Otherwise, the sign and prefix goes after the padding @@ -1418,12 +1659,7 @@ impl<'a> Formatter<'a> { padding: usize, default: Alignment, ) -> result::Result { - let align = match self.align { - rt::Alignment::Unknown => default, - rt::Alignment::Left => Alignment::Left, - rt::Alignment::Right => Alignment::Right, - rt::Alignment::Center => Alignment::Center, - }; + let align = self.align().unwrap_or(default); let (pre_pad, post_pad) = match align { Alignment::Left => (0, padding), @@ -1460,8 +1696,8 @@ impl<'a> Formatter<'a> { // remove the sign from the formatted parts formatted.sign = ""; width = width.saturating_sub(sign.len()); - self.fill = '0'; - self.align = rt::Alignment::Right; + self.options.fill('0'); + self.options.alignment(Some(Alignment::Right)); } // remaining parts go through the ordinary padding process. @@ -1478,8 +1714,8 @@ impl<'a> Formatter<'a> { } post_padding.write(self) }; - self.fill = old_fill; - self.align = old_align; + self.options.fill(old_fill); + self.options.alignment(old_align); ret } else { // this is the common case and we take a shortcut @@ -1595,7 +1831,7 @@ impl<'a> Formatter<'a> { or `sign_aware_zero_pad` methods instead" )] pub fn flags(&self) -> u32 { - self.flags + self.options.get_flags() } /// Character used as 'fill' whenever there is alignment. @@ -1628,7 +1864,7 @@ impl<'a> Formatter<'a> { #[must_use] #[stable(feature = "fmt_flags", since = "1.5.0")] pub fn fill(&self) -> char { - self.fill + self.options.get_fill() } /// Flag indicating what form of alignment was requested. @@ -1663,12 +1899,7 @@ impl<'a> Formatter<'a> { #[must_use] #[stable(feature = "fmt_flags_align", since = "1.28.0")] pub fn align(&self) -> Option { - match self.align { - rt::Alignment::Left => Some(Alignment::Left), - rt::Alignment::Right => Some(Alignment::Right), - rt::Alignment::Center => Some(Alignment::Center), - rt::Alignment::Unknown => None, - } + self.options.get_alignment() } /// Optionally specified integer width that the output should be. @@ -1698,7 +1929,7 @@ impl<'a> Formatter<'a> { #[must_use] #[stable(feature = "fmt_flags", since = "1.5.0")] pub fn width(&self) -> Option { - self.width + self.options.get_width() } /// Optionally specified precision for numeric types. Alternatively, the @@ -1729,7 +1960,7 @@ impl<'a> Formatter<'a> { #[must_use] #[stable(feature = "fmt_flags", since = "1.5.0")] pub fn precision(&self) -> Option { - self.precision + self.options.get_precision() } /// Determines if the `+` flag was specified. @@ -1761,7 +1992,7 @@ impl<'a> Formatter<'a> { #[must_use] #[stable(feature = "fmt_flags", since = "1.5.0")] pub fn sign_plus(&self) -> bool { - self.flags & (1 << rt::Flag::SignPlus as u32) != 0 + self.options.get_sign() == Some(Sign::Plus) } /// Determines if the `-` flag was specified. @@ -1790,7 +2021,7 @@ impl<'a> Formatter<'a> { #[must_use] #[stable(feature = "fmt_flags", since = "1.5.0")] pub fn sign_minus(&self) -> bool { - self.flags & (1 << rt::Flag::SignMinus as u32) != 0 + self.options.get_sign() == Some(Sign::Minus) } /// Determines if the `#` flag was specified. @@ -1818,7 +2049,7 @@ impl<'a> Formatter<'a> { #[must_use] #[stable(feature = "fmt_flags", since = "1.5.0")] pub fn alternate(&self) -> bool { - self.flags & (1 << rt::Flag::Alternate as u32) != 0 + self.options.get_alternate() } /// Determines if the `0` flag was specified. @@ -1844,17 +2075,17 @@ impl<'a> Formatter<'a> { #[must_use] #[stable(feature = "fmt_flags", since = "1.5.0")] pub fn sign_aware_zero_pad(&self) -> bool { - self.flags & (1 << rt::Flag::SignAwareZeroPad as u32) != 0 + self.options.get_sign_aware_zero_pad() } // FIXME: Decide what public API we want for these two flags. // https://github.com/rust-lang/rust/issues/48584 fn debug_lower_hex(&self) -> bool { - self.flags & (1 << rt::Flag::DebugLowerHex as u32) != 0 + self.options.debug_as_hex == Some(DebugAsHex::Lower) } fn debug_upper_hex(&self) -> bool { - self.flags & (1 << rt::Flag::DebugUpperHex as u32) != 0 + self.options.debug_as_hex == Some(DebugAsHex::Upper) } /// Creates a [`DebugStruct`] builder designed to assist with creation of @@ -2260,6 +2491,18 @@ impl<'a> Formatter<'a> { pub fn debug_map<'b>(&'b mut self) -> DebugMap<'b, 'a> { builders::debug_map_new(self) } + + /// Returns the sign of this formatter (`+` or `-`). + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn sign(&self) -> Option { + self.options.get_sign() + } + + /// Returns the formatting options this formatter corresponds to. + #[unstable(feature = "formatting_options", issue = "118117")] + pub fn options(&self) -> FormattingOptions { + self.options + } } #[stable(since = "1.2.0", feature = "formatter_write")] @@ -2409,25 +2652,27 @@ impl Pointer for *const T { /// [problematic]: https://github.com/rust-lang/rust/issues/95489 pub(crate) fn pointer_fmt_inner(ptr_addr: usize, f: &mut Formatter<'_>) -> Result { let old_width = f.width(); - let old_flags = f.flags(); + let old_alternate = f.alternate(); + let old_zero_pad = f.sign_aware_zero_pad(); // The alternate flag is already treated by LowerHex as being special- // it denotes whether to prefix with 0x. We use it to work out whether // or not to zero extend, and then unconditionally set it to get the // prefix. if f.alternate() { - f.flags |= 1 << (rt::Flag::SignAwareZeroPad as u32); + f.options.sign_aware_zero_pad(true); if f.width().is_none() { - f.width = Some((usize::BITS / 4) as usize + 2); + f.options.width(Some((usize::BITS / 4) as usize + 2)); } } - f.flags |= 1 << (rt::Flag::Alternate as u32); + f.options.alternate(true); let ret = LowerHex::fmt(&ptr_addr, f); - f.width = old_width; - f.flags = old_flags; + f.options.width(old_width); + f.options.alternate(old_alternate); + f.options.sign_aware_zero_pad(old_zero_pad); ret } diff --git a/library/core/tests/fmt/mod.rs b/library/core/tests/fmt/mod.rs index c1c80c46c78b7..003a35dfaf8ac 100644 --- a/library/core/tests/fmt/mod.rs +++ b/library/core/tests/fmt/mod.rs @@ -22,11 +22,11 @@ fn test_pointer_formats_data_pointer() { #[test] fn test_estimated_capacity() { assert_eq!(format_args!("").estimated_capacity(), 0); - assert_eq!(format_args!("{}", {""}).estimated_capacity(), 0); + assert_eq!(format_args!("{}", { "" }).estimated_capacity(), 0); assert_eq!(format_args!("Hello").estimated_capacity(), 5); - assert_eq!(format_args!("Hello, {}!", {""}).estimated_capacity(), 16); - assert_eq!(format_args!("{}, hello!", {"World"}).estimated_capacity(), 0); - assert_eq!(format_args!("{}. 16-bytes piece", {"World"}).estimated_capacity(), 32); + assert_eq!(format_args!("Hello, {}!", { "" }).estimated_capacity(), 16); + assert_eq!(format_args!("{}, hello!", { "World" }).estimated_capacity(), 0); + assert_eq!(format_args!("{}. 16-bytes piece", { "World" }).estimated_capacity(), 32); } #[test] @@ -43,3 +43,31 @@ fn pad_integral_resets() { assert_eq!(format!("{Bar:<03}"), "1 0051 "); } + +#[test] +fn formatting_options_flags() { + use core::fmt::*; + for sign in [None, Some(Sign::Plus), Some(Sign::Minus)] { + for alternate in [true, false] { + for sign_aware_zero_pad in [true, false] { + for debug_as_hex in [None, Some(DebugAsHex::Lower), Some(DebugAsHex::Upper)] { + let mut original_formatting_options = FormattingOptions::new(); + original_formatting_options + .sign(sign) + .sign_aware_zero_pad(sign_aware_zero_pad) + .alternate(alternate) + .debug_as_hex(debug_as_hex); + + let mut formatting_options_with_flags_set_to_self = original_formatting_options; + formatting_options_with_flags_set_to_self + .flags(formatting_options_with_flags_set_to_self.get_flags()); + + assert_eq!( + original_formatting_options, formatting_options_with_flags_set_to_self, + "Reading and setting flags changes FormattingOptions; Sign({sign:?}), Alternate({alternate:?}). DebugAsHex({debug_as_hex:?})" + ) + } + } + } + } +} diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 5946862d3e435..b91f1c8262da2 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -38,6 +38,7 @@ #![feature(flt2dec)] #![feature(fmt_internals)] #![feature(float_minimum_maximum)] +#![feature(formatting_options)] #![feature(future_join)] #![feature(generic_assert_internals)] #![feature(array_try_from_fn)] From 59ebd644054bd07af4cb02b7eefae334611b084e Mon Sep 17 00:00:00 2001 From: Elias Holzmann <9659253+EliasHolzmann@users.noreply.github.com> Date: Wed, 22 Nov 2023 22:50:58 +0100 Subject: [PATCH 3/6] Fixed mir-opt test broken because of `std::Formatter` changes --- ...to_exponential_common.GVN.panic-abort.diff | 109 +++++++++++++++--- ...o_exponential_common.GVN.panic-unwind.diff | 109 +++++++++++++++--- 2 files changed, 192 insertions(+), 26 deletions(-) diff --git a/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-abort.diff b/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-abort.diff index 0ba1bac0a0308..05f11cec61a21 100644 --- a/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-abort.diff +++ b/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-abort.diff @@ -28,29 +28,70 @@ scope 3 { debug precision => _8; let _8: usize; - scope 5 (inlined Formatter::<'_>::precision) { + scope 12 (inlined Formatter::<'_>::precision) { debug self => _1; + let mut _30: &std::fmt::FormattingOptions; + scope 13 (inlined FormattingOptions::get_precision) { + debug self => _30; + } } } } } scope 4 (inlined Formatter::<'_>::sign_plus) { debug self => _1; - let mut _20: u32; - let mut _21: u32; + let mut _20: &std::option::Option; + let _21: std::option::Option; + let mut _22: &std::fmt::FormattingOptions; + let mut _23: &std::option::Option; + scope 5 (inlined FormattingOptions::get_sign) { + debug self => _22; + } + scope 6 (inlined as PartialEq>::eq) { + debug self => _20; + debug other => const _; + scope 7 (inlined ::eq) { + debug l => _20; + debug r => const _; + let mut _24: isize; + let _25: &std::fmt::Sign; + let _26: &std::fmt::Sign; + let mut _27: &std::fmt::Sign; + let mut _28: &std::fmt::Sign; + scope 8 { + debug l => _25; + debug r => _26; + scope 9 (inlined ::eq) { + debug self => _27; + debug other => _28; + let _29: isize; + scope 10 { + debug __self_tag => _29; + scope 11 { + debug __arg1_tag => const 0_isize; + } + } + } + } + } + } } bb0: { StorageLive(_4); +- StorageLive(_23); ++ nop; StorageLive(_20); StorageLive(_21); - _21 = ((*_1).0: u32); - _20 = BitAnd(move _21, const 1_u32); - StorageDead(_21); - _4 = Ne(move _20, const 0_u32); - StorageDead(_20); - StorageLive(_5); - switchInt(_4) -> [0: bb2, otherwise: bb1]; + StorageLive(_22); + _22 = &((*_1).0: std::fmt::FormattingOptions); + _21 = (((*_1).0: std::fmt::FormattingOptions).0: std::option::Option); + _20 = &_21; + StorageDead(_22); + _23 = const _; + StorageLive(_24); + _24 = discriminant(_21); + switchInt(move _24) -> [0: bb10, 1: bb11, otherwise: bb12]; } bb1: { @@ -60,14 +101,17 @@ } bb2: { -- _5 = Minus; -+ _5 = const Minus; +- _5 = core::num::flt2dec::Sign::Minus; ++ _5 = const core::num::flt2dec::Sign::Minus; goto -> bb3; } bb3: { StorageLive(_6); - _6 = ((*_1).4: std::option::Option); + StorageLive(_30); + _30 = &((*_1).0: std::fmt::FormattingOptions); + _6 = (((*_1).0: std::fmt::FormattingOptions).6: std::option::Option); + StorageDead(_30); _7 = discriminant(_6); switchInt(move _7) -> [1: bb4, otherwise: bb6]; } @@ -107,5 +151,44 @@ StorageDead(_6); return; } + + bb9: { + StorageDead(_24); + StorageDead(_20); + StorageDead(_21); +- StorageDead(_23); ++ nop; + StorageLive(_5); + switchInt(_4) -> [0: bb2, otherwise: bb1]; + } + + bb10: { + _4 = const false; + goto -> bb9; + } + + bb11: { + StorageLive(_25); + _25 = &((_21 as Some).0: std::fmt::Sign); + StorageLive(_26); + _26 = &(((*_23) as Some).0: std::fmt::Sign); + StorageLive(_27); + _27 = &((_21 as Some).0: std::fmt::Sign); + StorageLive(_28); + _28 = &(((*_23) as Some).0: std::fmt::Sign); + StorageLive(_29); + _29 = discriminant(((_21 as Some).0: std::fmt::Sign)); + _4 = Eq(_29, const 0_isize); + StorageDead(_29); + StorageDead(_28); + StorageDead(_27); + StorageDead(_26); + StorageDead(_25); + goto -> bb9; + } + + bb12: { + unreachable; + } } diff --git a/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-unwind.diff b/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-unwind.diff index 27ea43ef12b77..030758140bb2b 100644 --- a/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-unwind.diff +++ b/tests/mir-opt/funky_arms.float_to_exponential_common.GVN.panic-unwind.diff @@ -28,29 +28,70 @@ scope 3 { debug precision => _8; let _8: usize; - scope 5 (inlined Formatter::<'_>::precision) { + scope 12 (inlined Formatter::<'_>::precision) { debug self => _1; + let mut _30: &std::fmt::FormattingOptions; + scope 13 (inlined FormattingOptions::get_precision) { + debug self => _30; + } } } } } scope 4 (inlined Formatter::<'_>::sign_plus) { debug self => _1; - let mut _20: u32; - let mut _21: u32; + let mut _20: &std::option::Option; + let _21: std::option::Option; + let mut _22: &std::fmt::FormattingOptions; + let mut _23: &std::option::Option; + scope 5 (inlined FormattingOptions::get_sign) { + debug self => _22; + } + scope 6 (inlined as PartialEq>::eq) { + debug self => _20; + debug other => const _; + scope 7 (inlined ::eq) { + debug l => _20; + debug r => const _; + let mut _24: isize; + let _25: &std::fmt::Sign; + let _26: &std::fmt::Sign; + let mut _27: &std::fmt::Sign; + let mut _28: &std::fmt::Sign; + scope 8 { + debug l => _25; + debug r => _26; + scope 9 (inlined ::eq) { + debug self => _27; + debug other => _28; + let _29: isize; + scope 10 { + debug __self_tag => _29; + scope 11 { + debug __arg1_tag => const 0_isize; + } + } + } + } + } + } } bb0: { StorageLive(_4); +- StorageLive(_23); ++ nop; StorageLive(_20); StorageLive(_21); - _21 = ((*_1).0: u32); - _20 = BitAnd(move _21, const 1_u32); - StorageDead(_21); - _4 = Ne(move _20, const 0_u32); - StorageDead(_20); - StorageLive(_5); - switchInt(_4) -> [0: bb2, otherwise: bb1]; + StorageLive(_22); + _22 = &((*_1).0: std::fmt::FormattingOptions); + _21 = (((*_1).0: std::fmt::FormattingOptions).0: std::option::Option); + _20 = &_21; + StorageDead(_22); + _23 = const _; + StorageLive(_24); + _24 = discriminant(_21); + switchInt(move _24) -> [0: bb10, 1: bb11, otherwise: bb12]; } bb1: { @@ -60,14 +101,17 @@ } bb2: { -- _5 = Minus; -+ _5 = const Minus; +- _5 = core::num::flt2dec::Sign::Minus; ++ _5 = const core::num::flt2dec::Sign::Minus; goto -> bb3; } bb3: { StorageLive(_6); - _6 = ((*_1).4: std::option::Option); + StorageLive(_30); + _30 = &((*_1).0: std::fmt::FormattingOptions); + _6 = (((*_1).0: std::fmt::FormattingOptions).6: std::option::Option); + StorageDead(_30); _7 = discriminant(_6); switchInt(move _7) -> [1: bb4, otherwise: bb6]; } @@ -107,5 +151,44 @@ StorageDead(_6); return; } + + bb9: { + StorageDead(_24); + StorageDead(_20); + StorageDead(_21); +- StorageDead(_23); ++ nop; + StorageLive(_5); + switchInt(_4) -> [0: bb2, otherwise: bb1]; + } + + bb10: { + _4 = const false; + goto -> bb9; + } + + bb11: { + StorageLive(_25); + _25 = &((_21 as Some).0: std::fmt::Sign); + StorageLive(_26); + _26 = &(((*_23) as Some).0: std::fmt::Sign); + StorageLive(_27); + _27 = &((_21 as Some).0: std::fmt::Sign); + StorageLive(_28); + _28 = &(((*_23) as Some).0: std::fmt::Sign); + StorageLive(_29); + _29 = discriminant(((_21 as Some).0: std::fmt::Sign)); + _4 = Eq(_29, const 0_isize); + StorageDead(_29); + StorageDead(_28); + StorageDead(_27); + StorageDead(_26); + StorageDead(_25); + goto -> bb9; + } + + bb12: { + unreachable; + } } From 44c9ec93a8831d78480954f96fe6a484d326bd00 Mon Sep 17 00:00:00 2001 From: Elias Holzmann <9659253+EliasHolzmann@users.noreply.github.com> Date: Wed, 22 Nov 2023 23:40:01 +0100 Subject: [PATCH 4/6] Fixed another broken test --- library/alloc/src/string.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/string.rs b/library/alloc/src/string.rs index 884fbf2c7cc3b..f9903958c8b21 100644 --- a/library/alloc/src/string.rs +++ b/library/alloc/src/string.rs @@ -44,7 +44,6 @@ use core::error::Error; use core::fmt; -use core::fmt::FormattingOptions; use core::hash; #[cfg(not(no_global_oom_handling))] use core::iter::from_fn; @@ -2606,7 +2605,8 @@ impl ToString for T { #[inline] default fn to_string(&self) -> String { let mut buf = String::new(); - let mut formatter = core::fmt::Formatter::new(&mut buf, FormattingOptions::new()); + let mut formatter = + core::fmt::Formatter::new(&mut buf, core::fmt::FormattingOptions::new()); // Bypass format_args!() to avoid write_str with zero-length strs fmt::Display::fmt(self, &mut formatter) .expect("a Display implementation returned an error unexpectedly"); From f71232ee43c01b5d3e63ae98545c735bdca7d338 Mon Sep 17 00:00:00 2001 From: Elias Holzmann <9659253+EliasHolzmann@users.noreply.github.com> Date: Mon, 27 Nov 2023 03:31:29 +0100 Subject: [PATCH 5/6] Formatter::with_options: Use different lifetimes Formatter::with_options takes self as a mutable reference (`&'a mut Formatter<'b>`). `'a` and `'b` need to be different lifetimes. Just taking `&'a mut Formatter<'a>` and trusting in Rust being able to implicitely convert from `&'a mut Formatter<'b>` if necessary (after all, `'a` must be smaller than `'b` anyway) fails because `'b` is behind a *mutable* reference. For background on on this behavior, see https://doc.rust-lang.org/nomicon/subtyping.html#variance. --- library/core/src/fmt/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 5dc37d1e5227c..85192770e6239 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -514,7 +514,7 @@ impl<'a> Formatter<'a> { /// Creates a new formatter based on this one with given [`FormattingOptions`]. #[unstable(feature = "formatting_options", issue = "118117")] - pub fn with_options(&'a mut self, options: FormattingOptions) -> Self { + pub fn with_options<'b>(&'b mut self, options: FormattingOptions) -> Formatter<'b> { Formatter { options, buf: self.buf } } } From 385944ad3c125eeaf6efa22abf6914233617f054 Mon Sep 17 00:00:00 2001 From: Elias Holzmann <9659253+EliasHolzmann@users.noreply.github.com> Date: Mon, 27 Nov 2023 04:06:00 +0100 Subject: [PATCH 6/6] fmt::FormattingOptions: Renamed `alignment` to `align` Likewise for `get_alignment`. This is how the method is named on `Formatter`, I want to keep it consistent. --- library/core/src/fmt/mod.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 85192770e6239..dd4884aea1ba8 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -276,7 +276,7 @@ pub struct FormattingOptions { sign_aware_zero_pad: bool, alternate: bool, fill: char, - alignment: Option, + align: Option, width: Option, precision: Option, debug_as_hex: Option, @@ -300,7 +300,7 @@ impl FormattingOptions { sign_aware_zero_pad: false, alternate: false, fill: ' ', - alignment: None, + align: None, width: None, precision: None, debug_as_hex: None, @@ -357,15 +357,15 @@ impl FormattingOptions { /// The alignment specifies how the value being formatted should be /// positioned if it is smaller than the width of the formatter. #[unstable(feature = "formatting_options", issue = "118117")] - pub fn alignment(&mut self, alignment: Option) -> &mut Self { - self.alignment = alignment; + pub fn align(&mut self, align: Option) -> &mut Self { + self.align = align; self } /// Sets or removes the width. /// /// This is a parameter for the “minimum width” that the format should take /// up. If the value’s string does not fill up this many characters, then - /// the padding specified by [`FormattingOptions::fill`]/[`FormattingOptions::alignment`] + /// the padding specified by [`FormattingOptions::fill`]/[`FormattingOptions::align`] /// will be used to take up the required space. #[unstable(feature = "formatting_options", issue = "118117")] pub fn width(&mut self, width: Option) -> &mut Self { @@ -416,8 +416,8 @@ impl FormattingOptions { } /// Returns the current alignment. #[unstable(feature = "formatting_options", issue = "118117")] - pub fn get_alignment(&self) -> Option { - self.alignment + pub fn get_align(&self) -> Option { + self.align } /// Returns the current width. #[unstable(feature = "formatting_options", issue = "118117")] @@ -1393,7 +1393,7 @@ pub fn write(output: &mut dyn Write, args: Arguments<'_>) -> Result { unsafe fn run(fmt: &mut Formatter<'_>, arg: &rt::Placeholder, args: &[rt::Argument<'_>]) -> Result { fmt.options.fill(arg.fill); - fmt.options.alignment(arg.align.into()); + fmt.options.align(arg.align.into()); fmt.options.flags(arg.flags); // SAFETY: arg and args come from the same Arguments, // which guarantees the indexes are always within bounds. @@ -1556,13 +1556,13 @@ impl<'a> Formatter<'a> { Some(min) if self.sign_aware_zero_pad() => { let old_fill = crate::mem::replace(&mut self.options.fill, '0'); let old_align = - crate::mem::replace(&mut self.options.alignment, Some(Alignment::Right)); + crate::mem::replace(&mut self.options.align, Some(Alignment::Right)); write_prefix(self, sign, prefix)?; let post_padding = self.padding(min - width, Alignment::Right)?; self.buf.write_str(buf)?; post_padding.write(self)?; self.options.fill = old_fill; - self.options.alignment = old_align; + self.options.align = old_align; Ok(()) } // Otherwise, the sign and prefix goes after the padding @@ -1697,7 +1697,7 @@ impl<'a> Formatter<'a> { formatted.sign = ""; width = width.saturating_sub(sign.len()); self.options.fill('0'); - self.options.alignment(Some(Alignment::Right)); + self.options.align(Some(Alignment::Right)); } // remaining parts go through the ordinary padding process. @@ -1715,7 +1715,7 @@ impl<'a> Formatter<'a> { post_padding.write(self) }; self.options.fill(old_fill); - self.options.alignment(old_align); + self.options.align(old_align); ret } else { // this is the common case and we take a shortcut @@ -1899,7 +1899,7 @@ impl<'a> Formatter<'a> { #[must_use] #[stable(feature = "fmt_flags_align", since = "1.28.0")] pub fn align(&self) -> Option { - self.options.get_alignment() + self.options.get_align() } /// Optionally specified integer width that the output should be.