From 2231d9c9ca044d5965d8595e01b251bba14a5e80 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Sat, 16 Jan 2021 23:53:04 +0100 Subject: [PATCH 1/8] Always call PyNumber_Index when casting from Python to a C++ integral type, also pre-3.8 --- include/pybind11/cast.h | 30 ++++++++++++++++++++++++------ tests/test_builtin_casters.py | 11 ++++++----- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index ce85d997b6..8106231a4a 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1042,12 +1042,30 @@ struct type_caster::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::value) { - py_value = as_unsigned(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 obj = src; +#if PY_VERSION_HEX < 0x03080000 + bool do_decref = false; + if (PyIndex_Check(src.ptr())) { + PyObject *tmp = PyNumber_Index(src.ptr()); + if (!tmp) { + py_value = (py_type) -1; + } + do_decref = true; + obj = tmp; + } +#endif + if (std::is_unsigned::value) { + py_value = as_unsigned(obj.ptr()); + } else { // signed integer: + py_value = sizeof(T) <= sizeof(long) + ? (py_type) PyLong_AsLong(obj.ptr()) + : (py_type) PYBIND11_LONG_AS_LONGLONG(obj.ptr()); + } +#if PY_VERSION_HEX < 0x03080000 + if (do_decref) + obj.dec_ref(); +#endif } // Python API reported an error diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 0a469842e0..8b9c1064e7 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -289,11 +289,12 @@ 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 def test_numpy_int_convert(): From 2eb35acdc89088e83f54dc7e0404fcc8db7c0112 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Sun, 17 Jan 2021 01:26:20 +0100 Subject: [PATCH 2/8] Fixed on PyPy --- include/pybind11/cast.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 8106231a4a..a8022c7367 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1046,10 +1046,11 @@ struct type_caster::value && !is_std_char_t handle obj = src; #if PY_VERSION_HEX < 0x03080000 bool do_decref = false; - if (PyIndex_Check(src.ptr())) { + if (index_check(src.ptr())) { PyObject *tmp = PyNumber_Index(src.ptr()); if (!tmp) { - py_value = (py_type) -1; + PyErr_Clear(); + return false; } do_decref = true; obj = tmp; From 064d671f97f651cf9f426c53486d3f823aa5cd15 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Wed, 20 Jan 2021 22:47:59 +0100 Subject: [PATCH 3/8] Simplify use of PyNumber_Index, following @rwgk's idea, and ignore warnings in >=3.8 --- include/pybind11/cast.h | 21 ++++++++------------- tests/test_builtin_casters.py | 2 ++ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index a8022c7367..6363be3b14 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1043,30 +1043,25 @@ struct type_caster::value && !is_std_char_t } else if (!convert && !index_check(src.ptr()) && !PYBIND11_LONG_CHECK(src.ptr())) { return false; } else { - handle obj = src; + handle src_or_index = src; #if PY_VERSION_HEX < 0x03080000 - bool do_decref = false; + object index; if (index_check(src.ptr())) { - PyObject *tmp = PyNumber_Index(src.ptr()); - if (!tmp) { + index = reinterpret_steal(PyNumber_Index(src.ptr())); + if (!index) { PyErr_Clear(); return false; } - do_decref = true; - obj = tmp; + src_or_index = index; } #endif if (std::is_unsigned::value) { - py_value = as_unsigned(obj.ptr()); + py_value = as_unsigned(src_or_index.ptr()); } else { // signed integer: py_value = sizeof(T) <= sizeof(long) - ? (py_type) PyLong_AsLong(obj.ptr()) - : (py_type) PYBIND11_LONG_AS_LONGLONG(obj.ptr()); + ? (py_type) PyLong_AsLong(src_or_index.ptr()) + : (py_type) PYBIND11_LONG_AS_LONGLONG(src_or_index.ptr()); } -#if PY_VERSION_HEX < 0x03080000 - if (do_decref) - obj.dec_ref(); -#endif } // Python API reported an error diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 8b9c1064e7..6cfa3a816a 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -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): @@ -297,6 +298,7 @@ def cant_convert(v): 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") From caa53820704eb640d41fdc866b936896b79f1c91 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Fri, 22 Jan 2021 19:24:55 +0100 Subject: [PATCH 4/8] Reproduce mismatch between pre-3.8 and post-3.8 behavior on __index__ throwing TypeError --- tests/test_builtin_casters.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 6cfa3a816a..cfa07f04b5 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -268,6 +268,13 @@ class IndexedThought(object): def __index__(self): return 42 + class TypeErrorThought(object): + def __index__(self): + raise TypeError + + def __int__(self): + return 42 + class RaisingThought(object): def __index__(self): raise ValueError @@ -295,6 +302,8 @@ def cant_convert(v): # "backports" this behavior. assert convert(IndexedThought()) == 42 assert noconvert(IndexedThought()) == 42 + assert convert(TypeErrorThought()) == 42 + require_implicit(TypeErrorThought()) cant_convert(RaisingThought()) # no fall-back to `__int__`if `__index__` raises From cffc586f59d5f08d2b2520e6f7de82b7146a6e1e Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Fri, 22 Jan 2021 19:44:42 +0100 Subject: [PATCH 5/8] Fix tests on 3.6 <= Python < 3.8 --- include/pybind11/cast.h | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 6363be3b14..223a1d7016 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1049,18 +1049,22 @@ struct type_caster::value && !is_std_char_t if (index_check(src.ptr())) { index = reinterpret_steal(PyNumber_Index(src.ptr())); if (!index) { - PyErr_Clear(); - return false; + src_or_index = handle(); + py_value = (py_type) -1; + } + else { + src_or_index = index; } - src_or_index = index; } #endif - if (std::is_unsigned::value) { - py_value = as_unsigned(src_or_index.ptr()); - } else { // signed integer: - py_value = sizeof(T) <= sizeof(long) - ? (py_type) PyLong_AsLong(src_or_index.ptr()) - : (py_type) PYBIND11_LONG_AS_LONGLONG(src_or_index.ptr()); + if (src_or_index) { + if (std::is_unsigned::value) { + py_value = as_unsigned(src_or_index.ptr()); + } else { // signed integer: + py_value = sizeof(T) <= sizeof(long) + ? (py_type) PyLong_AsLong(src_or_index.ptr()) + : (py_type) PYBIND11_LONG_AS_LONGLONG(src_or_index.ptr()); + } } } From 83560739b4872145dfbf28c650b5b19d32adb87b Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Fri, 22 Jan 2021 20:28:41 +0100 Subject: [PATCH 6/8] No, I don't have an uninitialized variable --- include/pybind11/cast.h | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 223a1d7016..6079283ffe 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1044,17 +1044,12 @@ struct type_caster::value && !is_std_char_t return false; } else { handle src_or_index = src; + py_value = (py_type) -1; #if PY_VERSION_HEX < 0x03080000 object index; - if (index_check(src.ptr())) { + if (!PYBIND11_LONG_CHECK(src.ptr()) && index_check(src.ptr())) { index = reinterpret_steal(PyNumber_Index(src.ptr())); - if (!index) { - src_or_index = handle(); - py_value = (py_type) -1; - } - else { - src_or_index = index; - } + src_or_index = index ? index : handle(); } #endif if (src_or_index) { From 2092295552848d3a6bbecdf73b9f14b29c321d0f Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Sat, 23 Jan 2021 18:44:41 +0100 Subject: [PATCH 7/8] Fix use of __index__ on Python 2 --- include/pybind11/cast.h | 37 ++++++++++++++++------------------- tests/test_builtin_casters.py | 24 +++++++++++++++-------- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 6079283ffe..64683f8e4b 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1040,26 +1040,30 @@ struct type_caster::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 { handle src_or_index = src; - py_value = (py_type) -1; #if PY_VERSION_HEX < 0x03080000 object index; - if (!PYBIND11_LONG_CHECK(src.ptr()) && index_check(src.ptr())) { + if (!PYBIND11_LONG_CHECK(src.ptr())) { // So: index_check(src.ptr()) index = reinterpret_steal(PyNumber_Index(src.ptr())); - src_or_index = index ? index : handle(); + if (!index) { + PyErr_Clear(); + if (!convert) + return false; + } + else { + src_or_index = index; + } } #endif - if (src_or_index) { - if (std::is_unsigned::value) { - py_value = as_unsigned(src_or_index.ptr()); - } else { // signed integer: - py_value = sizeof(T) <= sizeof(long) - ? (py_type) PyLong_AsLong(src_or_index.ptr()) - : (py_type) PYBIND11_LONG_AS_LONGLONG(src_or_index.ptr()); - } + if (std::is_unsigned::value) { + py_value = as_unsigned(src_or_index.ptr()); + } else { // signed integer: + py_value = sizeof(T) <= sizeof(long) + ? (py_type) PyLong_AsLong(src_or_index.ptr()) + : (py_type) PYBIND11_LONG_AS_LONGLONG(src_or_index.ptr()); } } @@ -1069,15 +1073,8 @@ struct type_caster::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::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(std::is_floating_point::value ? PyNumber_Float(src.ptr()) : PyNumber_Long(src.ptr())); diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index cfa07f04b5..6385346014 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -251,12 +251,18 @@ 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): return 42 + class DoubleThought(object): + def __int__(self): + return 42 + + def __index__(self): + return 0 + class ShallowThought(object): pass @@ -284,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): @@ -294,20 +300,22 @@ def cant_convert(v): assert noconvert(7) == 7 cant_convert(3.14159) assert convert(DeepThought()) == 42 - require_implicit(DeepThought()) + requires_conversion(DeepThought()) + assert convert(DoubleThought()) == 0 # Fishy; `int(DoubleThought)` == 42 + assert noconvert(DoubleThought()) == 0 cant_convert(ShallowThought()) cant_convert(FuzzyThought()) - # Before Python 3.8, `int(obj)` does not pick up on `obj.__index__`, but pybind11 - # "backports" this behavior. + # Before Python 3.8, `PyLong_AsLong` does not pick up on `obj.__index__`, + # but pybind11 "backports" this behavior. assert convert(IndexedThought()) == 42 assert noconvert(IndexedThought()) == 42 assert convert(TypeErrorThought()) == 42 - require_implicit(TypeErrorThought()) - cant_convert(RaisingThought()) # no fall-back to `__int__`if `__index__` raises + requires_conversion(TypeErrorThought()) + assert convert(RaisingThought()) == 42 + requires_conversion(RaisingThought()) -@pytest.mark.filterwarnings("ignore:an integer is required:DeprecationWarning") def test_numpy_int_convert(): np = pytest.importorskip("numpy") From 7fd2db5a1105aa073edc46f23e20b063ed7ec04f Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Mon, 25 Jan 2021 15:02:33 +0100 Subject: [PATCH 8/8] Make types in test_int_convert more ~boring~ descriptive --- tests/test_builtin_casters.py | 50 +++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 6385346014..cb37dbcb3e 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -252,36 +252,36 @@ def test_integer_casting(): def test_int_convert(): - class DeepThought(object): + class Int(object): def __int__(self): return 42 - class DoubleThought(object): - def __int__(self): - return 42 - - def __index__(self): - return 0 - - 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 TypeErrorThought(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 RaisingThought(object): + class RaisingValueErrorOnIndex(object): def __index__(self): raise ValueError @@ -299,21 +299,21 @@ def cant_convert(v): assert convert(7) == 7 assert noconvert(7) == 7 cant_convert(3.14159) - assert convert(DeepThought()) == 42 - requires_conversion(DeepThought()) - assert convert(DoubleThought()) == 0 # Fishy; `int(DoubleThought)` == 42 - assert noconvert(DoubleThought()) == 0 - cant_convert(ShallowThought()) - cant_convert(FuzzyThought()) + 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(IndexedThought()) == 42 - assert noconvert(IndexedThought()) == 42 - assert convert(TypeErrorThought()) == 42 - requires_conversion(TypeErrorThought()) - assert convert(RaisingThought()) == 42 - requires_conversion(RaisingThought()) + 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():