From c1c25d86f8baa23feab8bd5c2a133762b1df311f Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 4 Jan 2022 22:58:33 +0000 Subject: [PATCH] pymethods: __ipow__ always receive None on Python 3.7 --- pyo3-macros-backend/Cargo.toml | 3 - pyo3-macros-backend/build.rs | 14 ---- pyo3-macros-backend/src/defs.rs | 6 -- pyo3-macros-backend/src/pymethod.rs | 22 ++++-- src/class/number.rs | 47 +++-------- src/ffi/mod.rs | 4 + src/impl_.rs | 2 + src/impl_/pymethods.rs | 33 ++++++++ tests/test_arithmetics.rs | 116 ---------------------------- tests/test_arithmetics_protos.rs | 104 ------------------------- 10 files changed, 63 insertions(+), 288 deletions(-) delete mode 100644 pyo3-macros-backend/build.rs create mode 100644 src/impl_/pymethods.rs diff --git a/pyo3-macros-backend/Cargo.toml b/pyo3-macros-backend/Cargo.toml index 8ed07db286c..5abd99afdd1 100644 --- a/pyo3-macros-backend/Cargo.toml +++ b/pyo3-macros-backend/Cargo.toml @@ -18,9 +18,6 @@ quote = { version = "1", default-features = false } proc-macro2 = { version = "1", default-features = false } pyo3-build-config = { path = "../pyo3-build-config", version = "0.15.1", features = ["resolve-config"] } -[build-dependencies] -pyo3-build-config = { path = "../pyo3-build-config", version = "0.15.1", features = ["resolve-config"] } - [dependencies.syn] version = "1" default-features = false diff --git a/pyo3-macros-backend/build.rs b/pyo3-macros-backend/build.rs deleted file mode 100644 index afcf26de3ca..00000000000 --- a/pyo3-macros-backend/build.rs +++ /dev/null @@ -1,14 +0,0 @@ -use pyo3_build_config::pyo3_build_script_impl::{errors::Result, resolve_interpreter_config}; - -fn configure_pyo3() -> Result<()> { - let interpreter_config = resolve_interpreter_config()?; - interpreter_config.emit_pyo3_cfgs(); - Ok(()) -} - -fn main() { - if let Err(e) = configure_pyo3() { - eprintln!("error: {}", e.report()); - std::process::exit(1) - } -} diff --git a/pyo3-macros-backend/src/defs.rs b/pyo3-macros-backend/src/defs.rs index 938a91c51f1..258029d63f7 100644 --- a/pyo3-macros-backend/src/defs.rs +++ b/pyo3-macros-backend/src/defs.rs @@ -420,15 +420,9 @@ pub const NUM: Proto = Proto { MethodProto::new("__imod__", "PyNumberIModProtocol") .args(&["Other"]) .has_self(), - // See https://bugs.python.org/issue36379 - #[cfg(Py_3_8)] MethodProto::new("__ipow__", "PyNumberIPowProtocol") .args(&["Other", "Modulo"]) .has_self(), - #[cfg(not(Py_3_8))] - MethodProto::new("__ipow__", "PyNumberIPowProtocol") - .args(&["Other"]) - .has_self(), MethodProto::new("__ilshift__", "PyNumberILShiftProtocol") .args(&["Other"]) .has_self(), diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 8c52369cdd8..dcc51e9f2b6 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -532,14 +532,8 @@ const __IMOD__: SlotDef = SlotDef::new("Py_nb_inplace_remainder", "binaryfunc") .arguments(&[Ty::Object]) .extract_error_mode(ExtractErrorMode::NotImplemented) .return_self(); -#[cfg(Py_3_8)] -const __IPOW__: SlotDef = SlotDef::new("Py_nb_inplace_power", "ternaryfunc") - .arguments(&[Ty::Object, Ty::Object]) - .extract_error_mode(ExtractErrorMode::NotImplemented) - .return_self(); -#[cfg(not(Py_3_8))] -const __IPOW__: SlotDef = SlotDef::new("Py_nb_inplace_power", "binaryfunc") - .arguments(&[Ty::Object]) +const __IPOW__: SlotDef = SlotDef::new("Py_nb_inplace_power", "ipowfunc") + .arguments(&[Ty::Object, Ty::IPowModulo]) .extract_error_mode(ExtractErrorMode::NotImplemented) .return_self(); const __ILSHIFT__: SlotDef = SlotDef::new("Py_nb_inplace_lshift", "binaryfunc") @@ -609,6 +603,7 @@ enum Ty { Object, MaybeNullObject, NonNullObject, + IPowModulo, CompareOp, Int, PyHashT, @@ -621,6 +616,7 @@ impl Ty { match self { Ty::Object | Ty::MaybeNullObject => quote! { *mut _pyo3::ffi::PyObject }, Ty::NonNullObject => quote! { ::std::ptr::NonNull<_pyo3::ffi::PyObject> }, + Ty::IPowModulo => quote! { _pyo3::impl_::pymethods::IPowModulo }, Ty::Int | Ty::CompareOp => quote! { ::std::os::raw::c_int }, Ty::PyHashT => quote! { _pyo3::ffi::Py_hash_t }, Ty::PySsizeT => quote! { _pyo3::ffi::Py_ssize_t }, @@ -673,6 +669,16 @@ impl Ty { ); extract_object(cls, arg.ty, ident, extract) } + Ty::IPowModulo => { + let extract = handle_error( + extract_error_mode, + py, + quote! { + #ident.extract(#py) + }, + ); + extract_object(cls, arg.ty, ident, extract) + } Ty::CompareOp => { let extract = handle_error( extract_error_mode, diff --git a/src/class/number.rs b/src/class/number.rs index 2dbe5403793..b320f0c1bcd 100644 --- a/src/class/number.rs +++ b/src/class/number.rs @@ -221,20 +221,12 @@ pub trait PyNumberProtocol<'p>: PyClass { { unimplemented!() } - #[cfg(Py_3_8)] fn __ipow__(&'p mut self, other: Self::Other, modulo: Option) -> Self::Result where Self: PyNumberIPowProtocol<'p>, { unimplemented!() } - #[cfg(not(Py_3_8))] - fn __ipow__(&'p mut self, other: Self::Other) -> Self::Result - where - Self: PyNumberIPowProtocol<'p>, - { - unimplemented!() - } fn __ilshift__(&'p mut self, other: Self::Other) -> Self::Result where Self: PyNumberILShiftProtocol<'p>, @@ -512,7 +504,7 @@ pub trait PyNumberIDivmodProtocol<'p>: PyNumberProtocol<'p> { pub trait PyNumberIPowProtocol<'p>: PyNumberProtocol<'p> { type Other: FromPyObject<'p>; type Result: IntoPyCallbackOutput<()>; - #[cfg(Py_3_8)] + // See https://bugs.python.org/issue36379 type Modulo: FromPyObject<'p>; } @@ -724,28 +716,30 @@ py_binary_self_func!(isub, PyNumberISubProtocol, T::__isub__); py_binary_self_func!(imul, PyNumberIMulProtocol, T::__imul__); py_binary_self_func!(imod, PyNumberIModProtocol, T::__imod__); -// See https://bugs.python.org/issue36379 #[doc(hidden)] -#[cfg(Py_3_8)] pub unsafe extern "C" fn ipow( slf: *mut ffi::PyObject, other: *mut ffi::PyObject, - modulo: *mut ffi::PyObject, + modulo: crate::impl_::pymethods::IPowModulo, ) -> *mut ffi::PyObject where T: for<'p> PyNumberIPowProtocol<'p>, { - // NOTE: Somehow __ipow__ causes SIGSEGV in Python < 3.8 when we extract, - // so we ignore it. It's the same as what CPython does. crate::callback_body!(py, { let slf_cell = py.from_borrowed_ptr::>(slf); let other = py.from_borrowed_ptr::(other); - let modulo = py.from_borrowed_ptr::(modulo); slf_cell .try_borrow_mut()? .__ipow__( extract_or_return_not_implemented!(other), - extract_or_return_not_implemented!(modulo), + match modulo.extract(py) { + Ok(value) => value, + Err(_) => { + let res = crate::ffi::Py_NotImplemented(); + crate::ffi::Py_INCREF(res); + return Ok(res); + } + }, ) .convert(py)?; ffi::Py_INCREF(slf); @@ -753,27 +747,6 @@ where }) } -#[doc(hidden)] -#[cfg(not(Py_3_8))] -pub unsafe extern "C" fn ipow( - slf: *mut ffi::PyObject, - other: *mut ffi::PyObject, -) -> *mut ffi::PyObject -where - T: for<'p> PyNumberIPowProtocol<'p>, -{ - crate::callback_body!(py, { - let slf_cell = py.from_borrowed_ptr::>(slf); - let other = py.from_borrowed_ptr::(other); - slf_cell - .try_borrow_mut()? - .__ipow__(extract_or_return_not_implemented!(other)) - .convert(py)?; - ffi::Py_INCREF(slf); - Ok::<_, PyErr>(slf) - }) -} - py_binary_self_func!(ilshift, PyNumberILShiftProtocol, T::__ilshift__); py_binary_self_func!(irshift, PyNumberIRShiftProtocol, T::__irshift__); py_binary_self_func!(iand, PyNumberIAndProtocol, T::__iand__); diff --git a/src/ffi/mod.rs b/src/ffi/mod.rs index 428c71c6694..8d34e3d9a0f 100644 --- a/src/ffi/mod.rs +++ b/src/ffi/mod.rs @@ -204,3 +204,7 @@ mod cpython; #[cfg(not(Py_LIMITED_API))] pub use self::cpython::*; + +/// Helper to enable #[pymethods] to see the workaround for __ipow__ on Python 3.7 +#[doc(hidden)] +pub use crate::impl_::pymethods::ipowfunc; diff --git a/src/impl_.rs b/src/impl_.rs index d591886f901..3a35a8788b8 100644 --- a/src/impl_.rs +++ b/src/impl_.rs @@ -12,4 +12,6 @@ pub mod frompyobject; pub(crate) mod not_send; #[doc(hidden)] pub mod pyclass; +#[doc(hidden)] +pub mod pymethods; pub mod pymodule; diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs new file mode 100644 index 00000000000..f0153ce14ed --- /dev/null +++ b/src/impl_/pymethods.rs @@ -0,0 +1,33 @@ +use crate::{Python, FromPyObject, PyResult, ffi, PyAny}; + +/// Python 3.8 and up - __ipow__ has modulo argument correctly populated. +#[cfg(Py_3_8)] +#[repr(transparent)] +pub struct IPowModulo(*mut ffi::PyObject); + +/// Python 3.7 and older - __ipow__ does not have modulo argument correctly populated. +#[cfg(not(Py_3_8))] +#[repr(transparent)] +pub struct IPowModulo(std::mem::MaybeUninit<*mut ffi::PyObject>); + +/// Helper to use as pymethod ffi definition +#[allow(non_camel_case_types)] +pub type ipowfunc = unsafe extern "C" fn( + arg1: *mut ffi::PyObject, + arg2: *mut ffi::PyObject, + arg3: IPowModulo, +) -> *mut ffi::PyObject; + +impl IPowModulo { + #[cfg(Py_3_8)] + #[inline] + pub fn extract<'a, T: FromPyObject<'a>>(self, py: Python<'a>) -> PyResult { + unsafe { py.from_borrowed_ptr::(self.0) }.extract() + } + + #[cfg(not(Py_3_8))] + #[inline] + pub fn extract<'a, T: FromPyObject<'a>>(self, py: Python<'a>) -> PyResult { + unsafe { py.from_borrowed_ptr::(ffi::Py_None()) }.extract() + } +} diff --git a/tests/test_arithmetics.rs b/tests/test_arithmetics.rs index 7f317407a64..ed1870bbcbe 100644 --- a/tests/test_arithmetics.rs +++ b/tests/test_arithmetics.rs @@ -57,7 +57,6 @@ struct InPlaceOperations { value: u32, } -#[cfg(Py_3_8)] #[pymethods] impl InPlaceOperations { fn __repr__(&self) -> String { @@ -101,50 +100,6 @@ impl InPlaceOperations { } } -#[cfg(not(Py_3_8))] -#[pymethods] -impl InPlaceOperations { - fn __repr__(&self) -> String { - format!("IPO({:?})", self.value) - } - - fn __iadd__(&mut self, other: u32) { - self.value += other; - } - - fn __isub__(&mut self, other: u32) { - self.value -= other; - } - - fn __imul__(&mut self, other: u32) { - self.value *= other; - } - - fn __ilshift__(&mut self, other: u32) { - self.value <<= other; - } - - fn __irshift__(&mut self, other: u32) { - self.value >>= other; - } - - fn __iand__(&mut self, other: u32) { - self.value &= other; - } - - fn __ixor__(&mut self, other: u32) { - self.value ^= other; - } - - fn __ior__(&mut self, other: u32) { - self.value |= other; - } - - fn __ipow__(&mut self, other: u32) { - self.value = self.value.pow(other); - } -} - #[test] fn inplace_operations() { let gil = Python::acquire_gil(); @@ -550,7 +505,6 @@ mod return_not_implemented { #[pyclass] struct RichComparisonToSelf {} - #[cfg(Py_3_8)] #[pymethods] impl RichComparisonToSelf { fn __repr__(&self) -> &'static str { @@ -620,76 +574,6 @@ mod return_not_implemented { fn __ipow__(&mut self, _other: PyRef, _modulo: Option) {} } - #[cfg(not(Py_3_8))] - #[pymethods] - impl RichComparisonToSelf { - fn __repr__(&self) -> &'static str { - "RC_Self" - } - - fn __richcmp__(&self, other: PyRef, _op: CompareOp) -> PyObject { - other.py().None() - } - - fn __add__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __sub__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __mul__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __matmul__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __truediv__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __floordiv__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __mod__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __pow__(slf: PyRef, _other: u8, _modulo: Option) -> PyRef { - slf - } - fn __lshift__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __rshift__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __divmod__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __and__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __or__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - fn __xor__<'p>(slf: PyRef<'p, Self>, _other: PyRef<'p, Self>) -> PyRef<'p, Self> { - slf - } - - // Inplace assignments - fn __iadd__(&mut self, _other: PyRef) {} - fn __isub__(&mut self, _other: PyRef) {} - fn __imul__(&mut self, _other: PyRef) {} - fn __imatmul__(&mut self, _other: PyRef) {} - fn __itruediv__(&mut self, _other: PyRef) {} - fn __ifloordiv__(&mut self, _other: PyRef) {} - fn __imod__(&mut self, _other: PyRef) {} - fn __ilshift__(&mut self, _other: PyRef) {} - fn __irshift__(&mut self, _other: PyRef) {} - fn __iand__(&mut self, _other: PyRef) {} - fn __ior__(&mut self, _other: PyRef) {} - fn __ixor__(&mut self, _other: PyRef) {} - fn __ipow__(&mut self, _other: PyRef) {} - } - fn _test_binary_dunder(dunder: &str) { let gil = Python::acquire_gil(); let py = gil.python(); diff --git a/tests/test_arithmetics_protos.rs b/tests/test_arithmetics_protos.rs index 02ff0173516..86b30c1385e 100644 --- a/tests/test_arithmetics_protos.rs +++ b/tests/test_arithmetics_protos.rs @@ -74,47 +74,6 @@ impl PyObjectProtocol for InPlaceOperations { } } -#[cfg(not(Py_3_8))] -#[pyproto] -impl PyNumberProtocol for InPlaceOperations { - fn __iadd__(&mut self, other: u32) { - self.value += other; - } - - fn __isub__(&mut self, other: u32) { - self.value -= other; - } - - fn __imul__(&mut self, other: u32) { - self.value *= other; - } - - fn __ilshift__(&mut self, other: u32) { - self.value <<= other; - } - - fn __irshift__(&mut self, other: u32) { - self.value >>= other; - } - - fn __iand__(&mut self, other: u32) { - self.value &= other; - } - - fn __ixor__(&mut self, other: u32) { - self.value ^= other; - } - - fn __ior__(&mut self, other: u32) { - self.value |= other; - } - - fn __ipow__(&mut self, other: u32) { - self.value = self.value.pow(other); - } -} - -#[cfg(Py_3_8)] #[pyproto] impl PyNumberProtocol for InPlaceOperations { fn __iadd__(&mut self, other: u32) { @@ -576,7 +535,6 @@ mod return_not_implemented { } } - #[cfg(Py_3_8)] #[pyproto] impl<'p> PyNumberProtocol<'p> for RichComparisonToSelf { fn __add__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { @@ -638,68 +596,6 @@ mod return_not_implemented { fn __ixor__(&'p mut self, _other: PyRef<'p, Self>) {} } - #[cfg(not(Py_3_8))] - #[pyproto] - impl<'p> PyNumberProtocol<'p> for RichComparisonToSelf { - fn __add__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __sub__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __mul__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __matmul__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __truediv__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __floordiv__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __mod__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __pow__(lhs: &'p PyAny, _other: u8, _modulo: Option) -> &'p PyAny { - lhs - } - fn __lshift__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __rshift__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __divmod__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __and__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __or__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - fn __xor__(lhs: &'p PyAny, _other: PyRef<'p, Self>) -> &'p PyAny { - lhs - } - - // Inplace assignments - fn __iadd__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __isub__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __imul__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __imatmul__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __itruediv__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __ifloordiv__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __imod__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __ipow__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __ilshift__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __irshift__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __iand__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __ior__(&'p mut self, _other: PyRef<'p, Self>) {} - fn __ixor__(&'p mut self, _other: PyRef<'p, Self>) {} - } - fn _test_binary_dunder(dunder: &str) { let gil = Python::acquire_gil(); let py = gil.python();