Skip to content

Commit

Permalink
Use PyNumber_Index when converting objects to integers.
Browse files Browse the repository at this point in the history
Python has three ways of obtaining an integer from an object:
1) directly calling `PyLong_AsLong`
  * uses `__index__` if available
  * in Python 3.9 and ealier, but not in 3.10, uses `__int__` if `__index__` is unavailable
  * throws `TypeError` if the conversion is not possible
2) using `PyNumber_Long`
  * uses `__int__`
  * may throw other exceptions like `ValueError`, e.g. when converting from empty string
3) using `PyNumber_Index`
  * always uses `__index__` with any Python version
  * throws `TypeError` if the conversion is not possible

Previously, the behavior of rust-python was inconsistent: small integer types (e.g. `i32`) used approach 1; where as larger integer types (e.g. `i64`) used approach 2.
This this commit, all integer types consistently use approach 3.

This is a breaking change: extension methods declared with `arg: i32` parameters no longer accept Python floating-point values.
This fixes `objects::num::test::float_to_*` test failures with Python 3.10-rc1 (#265).
  • Loading branch information
dgrunwald committed Sep 1, 2021
1 parent 8d94120 commit 349baa3
Showing 1 changed file with 63 additions and 10 deletions.
73 changes: 63 additions & 10 deletions src/objects/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use super::object::PyObject;
use crate::conversion::{FromPyObject, ToPyObject};
use crate::err::{self, PyErr, PyResult};
use crate::ffi;
use crate::python::{PyClone, Python, PythonObject};
use crate::python::{PyClone, PyDrop, Python, PythonObject};

/// Represents a Python `int` object.
///
Expand Down Expand Up @@ -133,7 +133,17 @@ macro_rules! int_fits_c_long(
/// Returns OverflowError if the input integer does not fit the Rust type;
/// or TypeError if the input is not an integer.
py => {
let val = unsafe { ffi::PyLong_AsLong(obj.as_ptr()) };
let ptr = obj.as_ptr();
let val;
unsafe {
if ffi::PyLong_Check(ptr) != 0 {
val = ffi::PyLong_AsLong(obj.as_ptr());
} else {
let num = err::result_from_owned_ptr(py, ffi::PyNumber_Index(ptr))?;
val = ffi::PyLong_AsLong(num.as_ptr());
num.release_ref(py);
}
};
if val == -1 && PyErr::occurred(py) {
return Err(PyErr::fetch(py));
}
Expand Down Expand Up @@ -236,8 +246,10 @@ macro_rules! int_convert_u64_or_i64 (
None => Err(overflow_error(py))
}
} else {
let num = err::result_from_owned_ptr(py, ffi::PyNumber_Long(ptr))?;
err_if_invalid_value(py, !0, $pylong_as_ull_or_ull(num.as_ptr()))
let num = err::result_from_owned_ptr(py, ffi::PyNumber_Index(ptr))?;
let res = err_if_invalid_value(py, !0, $pylong_as_ull_or_ull(num.as_ptr()));
num.release_ref(py);
res
}
}
}
Expand All @@ -249,8 +261,10 @@ macro_rules! int_convert_u64_or_i64 (
if ffi::PyLong_Check(ptr) != 0 {
err_if_invalid_value(py, !0, $pylong_as_ull_or_ull(ptr))
} else {
let num = err::result_from_owned_ptr(py, ffi::PyNumber_Long(ptr))?;
err_if_invalid_value(py, !0, $pylong_as_ull_or_ull(num.as_ptr()))
let num = err::result_from_owned_ptr(py, ffi::PyNumber_Index(ptr))?;
let res = err_if_invalid_value(py, !0, $pylong_as_ull_or_ull(num.as_ptr()));
num.release_ref(py);
res
}
}
}
Expand Down Expand Up @@ -339,6 +353,7 @@ extract!(

#[cfg(test)]
mod test {
use crate::exc;
use crate::conversion::ToPyObject;
use crate::python::{Python, PythonObject};

Expand Down Expand Up @@ -367,12 +382,50 @@ mod test {
num_to_py_object_and_back!(to_from_u64, u64, u64);
num_to_py_object_and_back!(to_from_isize, isize, isize);
num_to_py_object_and_back!(to_from_usize, usize, usize);
num_to_py_object_and_back!(float_to_i32, f64, i32);
num_to_py_object_and_back!(float_to_u32, f64, u32);
num_to_py_object_and_back!(float_to_i64, f64, i64);
num_to_py_object_and_back!(float_to_u64, f64, u64);
num_to_py_object_and_back!(int_to_float, i32, f64);


macro_rules! float_to_int_fails (
($func_name:ident, $t:ty) => (
#[test]
fn $func_name() {
let gil = Python::acquire_gil();
let py = gil.python();
let obj = (1.0f64).to_py_object(py).into_object();
let err = obj.extract::<$t>(py).unwrap_err();
assert!(err.matches(py, py.get_type::<exc::TypeError>()));
}
)
);
float_to_int_fails!(float_to_i32, i32);
float_to_int_fails!(float_to_u32, u32);
float_to_int_fails!(float_to_i64, i64);
float_to_int_fails!(float_to_u64, u64);

macro_rules! str_to_int_fails (
($func_name:ident, $t:ty) => (
#[test]
fn $func_name() {
let gil = Python::acquire_gil();
let py = gil.python();
// empty string
let obj = "".to_py_object(py).into_object();
let err = obj.extract::<$t>(py).unwrap_err();
assert!(err.matches(py, py.get_type::<exc::TypeError>()));

// numeric-looking string
let obj = "1".to_py_object(py).into_object();
let err = obj.extract::<$t>(py).unwrap_err();
assert!(err.matches(py, py.get_type::<exc::TypeError>()));
}
)
);

str_to_int_fails!(str_to_i32, i32);
str_to_int_fails!(str_to_u32, u32);
str_to_int_fails!(str_to_i64, i64);
str_to_int_fails!(str_to_u64, u64);

#[test]
fn test_u32_max() {
let gil = Python::acquire_gil();
Expand Down

0 comments on commit 349baa3

Please sign in to comment.