From 403a38cf49b46274d8f3075a1541b08a7a4fe226 Mon Sep 17 00:00:00 2001 From: 1ndahous3 <9205230+1ndahous3@users.noreply.github.com> Date: Mon, 28 Sep 2026 16:08:26 +0300 Subject: [PATCH] Fix percent-format precision and integer conversion (#8876) * Fix percent-format precision and integer conversion Assisted-by: Codex:GPT-6 * Honor float subclass integer conversions in percent formatting Assisted-by: Codex:GPT-6 --- crates/vm/src/cformat.rs | 164 ++++++++++---------------- extra_tests/snippets/builtin_bytes.py | 8 ++ extra_tests/snippets/builtin_str.py | 60 ++++++++++ 3 files changed, 130 insertions(+), 102 deletions(-) diff --git a/crates/vm/src/cformat.rs b/crates/vm/src/cformat.rs index 4cdb98e59..c9533835f 100644 --- a/crates/vm/src/cformat.rs +++ b/crates/vm/src/cformat.rs @@ -26,6 +26,35 @@ use crate::{ stdlib::builtins, }; +fn format_decimal_object( + vm: &VirtualMachine, + spec: &CFormatSpec, + obj: &PyObject, +) -> PyResult { + let type_error = || { + vm.new_type_error(format!( + "%{} format: a real number is required, not {}", + spec.format_type.to_char(), + obj.class().slot_name() + )) + }; + // Decimal conversions prefer __int__; __index__ is only a fallback when absent. + let i = obj + .number() + .int(vm) + .or_else(|| obj.try_index_opt(vm)) + .ok_or_else(type_error)? + .map_err(|error| { + if error.fast_isinstance(vm.ctx.exceptions.type_error) { + type_error() + } else { + error + } + })?; + check_int_to_str_digits(i.as_bigint(), vm)?; + Ok(spec.format_number(i.as_bigint())) +} + fn spec_format_bytes( vm: &VirtualMachine, spec: &CFormatSpec, @@ -69,40 +98,16 @@ fn spec_format_bytes( } CFormatType::Number(number_type) => match number_type { CNumberType::DecimalD | CNumberType::DecimalI | CNumberType::DecimalU => { - match_class!(match &obj { - ref i @ PyInt => { - check_int_to_str_digits(i.as_bigint(), vm)?; - Ok(spec.format_number(i.as_bigint()).into_bytes()) - } - ref f @ PyFloat => { - let bigint = try_f64_to_bigint(f.to_f64(), vm)?; - check_int_to_str_digits(&bigint, vm)?; - Ok(spec.format_number(&bigint).into_bytes()) - } - obj => { - // CPython parity: `%d` / `%i` / `%u` accept any object - // with `__index__` (preferred) or `__int__`. - if let Some(int_result) = obj.try_index_opt(vm) { - let i = int_result?; - check_int_to_str_digits(i.as_bigint(), vm)?; - return Ok(spec.format_number(i.as_bigint()).into_bytes()); - } - - if let Some(method) = vm.get_method(obj.clone(), identifier!(vm, __int__)) { - let result = method?.call((), vm)?; - if let Some(i) = result.downcast_ref::() { - check_int_to_str_digits(i.as_bigint(), vm)?; - return Ok(spec.format_number(i.as_bigint()).into_bytes()); - } - } - - Err(vm.new_type_error(format!( - "%{} format: a real number is required, not {}", - spec.format_type.to_char(), - obj.class().slot_name() - ))) - } - }) + if let Some(i) = obj.downcast_ref::() { + check_int_to_str_digits(i.as_bigint(), vm)?; + Ok(spec.format_number(i.as_bigint()).into_bytes()) + } else if let Some(f) = obj.downcast_ref_if_exact::(vm) { + let bigint = try_f64_to_bigint(f.to_f64(), vm)?; + check_int_to_str_digits(&bigint, vm)?; + Ok(spec.format_number(&bigint).into_bytes()) + } else { + format_decimal_object(vm, spec, &obj).map(String::into_bytes) + } } _ => { // CPython parity: `%x` / `%o` / `%X` accept any object with @@ -198,38 +203,16 @@ fn spec_format_string( } CFormatType::Number(number_type) => match number_type { CNumberType::DecimalD | CNumberType::DecimalI | CNumberType::DecimalU => { - match_class!(match &obj { - ref i @ PyInt => { - check_int_to_str_digits(i.as_bigint(), vm)?; - Ok(spec.format_number(i.as_bigint()).into()) - } - ref f @ PyFloat => { - let bigint = try_f64_to_bigint(f.to_f64(), vm)?; - check_int_to_str_digits(&bigint, vm)?; - Ok(spec.format_number(&bigint).into()) - } - obj => { - // CPython parity: `%d` / `%i` / `%u` accept any object - // with `__index__` (preferred) or `__int__`. - if let Some(int_result) = obj.try_index_opt(vm) { - let i = int_result?; - check_int_to_str_digits(i.as_bigint(), vm)?; - return Ok(spec.format_number(i.as_bigint()).into()); - } - if let Some(method) = vm.get_method(obj.clone(), identifier!(vm, __int__)) { - let result = method?.call((), vm)?; - if let Some(i) = result.downcast_ref::() { - check_int_to_str_digits(i.as_bigint(), vm)?; - return Ok(spec.format_number(i.as_bigint()).into()); - } - } - Err(vm.new_type_error(format!( - "%{} format: a real number is required, not {}", - spec.format_type.to_char(), - obj.class().slot_name() - ))) - } - }) + if let Some(i) = obj.downcast_ref::() { + check_int_to_str_digits(i.as_bigint(), vm)?; + Ok(spec.format_number(i.as_bigint()).into()) + } else if let Some(f) = obj.downcast_ref_if_exact::(vm) { + let bigint = try_f64_to_bigint(f.to_f64(), vm)?; + check_int_to_str_digits(&bigint, vm)?; + Ok(spec.format_number(&bigint).into()) + } else { + format_decimal_object(vm, spec, &obj).map(Into::into) + } } _ => { // CPython parity: `%x` / `%o` / `%X` accept any object with @@ -285,37 +268,11 @@ fn spec_format_string( } } -fn try_update_quantity_from_element( - vm: &VirtualMachine, - element: Option<&PyObject>, -) -> PyResult { +fn get_star_arg(vm: &VirtualMachine, element: Option<&PyObject>) -> PyResult { match element { - Some(width_obj) => { - if let Some(i) = width_obj.downcast_ref::() { - let i = i.try_to_primitive::(vm)?.unsigned_abs(); - Ok(CFormatQuantity::Amount(i as usize)) - } else { - Err(vm.new_type_error("* wants int")) - } - } - None => Err(vm.new_type_error("not enough arguments for format string")), - } -} - -fn try_conversion_flag_from_tuple( - vm: &VirtualMachine, - element: Option<&PyObject>, -) -> PyResult { - match element { - Some(width_obj) => { - if let Some(i) = width_obj.downcast_ref::() { - let i = i.try_to_primitive::(vm)?; - let flags = if i < 0 { - CConversionFlags::LEFT_ADJUST - } else { - CConversionFlags::from_bits(0).unwrap() - }; - Ok(flags) + Some(obj) => { + if let Some(i) = obj.downcast_ref::() { + i.try_to_primitive::(vm) } else { Err(vm.new_type_error("* wants int")) } @@ -334,10 +291,11 @@ fn try_update_quantity_from_tuple<'a, I: Iterator>( return Ok(()); }; - let element = elements.next(); - f.insert(try_conversion_flag_from_tuple(vm, element)?); - let quantity = try_update_quantity_from_element(vm, element)?; - *q = Some(quantity); + let width = get_star_arg(vm, elements.next())?; + if width < 0 { + f.insert(CConversionFlags::LEFT_ADJUST); + } + *q = Some(CFormatQuantity::Amount(width.unsigned_abs() as usize)); Ok(()) } @@ -350,8 +308,10 @@ fn try_update_precision_from_tuple<'a, I: Iterator>( return Ok(()); }; - let quantity = try_update_quantity_from_element(vm, elements.next())?; - *p = Some(CFormatPrecision::Quantity(quantity)); + let precision = get_star_arg(vm, elements.next())?.max(0) as usize; + *p = Some(CFormatPrecision::Quantity(CFormatQuantity::Amount( + precision, + ))); Ok(()) } diff --git a/extra_tests/snippets/builtin_bytes.py b/extra_tests/snippets/builtin_bytes.py index eb03698db..57694f6d6 100644 --- a/extra_tests/snippets/builtin_bytes.py +++ b/extra_tests/snippets/builtin_bytes.py @@ -644,6 +644,14 @@ assert b"-\xff".decode(sys.getfilesystemencoding(), "surrogateescape") == "-\udc # mod assert b"rust%bpython%b" % (b" ", b"!") == b"rust python!" assert b"x=%i y=%f" % (1, 2.5) == b"x=1 y=2.500000" +for precision in (-1, -3, -(2**31)): + assert b"%.*b" % (precision, b"hello") == b"" +assert b"%.*s" % (-3, bytearray(b"hello")) == b"" +assert b"%.*a" % (-3, "hello") == b"" +assert b"%*.*d" % (-6, -3, 12) == b"12 " +assert b"%.*f" % (-3, 1.25) == b"1" +assert_raises(OverflowError, b"%.*b".__mod__, (-(2**31) - 1, b"abc")) +assert_raises(TypeError, b"%.*b".__mod__, (1.0, b"abc")) # __bytes__ diff --git a/extra_tests/snippets/builtin_str.py b/extra_tests/snippets/builtin_str.py index 5c52b0d54..4687d6f5b 100644 --- a/extra_tests/snippets/builtin_str.py +++ b/extra_tests/snippets/builtin_str.py @@ -338,6 +338,66 @@ assert ( # Printf-style String formatting assert "%d %d" % (1, 2) == "1 2" assert "%*c " % (3, "❤") == " ❤ " +for precision in (-1, -3, -(2**31)): + assert "%.*s" % (precision, "🐍hello") == "" +assert "%*.*d" % (-6, -3, 12) == "12 " +assert "%.*f" % (-3, 1.25) == "1" +assert "%.*g" % (-3, 12.5) == "1e+01" +assert_raises(OverflowError, "%.*s".__mod__, (-(2**31) - 1, "abc")) +assert_raises(TypeError, "%.*s".__mod__, (1.0, "abc")) + + +class PercentIndex: + def __index__(self): + return 7 + + +class PercentInt(PercentIndex): + def __init__(self, value): + self.value = value + + def __int__(self): + if isinstance(self.value, Exception): + raise self.value + return self.value + + +class PercentFloat(float): + __int__ = PercentInt.__int__ + __index__ = PercentIndex.__index__ + + +class PercentFloatIndex(float): + __index__ = PercentIndex.__index__ + + +class PercentIntSubclass(int): + def __int__(self): + raise AssertionError("int subclasses must use their stored value") + + +for template in ("%d", "%i", "%u", b"%d", b"%i", b"%u"): + assert template % PercentInt(3) == template % 3 + assert template % PercentIndex() == template % 7 + for value in (None, TypeError("conversion failed")): + with assert_raises(TypeError) as cm: + template % PercentInt(value) + assert str(cm.exception).endswith("a real number is required, not PercentInt") + assert_raises( + RuntimeError, template.__mod__, PercentInt(RuntimeError("conversion failed")) + ) + number = PercentFloat(1.25) + number.value = 3 + assert template % number == template % 3 + number.value = None + assert_raises(TypeError, template.__mod__, number) + number.value = RuntimeError("conversion failed") + assert_raises(RuntimeError, template.__mod__, number) + assert template % PercentFloatIndex(1.25) == template % 1 + assert template % PercentIntSubclass(1) == template % 1 +assert "%x" % PercentInt(3) == "7" +assert b"%o" % PercentInt(3) == b"7" + assert ( "%(first)s %(second)s" % {"second": "World!", "first": "Hello,"} == "Hello, World!" )