pymethods: __ipow__ always receive None on Python 3.7

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

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(),

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,

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::Modulo>) -> 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,56 +716,37 @@ 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<T>(
     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::<crate::PyCell<T>>(slf);
         let other = py.from_borrowed_ptr::<crate::PyAny>(other);
-        let modulo = py.from_borrowed_ptr::<crate::PyAny>(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);
         Ok::<_, PyErr>(slf)
     })
 }
 
-#[doc(hidden)]
-#[cfg(not(Py_3_8))]
-pub unsafe extern "C" fn ipow<T>(
-    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::<crate::PyCell<T>>(slf);
-        let other = py.from_borrowed_ptr::<crate::PyAny>(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__);

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;

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;

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<T> {
+        unsafe { py.from_borrowed_ptr::<PyAny>(self.0) }.extract()
+    }
+
+    #[cfg(not(Py_3_8))]
+    #[inline]
+    pub fn extract<'a, T: FromPyObject<'a>>(self, py: Python<'a>) -> PyResult<T> {
+        unsafe { py.from_borrowed_ptr::<PyAny>(ffi::Py_None()) }.extract()
+    }
+}

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<Self>, _modulo: Option<u8>) {}
     }
 
-    #[cfg(not(Py_3_8))]
-    #[pymethods]
-    impl RichComparisonToSelf {
-        fn __repr__(&self) -> &'static str {
-            "RC_Self"
-        }
-
-        fn __richcmp__(&self, other: PyRef<Self>, _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<Self>, _other: u8, _modulo: Option<u8>) -> PyRef<Self> {
-            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<Self>) {}
-        fn __isub__(&mut self, _other: PyRef<Self>) {}
-        fn __imul__(&mut self, _other: PyRef<Self>) {}
-        fn __imatmul__(&mut self, _other: PyRef<Self>) {}
-        fn __itruediv__(&mut self, _other: PyRef<Self>) {}
-        fn __ifloordiv__(&mut self, _other: PyRef<Self>) {}
-        fn __imod__(&mut self, _other: PyRef<Self>) {}
-        fn __ilshift__(&mut self, _other: PyRef<Self>) {}
-        fn __irshift__(&mut self, _other: PyRef<Self>) {}
-        fn __iand__(&mut self, _other: PyRef<Self>) {}
-        fn __ior__(&mut self, _other: PyRef<Self>) {}
-        fn __ixor__(&mut self, _other: PyRef<Self>) {}
-        fn __ipow__(&mut self, _other: PyRef<Self>) {}
-    }
-
     fn _test_binary_dunder(dunder: &str) {
         let gil = Python::acquire_gil();
         let py = gil.python();