Skip to content

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

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.

Sign up for GitHub

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

Merged
YannickJadoul merged 8 commits into from
Jan 25, 2021
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2231d9c
Always call PyNumber_Index when casting from Python to a C++ integral…
YannickJadoul Jan 16, 2021
2eb35ac
Fixed on PyPy
YannickJadoul Jan 17, 2021
064d671
Simplify use of PyNumber_Index, following @rwgk's idea, and ignore wa…
YannickJadoul Jan 20, 2021
caa5382
Reproduce mismatch between pre-3.8 and post-3.8 behavior on __index__…
YannickJadoul Jan 22, 2021
cffc586
Fix tests on 3.6 <= Python < 3.8
YannickJadoul Jan 22, 2021
8356073
No, I don't have an uninitialized variable
YannickJadoul Jan 22, 2021
2092295
Fix use of __index__ on Python 2
YannickJadoul Jan 23, 2021
7fd2db5
Make types in test_int_convert more ~boring~ descriptive
YannickJadoul Jan 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions include/pybind11/cast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1042,12 +1042,26 @@ struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value && !is_std_char_t
return false;
} else if (!convert && !index_check(src.ptr()) && !PYBIND11_LONG_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 (index_check(src.ptr())) {
index = reinterpret_steal<object>(PyNumber_Index(src.ptr()));
if (!index) {
PyErr_Clear();
return false;
}
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

@YannickJadoul YannickJadoul Jan 23, 2021

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 Down
13 changes: 8 additions & 5 deletions tests/test_builtin_casters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ def test_integer_casting():
assert "incompatible function arguments" in str(excinfo.value)


@pytest.mark.filterwarnings("ignore:an integer is required:DeprecationWarning")
def test_int_convert():
class DeepThought(object):
def __int__(self):
Expand Down Expand Up @@ -289,13 +290,15 @@ def cant_convert(v):
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

# Before Python 3.8, `int(obj)` does not pick up on `obj.__index__`, but pybind11
# "backports" this behavior.
assert convert(IndexedThought()) == 42
assert noconvert(IndexedThought()) == 42
cant_convert(RaisingThought()) # no fall-back to `__int__`if `__index__` raises


@pytest.mark.filterwarnings("ignore:an integer is required:DeprecationWarning")
def test_numpy_int_convert():
np = pytest.importorskip("numpy")

Expand Down