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

@@ -26,6 +26,35 @@ use crate::{
     stdlib::builtins,
 };
 
+fn format_decimal_object(
+    vm: &VirtualMachine,
+    spec: &CFormatSpec,
+    obj: &PyObject,
+) -> PyResult<String> {
+    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::<PyInt>() {
-                                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::<PyInt>() {
+                    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::<PyFloat>(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::<PyInt>() {
-                                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::<PyInt>() {
+                    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::<PyFloat>(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<CFormatQuantity> {
+fn get_star_arg(vm: &VirtualMachine, element: Option<&PyObject>) -> PyResult<i32> {
     match element {
-        Some(width_obj) => {
-            if let Some(i) = width_obj.downcast_ref::<PyInt>() {
-                let i = i.try_to_primitive::<i32>(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<CConversionFlags> {
-    match element {
-        Some(width_obj) => {
-            if let Some(i) = width_obj.downcast_ref::<PyInt>() {
-                let i = i.try_to_primitive::<i32>(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::<PyInt>() {
+                i.try_to_primitive::<i32>(vm)
             } else {
                 Err(vm.new_type_error("* wants int"))
             }
@@ -334,10 +291,11 @@ fn try_update_quantity_from_tuple<'a, I: Iterator<Item = &'a PyObject>>(
         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<Item = &'a PyObject>>(
         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(())
 }
 

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__

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!"
 )