From 115357ddd101a19c71b58a5ee77c1d6c741fcdb1 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 19 Jan 2023 20:23:12 -0500 Subject: [PATCH 1/2] Implement lone-dot semantics for %-style format strings --- Lib/test/test_locale.py | 4 -- common/src/cformat.rs | 141 +++++++++++++++++++++++++++++++--------- vm/src/cformat.rs | 29 ++++++++- 3 files changed, 136 insertions(+), 38 deletions(-) diff --git a/Lib/test/test_locale.py b/Lib/test/test_locale.py index 16423808a..25d883f43 100644 --- a/Lib/test/test_locale.py +++ b/Lib/test/test_locale.py @@ -170,8 +170,6 @@ class EnUSNumberFormatting(BaseFormattingTest): self._test_format("%f", -42, grouping=1, out='-42.000000') self._test_format("%+f", -42, grouping=1, out='-42.000000') - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_grouping_and_padding(self): self._test_format("%20.f", -42, grouping=1, out='-42'.rjust(20)) if self.sep: @@ -197,8 +195,6 @@ class EnUSNumberFormatting(BaseFormattingTest): self._test_format("%f", -42, grouping=0, out='-42.000000') self._test_format("%+f", -42, grouping=0, out='-42.000000') - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_padding(self): self._test_format("%20.f", -42, grouping=0, out='-42'.rjust(20)) self._test_format("%+10.f", -4200, grouping=0, out='-4200'.rjust(10)) diff --git a/common/src/cformat.rs b/common/src/cformat.rs index a1e97169f..4f691e267 100644 --- a/common/src/cformat.rs +++ b/common/src/cformat.rs @@ -34,7 +34,7 @@ impl fmt::Display for CFormatError { use CFormatErrorType::*; match self.typ { UnmatchedKeyParentheses => write!(f, "incomplete format key"), - CFormatErrorType::IncompleteFormat => write!(f, "incomplete format"), + IncompleteFormat => write!(f, "incomplete format"), UnsupportedFormatChar(c) => write!( f, "unsupported format character '{}' ({:#x}) at index {}", @@ -76,6 +76,18 @@ pub enum CFormatType { String(CFormatPreconversor), } +#[derive(Debug, PartialEq)] +pub enum CFormatPrecision { + Quantity(CFormatQuantity), + Dot, +} + +impl From for CFormatPrecision { + fn from(quantity: CFormatQuantity) -> Self { + CFormatPrecision::Quantity(quantity) + } +} + bitflags! { pub struct CConversionFlags: u32 { const ALTERNATE_FORM = 0b0000_0001; @@ -110,7 +122,7 @@ pub struct CFormatSpec { pub mapping_key: Option, pub flags: CConversionFlags, pub min_field_width: Option, - pub precision: Option, + pub precision: Option, pub format_type: CFormatType, pub format_char: char, // chars_consumed: usize, @@ -143,10 +155,6 @@ impl CFormatSpec { let precision = parse_precision(iter)?; consume_length(iter); let (format_type, format_char) = parse_format_type(iter)?; - let precision = precision.or(match format_type { - CFormatType::Float(_) => Some(CFormatQuantity::Amount(6)), - _ => None, - }); Ok(CFormatSpec { mapping_key, @@ -169,20 +177,14 @@ impl CFormatSpec { string: String, fill_char: char, num_prefix_chars: Option, - fill_with_precision: bool, ) -> String { - let target_width = if fill_with_precision { - &self.precision - } else { - &self.min_field_width - }; let mut num_chars = string.chars().count(); if let Some(num_prefix_chars) = num_prefix_chars { num_chars += num_prefix_chars; } let num_chars = num_chars; - let width = match target_width { + let width = match &self.min_field_width { Some(CFormatQuantity::Amount(width)) => cmp::max(width, &num_chars), _ => &num_chars, }; @@ -190,10 +192,7 @@ impl CFormatSpec { let fill_string = CFormatSpec::compute_fill_string(fill_char, fill_chars_needed); if !fill_string.is_empty() { - // Don't left-adjust if precision-filling: that will always be prepending 0s to %d - // arguments, the LEFT_ADJUST flag will be used by a later call to fill_string with - // the 0-filled string as the string param. - if !fill_with_precision && self.flags.contains(CConversionFlags::LEFT_ADJUST) { + if self.flags.contains(CConversionFlags::LEFT_ADJUST) { format!("{string}{fill_string}") } else { format!("{fill_string}{string}") @@ -203,19 +202,47 @@ impl CFormatSpec { } } + fn fill_string_with_precision(&self, string: String, fill_char: char) -> String { + let num_chars = string.chars().count(); + + let width = match &self.precision { + Some(CFormatPrecision::Quantity(CFormatQuantity::Amount(width))) => { + cmp::max(width, &num_chars) + } + _ => &num_chars, + }; + let fill_chars_needed = width.saturating_sub(num_chars); + let fill_string = CFormatSpec::compute_fill_string(fill_char, fill_chars_needed); + + if !fill_string.is_empty() { + // Don't left-adjust if precision-filling: that will always be prepending 0s to %d + // arguments, the LEFT_ADJUST flag will be used by a later call to fill_string with + // the 0-filled string as the string param. + format!("{fill_string}{string}") + } else { + string + } + } + fn format_string_with_precision( &self, string: String, - precision: Option<&CFormatQuantity>, + precision: Option<&CFormatPrecision>, ) -> String { // truncate if needed let string = match precision { - Some(CFormatQuantity::Amount(precision)) if string.chars().count() > *precision => { + Some(CFormatPrecision::Quantity(CFormatQuantity::Amount(precision))) + if string.chars().count() > *precision => + { string.chars().take(*precision).collect::() } + Some(CFormatPrecision::Dot) => { + // truncate to 0 + String::new() + } _ => string, }; - self.fill_string(string, ' ', None, false) + self.fill_string(string, ' ', None) } #[inline] @@ -225,11 +252,16 @@ impl CFormatSpec { #[inline] pub fn format_char(&self, ch: char) -> String { - self.format_string_with_precision(ch.to_string(), Some(&CFormatQuantity::Amount(1))) + self.format_string_with_precision( + ch.to_string(), + Some(&(CFormatQuantity::Amount(1).into())), + ) } pub fn format_bytes(&self, bytes: &[u8]) -> Vec { - let bytes = if let Some(CFormatQuantity::Amount(precision)) = self.precision { + let bytes = if let Some(CFormatPrecision::Quantity(CFormatQuantity::Amount(precision))) = + self.precision + { &bytes[..cmp::min(bytes.len(), precision)] } else { bytes @@ -282,7 +314,7 @@ impl CFormatSpec { _ => self.flags.sign_string(), }; - let padded_magnitude_string = self.fill_string(magnitude_string, '0', None, true); + let padded_magnitude_string = self.fill_string_with_precision(magnitude_string, '0'); if self.flags.contains(CConversionFlags::ZERO_PAD) { let fill_char = if !self.flags.contains(CConversionFlags::LEFT_ADJUST) { @@ -298,7 +330,6 @@ impl CFormatSpec { padded_magnitude_string, fill_char, Some(signed_prefix.chars().count()), - false ), ) } else { @@ -306,7 +337,6 @@ impl CFormatSpec { format!("{sign_string}{prefix}{padded_magnitude_string}"), ' ', None, - false, ) } } @@ -318,9 +348,13 @@ impl CFormatSpec { self.flags.sign_string() }; - let precision = match self.precision { - Some(CFormatQuantity::Amount(p)) => p, - _ => 6, + let precision = match &self.precision { + Some(CFormatPrecision::Quantity(quantity)) => match quantity { + CFormatQuantity::Amount(amount) => *amount, + CFormatQuantity::FromValuesTuple => 6, + }, + Some(CFormatPrecision::Dot) => 0, + None => 6, }; let magnitude_string = match &self.format_type { @@ -381,11 +415,10 @@ impl CFormatSpec { magnitude_string, fill_char, Some(sign_string.chars().count()), - false ) ) } else { - self.fill_string(format!("{sign_string}{magnitude_string}"), ' ', None, false) + self.fill_string(format!("{sign_string}{magnitude_string}"), ' ', None) } } } @@ -510,7 +543,7 @@ where Ok(None) } -fn parse_precision(iter: &mut ParseIter) -> Result, ParsingError> +fn parse_precision(iter: &mut ParseIter) -> Result, ParsingError> where T: Into + Copy, I: Iterator, @@ -518,7 +551,11 @@ where if let Some(&(_, c)) = iter.peek() { if c.into() == '.' { iter.next().unwrap(); - return parse_quantity(iter); + return Ok(Some( + parse_quantity(iter)? + .map(CFormatPrecision::Quantity) + .unwrap_or(CFormatPrecision::Dot), + )); } } Ok(None) @@ -848,6 +885,20 @@ mod tests { .format_string("Hello, World!".to_owned()), "Hell ".to_owned() ); + assert_eq!( + "%.s" + .parse::() + .unwrap() + .format_string("Hello, World!".to_owned()), + "".to_owned() + ); + assert_eq!( + "%5.s" + .parse::() + .unwrap() + .format_string("Hello, World!".to_owned()), + " ".to_owned() + ); } #[test] @@ -863,6 +914,13 @@ mod tests { #[test] fn test_parse_and_format_number() { + assert_eq!( + "%5d" + .parse::() + .unwrap() + .format_number(&BigInt::from(27)), + " 27".to_owned() + ); assert_eq!( "%05d" .parse::() @@ -870,6 +928,13 @@ mod tests { .format_number(&BigInt::from(27)), "00027".to_owned() ); + assert_eq!( + "%.5d" + .parse::() + .unwrap() + .format_number(&BigInt::from(27)), + "00027".to_owned() + ); assert_eq!( "%+05d" .parse::() @@ -927,6 +992,18 @@ mod tests { "%f".parse::().unwrap().format_float(1.2345), "1.234500" ); + assert_eq!( + "%.2f".parse::().unwrap().format_float(1.2345), + "1.23" + ); + assert_eq!( + "%.f".parse::().unwrap().format_float(1.2345), + "1" + ); + assert_eq!( + "%+.f".parse::().unwrap().format_float(1.2345), + "+1" + ); assert_eq!( "%+f".parse::().unwrap().format_float(1.2345), "+1.234500" diff --git a/vm/src/cformat.rs b/vm/src/cformat.rs index 09fa71a82..41e780e4f 100644 --- a/vm/src/cformat.rs +++ b/vm/src/cformat.rs @@ -223,6 +223,31 @@ fn try_update_quantity_from_tuple<'a, I: Iterator>( } } +fn try_update_precision_from_tuple<'a, I: Iterator>( + vm: &VirtualMachine, + elements: &mut I, + p: &mut Option, +) -> PyResult<()> { + match p { + Some(CFormatPrecision::Quantity(CFormatQuantity::FromValuesTuple)) => match elements.next() + { + Some(width_obj) => { + if let Some(i) = width_obj.payload::() { + let i = i.try_to_primitive::(vm)?.unsigned_abs(); + *p = Some(CFormatPrecision::Quantity(CFormatQuantity::Amount( + i as usize, + ))); + Ok(()) + } else { + Err(vm.new_type_error("* wants int".to_owned())) + } + } + None => Err(vm.new_type_error("not enough arguments for format string".to_owned())), + }, + _ => Ok(()), + } +} + fn specifier_error(vm: &VirtualMachine) -> PyBaseExceptionRef { vm.new_type_error("format requires a mapping".to_owned()) } @@ -299,7 +324,7 @@ pub(crate) fn cformat_bytes( CFormatPart::Literal(literal) => result.append(literal), CFormatPart::Spec(spec) => { try_update_quantity_from_tuple(vm, &mut value_iter, &mut spec.min_field_width)?; - try_update_quantity_from_tuple(vm, &mut value_iter, &mut spec.precision)?; + try_update_precision_from_tuple(vm, &mut value_iter, &mut spec.precision)?; let value = match value_iter.next() { Some(obj) => Ok(obj.clone()), @@ -393,7 +418,7 @@ pub(crate) fn cformat_string( CFormatPart::Literal(literal) => result.push_str(literal), CFormatPart::Spec(spec) => { try_update_quantity_from_tuple(vm, &mut value_iter, &mut spec.min_field_width)?; - try_update_quantity_from_tuple(vm, &mut value_iter, &mut spec.precision)?; + try_update_precision_from_tuple(vm, &mut value_iter, &mut spec.precision)?; let value = match value_iter.next() { Some(obj) => Ok(obj.clone()), From fc0b08042d2c8fc9cbecfb3b7bfd03bdd92b193b Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sat, 21 Jan 2023 11:53:56 +0900 Subject: [PATCH 2/2] refactor try_update_quantity_from_element --- common/src/cformat.rs | 8 +++--- vm/src/cformat.rs | 62 ++++++++++++++++++++----------------------- 2 files changed, 32 insertions(+), 38 deletions(-) diff --git a/common/src/cformat.rs b/common/src/cformat.rs index 4f691e267..2c1eda5c4 100644 --- a/common/src/cformat.rs +++ b/common/src/cformat.rs @@ -551,11 +551,9 @@ where if let Some(&(_, c)) = iter.peek() { if c.into() == '.' { iter.next().unwrap(); - return Ok(Some( - parse_quantity(iter)? - .map(CFormatPrecision::Quantity) - .unwrap_or(CFormatPrecision::Dot), - )); + let quantity = parse_quantity(iter)?; + let precision = quantity.map_or(CFormatPrecision::Dot, CFormatPrecision::Quantity); + return Ok(Some(precision)); } } Ok(None) diff --git a/vm/src/cformat.rs b/vm/src/cformat.rs index 41e780e4f..3148975a8 100644 --- a/vm/src/cformat.rs +++ b/vm/src/cformat.rs @@ -201,26 +201,34 @@ fn spec_format_string( } } +fn try_update_quantity_from_element( + vm: &VirtualMachine, + element: Option<&PyObjectRef>, +) -> PyResult { + match element { + Some(width_obj) => { + if let Some(i) = width_obj.payload::() { + let i = i.try_to_primitive::(vm)?.unsigned_abs(); + Ok(CFormatQuantity::Amount(i as usize)) + } else { + Err(vm.new_type_error("* wants int".to_owned())) + } + } + None => Err(vm.new_type_error("not enough arguments for format string".to_owned())), + } +} + fn try_update_quantity_from_tuple<'a, I: Iterator>( vm: &VirtualMachine, elements: &mut I, q: &mut Option, ) -> PyResult<()> { - match q { - Some(CFormatQuantity::FromValuesTuple) => match elements.next() { - Some(width_obj) => { - if let Some(i) = width_obj.payload::() { - let i = i.try_to_primitive::(vm)?.unsigned_abs(); - *q = Some(CFormatQuantity::Amount(i as usize)); - Ok(()) - } else { - Err(vm.new_type_error("* wants int".to_owned())) - } - } - None => Err(vm.new_type_error("not enough arguments for format string".to_owned())), - }, - _ => Ok(()), - } + let Some(CFormatQuantity::FromValuesTuple) = q else { + return Ok(()); + }; + let quantity = try_update_quantity_from_element(vm, elements.next())?; + *q = Some(quantity); + Ok(()) } fn try_update_precision_from_tuple<'a, I: Iterator>( @@ -228,24 +236,12 @@ fn try_update_precision_from_tuple<'a, I: Iterator>( elements: &mut I, p: &mut Option, ) -> PyResult<()> { - match p { - Some(CFormatPrecision::Quantity(CFormatQuantity::FromValuesTuple)) => match elements.next() - { - Some(width_obj) => { - if let Some(i) = width_obj.payload::() { - let i = i.try_to_primitive::(vm)?.unsigned_abs(); - *p = Some(CFormatPrecision::Quantity(CFormatQuantity::Amount( - i as usize, - ))); - Ok(()) - } else { - Err(vm.new_type_error("* wants int".to_owned())) - } - } - None => Err(vm.new_type_error("not enough arguments for format string".to_owned())), - }, - _ => Ok(()), - } + let Some(CFormatPrecision::Quantity(CFormatQuantity::FromValuesTuple)) = p else { + return Ok(()); + }; + let quantity = try_update_quantity_from_element(vm, elements.next())?; + *p = Some(CFormatPrecision::Quantity(quantity)); + Ok(()) } fn specifier_error(vm: &VirtualMachine) -> PyBaseExceptionRef {