Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always call PyNumber_Index when casting from Python to a C++ integral type, also pre-3.8 #2801

Merged
40 changes: 25 additions & 15 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -1040,14 +1040,31 @@ struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value && !is_std_char_t
return false;
} else if (PyFloat_Check(src.ptr())) {
return false;
} else if (!convert && !index_check(src.ptr()) && !PYBIND11_LONG_CHECK(src.ptr())) {
} else if (!convert && !PYBIND11_LONG_CHECK(src.ptr()) && !index_check(src.ptr())) {
return false;
} else if (std::is_unsigned<py_type>::value) {
py_value = as_unsigned<py_type>(src.ptr());
} else { // signed integer:
py_value = sizeof(T) <= sizeof(long)
? (py_type) PyLong_AsLong(src.ptr())
: (py_type) PYBIND11_LONG_AS_LONGLONG(src.ptr());
} else {
handle src_or_index = src;
#if PY_VERSION_HEX < 0x03080000
object index;
if (!PYBIND11_LONG_CHECK(src.ptr())) { // So: index_check(src.ptr())
index = reinterpret_steal<object>(PyNumber_Index(src.ptr()));
if (!index) {
PyErr_Clear();
if (!convert)
return false;
}
else {
src_or_index = index;
}
}
#endif
if (std::is_unsigned<py_type>::value) {
py_value = as_unsigned<py_type>(src_or_index.ptr());
} else { // signed integer:
py_value = sizeof(T) <= sizeof(long)
? (py_type) PyLong_AsLong(src_or_index.ptr())
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, another note: this results in warnings, and I think that's correct. Because it's not just getting out the long from the PyLong object, but it's also trying to do the conversion.

Yes, the C API is quite messy here. And it's only made worse by the structure of this caster.

: (py_type) PYBIND11_LONG_AS_LONGLONG(src_or_index.ptr());
}
}

// Python API reported an error
Expand All @@ -1056,15 +1073,8 @@ struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value && !is_std_char_t
// Check to see if the conversion is valid (integers should match exactly)
// Signed/unsigned checks happen elsewhere
if (py_err || (std::is_integral<T>::value && sizeof(py_type) != sizeof(T) && py_value != (py_type) (T) py_value)) {
bool type_error = py_err && PyErr_ExceptionMatches(
#if PY_VERSION_HEX < 0x03000000 && !defined(PYPY_VERSION)
PyExc_SystemError
#else
PyExc_TypeError
#endif
);
PyErr_Clear();
if (type_error && convert && PyNumber_Check(src.ptr())) {
if (py_err && convert && PyNumber_Check(src.ptr())) {
auto tmp = reinterpret_steal<object>(std::is_floating_point<T>::value
? PyNumber_Float(src.ptr())
: PyNumber_Long(src.ptr()));
Expand Down
50 changes: 35 additions & 15 deletions tests/test_builtin_casters.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,22 +252,36 @@ def test_integer_casting():


def test_int_convert():
class DeepThought(object):
class Int(object):
def __int__(self):
return 42

class ShallowThought(object):
class NotInt(object):
pass

class FuzzyThought(object):
class Float(object):
def __float__(self):
return 41.99999

class IndexedThought(object):
class Index(object):
def __index__(self):
return 42

class RaisingThought(object):
class IntAndIndex(object):
def __int__(self):
return 42

def __index__(self):
return 0

class RaisingTypeErrorOnIndex(object):
def __index__(self):
raise TypeError

def __int__(self):
return 42

class RaisingValueErrorOnIndex(object):
def __index__(self):
raise ValueError

Expand All @@ -276,7 +290,7 @@ def __int__(self):

convert, noconvert = m.int_passthrough, m.int_passthrough_noconvert

def require_implicit(v):
def requires_conversion(v):
pytest.raises(TypeError, noconvert, v)

def cant_convert(v):
Expand All @@ -285,15 +299,21 @@ def cant_convert(v):
assert convert(7) == 7
assert noconvert(7) == 7
cant_convert(3.14159)
assert convert(DeepThought()) == 42
require_implicit(DeepThought())
cant_convert(ShallowThought())
cant_convert(FuzzyThought())
if env.PY >= (3, 8):
# Before Python 3.8, `int(obj)` does not pick up on `obj.__index__`
assert convert(IndexedThought()) == 42
assert noconvert(IndexedThought()) == 42
cant_convert(RaisingThought()) # no fall-back to `__int__`if `__index__` raises
assert convert(Int()) == 42
requires_conversion(Int())
cant_convert(NotInt())
cant_convert(Float())

# Before Python 3.8, `PyLong_AsLong` does not pick up on `obj.__index__`,
# but pybind11 "backports" this behavior.
assert convert(Index()) == 42
assert noconvert(Index()) == 42
assert convert(IntAndIndex()) == 0 # Fishy; `int(DoubleThought)` == 42
assert noconvert(IntAndIndex()) == 0
assert convert(RaisingTypeErrorOnIndex()) == 42
requires_conversion(RaisingTypeErrorOnIndex())
assert convert(RaisingValueErrorOnIndex()) == 42
requires_conversion(RaisingValueErrorOnIndex())


def test_numpy_int_convert():
Expand Down