Generated extension functions are weakly typed

[Vlad-Shcherbina]

Consider the README example:

[pyfn(m, "sum_as_string")]
fn sum_as_string_py(a:i64, b:i64) -> PyResult<String>

Here is what happens when this function is called with non-int arguments:

>>> from rust2py import sum_as_string
>>> sum_as_string(1, 2)  # good
'3'

>>> sum_as_string(10**100, 2)  # good
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: int too big to convert

>>> sum_as_string([], [])  # good
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: int() argument must be a string, a bytes-like object or a number, not 'list'

>>> sum_as_string('1', 2)  # bad
'3'

>>> sum_as_string(1.9, 2.9)  # bad
'3'

Generally Python functions that expect int argument raise TypeError when fed strings or floats. This helps to catch some programming errors.

>>> range('42')  # good
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'str' object cannot be interpreted as an integer

>>> range(42.9)  # good
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'float' object cannot be interpreted as an integer

I think it would be more consistent if generated extension functions behaved the same way.

[fafhrd91]

thanks! this is definitely a bug.

[Vlad-Shcherbina]

So it seems relevant that I'm running this example on Windows (target_pointer_width="64", target_os="windows"), and that the argument type is i64 and not, say, i32.

The problem appears to be here:

pyo3/src/objects/num3.rs

Lines 121 to 130 in 63a2066

	if ffi::PyLong_Check(ptr) != 0 {
	err_if_invalid_value(ob.py(), !0, $pylong_as_ull_or_ull(ptr))
	} else {
	let num = ffi::PyNumber_Long(ptr);
	if num.is_null() {
	Err(PyErr::fetch(ob.py()))
	} else {
	err_if_invalid_value(ob.py(), !0, $pylong_as_ull_or_ull(num))
	}
	}

PyLong_Check(x) is roughly equivalent to isinstance(x, int), and PyNumber_Long(x) is analagous to int(x), hence the frivolous conversion.

This code was there since it was written in dgrunwald/rust-cpython@55ea6cc#diff-2eab662fc2799e2bdab71b4844e11090R155

I wonder if this whole else branch should just return TypeError instead.

[fafhrd91]

but ffi::PyLong_Check(ptr) should fail for string, I guess

[Vlad-Shcherbina]

PyLong_Check() returns 0 on a string, and then PyNumber_Long() attempts to parse it.

[fafhrd91]

Unexpected :)

[Vlad-Shcherbina]

Found another similar instance of weak-typedness that happens for a different reason:

#[pyfn(m, "take_i32")]
fn take_i32(a: i32) -> PyResult<()> {
    println!("my arg is {}", a);
    Ok(())
}

>>> take_i32(42)  # good
my arg is 42

>>> take_i32('42')  # good
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: an integer is required (got type str)

>>> take_i32(42.9)  # bad
my arg is 42

This happens because in

pyo3/src/objects/num3.rs

Lines 48 to 57 in 63a2066

	pyobject_extract!(obj to $rust_type => {
	let val = unsafe { ffi::PyLong_AsLong(obj.as_ptr()) };
	if val == -1 && PyErr::occurred(obj.py()) {
	return Err(PyErr::fetch(obj.py()));
	}
	match cast::<c_long, $rust_type>(val) {
	Some(v) => Ok(v),
	None => Err(exc::OverflowError.into())
	}
	});

, the call to PyLong_AsLong() is equivalent to calling __int__() method on an object. Strings don't have it but floats do.

[fafhrd91]

@Vlad-Shcherbina i think this should be fixed now?

[Vlad-Shcherbina]

Yes, it's fixed for Python 3.