From 778baaecb57704eec6b920e1a759b341b79ebfdc Mon Sep 17 00:00:00 2001 From: Daniel Rainer Date: Sun, 5 Oct 2025 17:17:03 +0200 Subject: [PATCH] sprintf: remove signed int size info The size was used to keep track of the number of bits of the input type the `Arg::SInt` variant was created from. This was only relevant for arguments defined in Rust, since the `printf` command takes all arguments as strings. The only thing the size was used for is for printing negative numbers with the `x` and `X` format specifiers. In these cases, the `i64` stored in the `SInt` variant would be cast to a `u64`, but only the number of bits present in the original argument would be kept, so `-1i8` would be formatted as `ff` instead of `ffffffffffffffff`. There are no users of this feature, so let's simplify the code by removing it. While we're at it, also remove the unused `bool` returned by `as_wrapping_sint`. Closes #11889 --- crates/printf/src/arg.rs | 51 +++++++++++++------------------- crates/printf/src/printf_impl.rs | 2 +- crates/printf/src/tests.rs | 2 +- 3 files changed, 23 insertions(+), 32 deletions(-) diff --git a/crates/printf/src/arg.rs b/crates/printf/src/arg.rs index 21c4c80e1..112f53066 100644 --- a/crates/printf/src/arg.rs +++ b/crates/printf/src/arg.rs @@ -15,7 +15,7 @@ pub enum Arg<'a> { #[cfg(feature = "widestring")] WString(WString), UInt(u64), - SInt(i64, u8), // signed integers track their width as the number of bits + SInt(i64), Float(f64), USizeRef(&'a mut usize), // for use with %n } @@ -59,7 +59,7 @@ pub fn as_str<'s>(&'s self, storage: &'s mut String) -> Result<&'s str, Error> pub fn as_uint(&self) -> Result { match *self { Arg::UInt(u) => Ok(u), - Arg::SInt(i, _w) => i.try_into().map_err(|_| Error::Overflow), + Arg::SInt(i) => i.try_into().map_err(|_| Error::Overflow), _ => Err(Error::BadArgType), } } @@ -68,25 +68,18 @@ pub fn as_uint(&self) -> Result { pub fn as_sint(&self) -> Result { match *self { Arg::UInt(u) => u.try_into().map_err(|_| Error::Overflow), - Arg::SInt(i, _w) => Ok(i), + Arg::SInt(i) => Ok(i), _ => Err(Error::BadArgType), } } - // If this is a signed value, then return the sign (true if negative) and the magnitude, - // masked to the value's width. This allows for e.g. -1 to be returned as 0xFF, 0xFFFF, etc. - // depending on the original width. - // If this is an unsigned value, simply return (false, u64). - pub fn as_wrapping_sint(&self) -> Result<(bool, u64), Error> { + /// Unwraps [`Arg::UInt`] to [`u64`]. + /// Unwraps [`Arg::SInt`] and casts the [`i64`] to [`u64`]. + /// Calling this on other variants of `[Arg]` is an error. + pub fn as_wrapping_sint(&self) -> Result { match *self { - Arg::UInt(u) => Ok((false, u)), - Arg::SInt(i, w) => { - // Need to shift twice in case w is 64. - debug_assert!(w > 0); - let mask = ((1u64 << (w - 1)) << 1).wrapping_sub(1); - let ui = (i as u64) & mask; - Ok((i < 0, ui)) - } + Arg::UInt(u) => Ok(u), + Arg::SInt(i) => Ok(i as u64), _ => Err(Error::BadArgType), } } @@ -97,7 +90,7 @@ pub fn as_float(&self) -> Result { match *self { Arg::Float(f) => Ok(f), Arg::UInt(u) => Ok(u as f64), - Arg::SInt(i, _w) => Ok(i as f64), + Arg::SInt(i) => Ok(i as f64), _ => Err(Error::BadArgType), } } @@ -181,7 +174,7 @@ macro_rules! impl_to_arg { $( impl<'a> ToArg<'a> for $t { fn to_arg(self) -> Arg<'a> { - Arg::SInt(self as i64, <$t>::BITS as u8) + Arg::SInt(self as i64) } } )* @@ -211,8 +204,6 @@ mod tests { #[test] fn test_to_arg() { - const SIZE_WIDTH: u8 = isize::BITS as u8; - assert!(matches!("test".to_arg(), Arg::Str("test"))); assert!(matches!(String::from("test").to_arg(), Arg::Str(_))); #[cfg(feature = "widestring")] @@ -224,17 +215,17 @@ fn test_to_arg() { assert!(matches!('x'.to_arg(), Arg::UInt(120))); let mut usize_val: usize = 0; assert!(matches!((&mut usize_val).to_arg(), Arg::USizeRef(_))); - assert!(matches!(42i8.to_arg(), Arg::SInt(42, 8))); - assert!(matches!(42i16.to_arg(), Arg::SInt(42, 16))); - assert!(matches!(42i32.to_arg(), Arg::SInt(42, 32))); - assert!(matches!(42i64.to_arg(), Arg::SInt(42, 64))); - assert!(matches!(42isize.to_arg(), Arg::SInt(42, SIZE_WIDTH))); + assert!(matches!(42i8.to_arg(), Arg::SInt(42))); + assert!(matches!(42i16.to_arg(), Arg::SInt(42))); + assert!(matches!(42i32.to_arg(), Arg::SInt(42))); + assert!(matches!(42i64.to_arg(), Arg::SInt(42))); + assert!(matches!(42isize.to_arg(), Arg::SInt(42))); - assert_eq!((-42i8).to_arg(), Arg::SInt(-42, 8)); - assert_eq!((-42i16).to_arg(), Arg::SInt(-42, 16)); - assert_eq!((-42i32).to_arg(), Arg::SInt(-42, 32)); - assert_eq!((-42i64).to_arg(), Arg::SInt(-42, 64)); - assert_eq!((-42isize).to_arg(), Arg::SInt(-42, SIZE_WIDTH)); + assert_eq!((-42i8).to_arg(), Arg::SInt(-42)); + assert_eq!((-42i16).to_arg(), Arg::SInt(-42)); + assert_eq!((-42i32).to_arg(), Arg::SInt(-42)); + assert_eq!((-42i64).to_arg(), Arg::SInt(-42)); + assert_eq!((-42isize).to_arg(), Arg::SInt(-42)); assert!(matches!(42u8.to_arg(), Arg::UInt(42))); assert!(matches!(42u16.to_arg(), Arg::UInt(42))); diff --git a/crates/printf/src/printf_impl.rs b/crates/printf/src/printf_impl.rs index 457dd4c09..9ed003398 100644 --- a/crates/printf/src/printf_impl.rs +++ b/crates/printf/src/printf_impl.rs @@ -472,7 +472,7 @@ pub fn sprintf_locale( // If someone passes us a negative value, format it with the width // we were given. let lower = conv_spec.is_lower(); - let (_, uint) = arg.as_wrapping_sint()?; + let uint = arg.as_wrapping_sint()?; if uint != 0 { if flags.alt_form { prefix = if lower { "0x" } else { "0X" }; diff --git a/crates/printf/src/tests.rs b/crates/printf/src/tests.rs index a667e5997..906dea1a4 100644 --- a/crates/printf/src/tests.rs +++ b/crates/printf/src/tests.rs @@ -225,7 +225,7 @@ fn test_int() { assert_fmt!("%d", -123 => "-123"); assert_fmt!("~%d~", 148 => "~148~"); assert_fmt!("00%dxx", -91232 => "00-91232xx"); - assert_fmt!("%x", -9232 => "ffffdbf0"); + assert_fmt!("%x", -9232 => "ffffffffffffdbf0"); assert_fmt!("%X", 432 => "1B0"); assert_fmt!("%09X", 432 => "0000001B0"); assert_fmt!("%9X", 432 => " 1B0");