Skip to content

Commit 14b4f00

Browse files
OddBlokeOddBloke
authored
cformat.rs: fix remaining test_format.test_common_format failures (RustPython#3405)
* cformat.rs: refactor fill_string to take fill_with_precision This allows it to be used with both self.min_field_width and self.precision, which is necessary for padding out %ds with precision. * cformat.rs: zero-pad %d entries using precision This matches CPython's behaviour. * cformat.rs: don't left-adjust when filling with precision 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. * floats: handle alternate form of general formatting * cformat.rs: convert width/precision to i32 In CPython, width can be isize but precision can only be i32. Our implementation currently assumes the same type for both: as CPython's tests assert on overflows for precision, but not for width, we use that size for both. * test_format: run test_common_format Except for the line which raises an OverflowError in CPython, because overflows in Rust (and therefore RustPython) abort the process immediately. * test_types: don't expect test_float_to_string to fail Its string formatting usage now works as expected.
1 parent c5250e2 commit 14b4f00

5 files changed

Lines changed: 72 additions & 30 deletions

File tree

‎Lib/test/test_format.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,6 @@ def test_exc_common(formatstr, args, exception, excmsg):
9494

9595
class FormatTest(unittest.TestCase):
9696

97-
# TODO: RUSTPYTHON
98-
@unittest.expectedFailure
9997
def test_common_format(self):
10098
# test the format identifiers that work the same across
10199
# str, bytes, and bytearrays (integer, float, oct, hex)

‎Lib/test/test_types.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,6 @@ def test_numeric_types(self):
8686
if float(1) == 1.0 and float(-1) == -1.0 and float(0) == 0.0: pass
8787
else: self.fail('float() does not work properly')
8888

89-
# TODO: RUSTPYTHON
90-
@unittest.expectedFailure
9189
def test_float_to_string(self):
9290
def test(f, result):
9391
self.assertEqual(f.__format__('e'), result)

‎common/src/float_ops.rs‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,9 @@ pub fn format_exponent(precision: usize, magnitude: f64, case: Case) -> String {
157157
/// If s represents a floating point value, trailing zeros and a possibly trailing
158158
/// decimal point will be removed.
159159
/// This function does NOT work with decimal commas.
160-
fn remove_trailing_redundant_chars(s: String) -> String {
161-
if s.contains('.') {
162-
// only truncate floating point values
160+
fn maybe_remove_trailing_redundant_chars(s: String, alternate_form: bool) -> String {
161+
if !alternate_form && s.contains('.') {
162+
// only truncate floating point values when not in alternate form
163163
let s = remove_trailing_zeros(s);
164164
remove_trailing_decimal_point(s)
165165
} else {
@@ -183,7 +183,12 @@ fn remove_trailing_decimal_point(s: String) -> String {
183183
s
184184
}
185185

186-
pub fn format_general(precision: usize, magnitude: f64, case: Case) -> String {
186+
pub fn format_general(
187+
precision: usize,
188+
magnitude: f64,
189+
case: Case,
190+
alternate_form: bool,
191+
) -> String {
187192
match magnitude {
188193
magnitude if magnitude.is_finite() => {
189194
let r_exp = format!("{:.*e}", precision.saturating_sub(1), magnitude);
@@ -196,12 +201,18 @@ pub fn format_general(precision: usize, magnitude: f64, case: Case) -> String {
196201
Case::Upper => 'E',
197202
};
198203

199-
let base = remove_trailing_redundant_chars(format!("{:.*}", precision + 1, base));
204+
let base = maybe_remove_trailing_redundant_chars(
205+
format!("{:.*}", precision + 1, base),
206+
alternate_form,
207+
);
200208
format!("{}{}{:+#03}", base, e, exponent)
201209
} else {
202210
let precision = (precision as i64) - 1 - exponent;
203211
let precision = precision as usize;
204-
remove_trailing_redundant_chars(format!("{:.*}", precision, magnitude))
212+
maybe_remove_trailing_redundant_chars(
213+
format!("{:.*}", precision, magnitude),
214+
alternate_form,
215+
)
205216
}
206217
}
207218
magnitude if magnitude.is_nan() => format_nan(case),
@@ -487,11 +498,20 @@ fn test_remove_trailing_decimal_point() {
487498
}
488499

489500
#[test]
490-
fn test_remove_trailing_redundant_chars() {
491-
assert!(remove_trailing_redundant_chars(String::from("100.")) == String::from("100"));
492-
assert!(remove_trailing_redundant_chars(String::from("1.")) == String::from("1"));
493-
assert!(remove_trailing_redundant_chars(String::from("10.0")) == String::from("10"));
501+
fn test_maybe_remove_trailing_redundant_chars() {
502+
assert!(
503+
maybe_remove_trailing_redundant_chars(String::from("100."), true) == String::from("100.")
504+
);
505+
assert!(
506+
maybe_remove_trailing_redundant_chars(String::from("100."), false) == String::from("100")
507+
);
508+
assert!(maybe_remove_trailing_redundant_chars(String::from("1."), false) == String::from("1"));
509+
assert!(
510+
maybe_remove_trailing_redundant_chars(String::from("10.0"), false) == String::from("10")
511+
);
494512

495513
// don't truncate integers
496-
assert!(remove_trailing_redundant_chars(String::from("1000")) == String::from("1000"));
514+
assert!(
515+
maybe_remove_trailing_redundant_chars(String::from("1000"), false) == String::from("1000")
516+
);
497517
}

‎vm/src/cformat.rs‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -167,22 +167,31 @@ impl CFormatSpec {
167167
string: String,
168168
fill_char: char,
169169
num_prefix_chars: Option<usize>,
170+
fill_with_precision: bool,
170171
) -> String {
172+
let target_width = if fill_with_precision {
173+
&self.precision
174+
} else {
175+
&self.min_field_width
176+
};
171177
let mut num_chars = string.chars().count();
172178
if let Some(num_prefix_chars) = num_prefix_chars {
173179
num_chars += num_prefix_chars;
174180
}
175181
let num_chars = num_chars;
176182

177-
let width = match self.min_field_width {
178-
Some(CFormatQuantity::Amount(width)) => cmp::max(width, num_chars),
179-
_ => num_chars,
183+
let width = match target_width {
184+
Some(CFormatQuantity::Amount(width)) => cmp::max(width, &num_chars),
185+
_ => &num_chars,
180186
};
181187
let fill_chars_needed = width.saturating_sub(num_chars);
182188
let fill_string = CFormatSpec::compute_fill_string(fill_char, fill_chars_needed);
183189

184190
if !fill_string.is_empty() {
185-
if self.flags.contains(CConversionFlags::LEFT_ADJUST) {
191+
// Don't left-adjust if precision-filling: that will always be prepending 0s to %d
192+
// arguments, the LEFT_ADJUST flag will be used by a later call to fill_string with
193+
// the 0-filled string as the string param.
194+
if !fill_with_precision && self.flags.contains(CConversionFlags::LEFT_ADJUST) {
186195
format!("{}{}", string, fill_string)
187196
} else {
188197
format!("{}{}", fill_string, string)
@@ -204,7 +213,7 @@ impl CFormatSpec {
204213
}
205214
_ => string,
206215
};
207-
self.fill_string(string, ' ', None)
216+
self.fill_string(string, ' ', None, false)
208217
}
209218

210219
pub(crate) fn format_string(&self, string: String) -> String {
@@ -269,6 +278,8 @@ impl CFormatSpec {
269278
_ => self.flags.sign_string(),
270279
};
271280

281+
let padded_magnitude_string = self.fill_string(magnitude_string, '0', None, true);
282+
272283
if self.flags.contains(CConversionFlags::ZERO_PAD) {
273284
let fill_char = if !self.flags.contains(CConversionFlags::LEFT_ADJUST) {
274285
'0'
@@ -280,16 +291,18 @@ impl CFormatSpec {
280291
"{}{}",
281292
signed_prefix,
282293
self.fill_string(
283-
magnitude_string,
294+
padded_magnitude_string,
284295
fill_char,
285-
Some(signed_prefix.chars().count())
286-
)
296+
Some(signed_prefix.chars().count()),
297+
false
298+
),
287299
)
288300
} else {
289301
self.fill_string(
290-
format!("{}{}{}", sign_string, prefix, magnitude_string),
302+
format!("{}{}{}", sign_string, prefix, padded_magnitude_string),
291303
' ',
292304
None,
305+
false,
293306
)
294307
}
295308
}
@@ -330,7 +343,12 @@ impl CFormatSpec {
330343
CFormatCase::Uppercase => float_ops::Case::Upper,
331344
};
332345
let magnitude = num.abs();
333-
float_ops::format_general(precision, magnitude, case)
346+
float_ops::format_general(
347+
precision,
348+
magnitude,
349+
case,
350+
self.flags.contains(CConversionFlags::ALTERNATE_FORM),
351+
)
334352
}
335353
_ => unreachable!(),
336354
};
@@ -347,11 +365,17 @@ impl CFormatSpec {
347365
self.fill_string(
348366
magnitude_string,
349367
fill_char,
350-
Some(sign_string.chars().count())
368+
Some(sign_string.chars().count()),
369+
false
351370
)
352371
)
353372
} else {
354-
self.fill_string(format!("{}{}", sign_string, magnitude_string), ' ', None)
373+
self.fill_string(
374+
format!("{}{}", sign_string, magnitude_string),
375+
' ',
376+
None,
377+
false,
378+
)
355379
};
356380

357381
formatted
@@ -588,7 +612,7 @@ fn try_update_quantity_from_tuple<'a, I: Iterator<Item = &'a PyObjectRef>>(
588612
Some(CFormatQuantity::FromValuesTuple) => match elements.next() {
589613
Some(width_obj) => {
590614
if let Some(i) = width_obj.payload::<PyInt>() {
591-
let i = i.try_to_primitive::<isize>(vm)?.abs() as usize;
615+
let i = i.try_to_primitive::<i32>(vm)?.abs() as usize;
592616
*q = Some(CFormatQuantity::Amount(i));
593617
Ok(())
594618
} else {
@@ -929,13 +953,13 @@ where
929953
return Ok(Some(CFormatQuantity::FromValuesTuple));
930954
}
931955
if let Some(i) = c.to_digit(10) {
932-
let mut num = i as isize;
956+
let mut num = i as i32;
933957
iter.next().unwrap();
934958
while let Some(&(index, c)) = iter.peek() {
935959
if let Some(i) = c.into().to_digit(10) {
936960
num = num
937961
.checked_mul(10)
938-
.and_then(|num| num.checked_add(i as isize))
962+
.and_then(|num| num.checked_add(i as i32))
939963
.ok_or((CFormatErrorType::IntTooBig, index))?;
940964
iter.next().unwrap();
941965
} else {

‎vm/src/format.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ impl FormatSpec {
376376
precision,
377377
magnitude,
378378
float_ops::Case::Upper,
379+
false,
379380
))
380381
}
381382
Some(FormatType::GeneralFormatLower) => {
@@ -384,6 +385,7 @@ impl FormatSpec {
384385
precision,
385386
magnitude,
386387
float_ops::Case::Lower,
388+
false,
387389
))
388390
}
389391
Some(FormatType::ExponentUpper) => Ok(float_ops::format_exponent(

0 commit comments

Comments
 (0)