Improve native object initialization

newsfragments/4798.fixed.md

@@ -0,0 +1,2 @@
+fixes several limitations with base native type initialization. Required for extending native base types
+with the limited API and extending base native types that require arguments passed to `__new__`.

pyo3-macros-backend/src/method.rs

@@ -831,12 +831,14 @@ impl<'a> FnSpec<'a> {
                         _kwargs: *mut #pyo3_path::ffi::PyObject
                     ) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
                         use #pyo3_path::impl_::callback::IntoPyCallbackOutput;
+                        let raw_args = _args;
+                        let raw_kwargs = _kwargs;
                         let function = #rust_name; // Shadow the function name to avoid #3017
                         #arg_convert
                         #init_holders
                         let result = #call;
                         let initializer: #pyo3_path::PyClassInitializer::<#cls> = result.convert(py)?;
-                        #pyo3_path::impl_::pymethods::tp_new_impl(py, initializer, _slf)
+                        #pyo3_path::impl_::pymethods::tp_new_impl(py, initializer, _slf, raw_args, raw_kwargs)
                     }
                 }
             }

src/impl_/pyclass.rs

@@ -1164,6 +1164,8 @@ pub trait PyClassBaseType: Sized {
     type BaseNativeType;
     type Initializer: PyObjectInit<Self>;
     type PyClassMutability: PyClassMutability;
+    /// Use the provided function instead of [ffi::PyTypeObject::tp_new] when creating an object of this type
+    const OVERRIDE_TP_NEW: Option<ffi::newfunc> = None;
 }
 
 /// Implementation of tp_dealloc for pyclasses without gc

src/impl_/pyclass_init.rs

@@ -1,11 +1,13 @@
 //! Contains initialization utilities for `#[pyclass]`.
+use crate::exceptions::PyTypeError;
 use crate::ffi_ptr_ext::FfiPtrExt;
-use crate::internal::get_slot::TP_ALLOC;
-use crate::types::PyType;
-use crate::{ffi, Borrowed, PyErr, PyResult, Python};
+use crate::internal::get_slot::TP_NEW;
+use crate::types::{PyDict, PyTuple, PyType};
+use crate::{ffi, Bound, PyErr, PyResult, Python};
 use crate::{ffi::PyTypeObject, sealed::Sealed, type_object::PyTypeInfo};
 use std::marker::PhantomData;
-use std::ptr;
+
+use super::pyclass::PyClassBaseType;
 
 /// Initializer for Python types.
 ///
@@ -18,72 +20,76 @@ pub trait PyObjectInit<T>: Sized + Sealed {
         self,
         py: Python<'_>,
         subtype: *mut PyTypeObject,
+        args: &Bound<'_, PyTuple>,
+        kwargs: Option<&Bound<'_, PyDict>>,
     ) -> PyResult<*mut ffi::PyObject>;
 
     #[doc(hidden)]
     fn can_be_subclassed(&self) -> bool;
 }
 
-/// Initializer for Python native types, like `PyDict`.
-pub struct PyNativeTypeInitializer<T: PyTypeInfo>(pub PhantomData<T>);
+/// Initializer for Python native types, like [PyDict].
+pub struct PyNativeTypeInitializer<T: PyTypeInfo + PyClassBaseType>(pub PhantomData<T>);
 
-impl<T: PyTypeInfo> PyObjectInit<T> for PyNativeTypeInitializer<T> {
+impl<T: PyTypeInfo + PyClassBaseType> PyObjectInit<T> for PyNativeTypeInitializer<T> {
+    /// call `__new__` ([ffi::PyTypeObject::tp_new]) for the native base type.
+    /// This will allocate a new python object and initialize the part relating to the native base type.
     unsafe fn into_new_object(
         self,
         py: Python<'_>,
         subtype: *mut PyTypeObject,
+        args: &Bound<'_, PyTuple>,
+        kwargs: Option<&Bound<'_, PyDict>>,
     ) -> PyResult<*mut ffi::PyObject> {
-        unsafe fn inner(
+        fn inner(
             py: Python<'_>,
-            type_object: *mut PyTypeObject,
+            native_base_type: *mut PyTypeObject,
             subtype: *mut PyTypeObject,
+            args: &Bound<'_, PyTuple>,
+            kwargs: Option<&Bound<'_, PyDict>>,
+            override_tp_new: Option<ffi::newfunc>,
         ) -> PyResult<*mut ffi::PyObject> {
-            // HACK (due to FIXME below): PyBaseObject_Type's tp_new isn't happy with NULL arguments
-            let is_base_object = ptr::eq(type_object, ptr::addr_of!(ffi::PyBaseObject_Type));
-            let subtype_borrowed: Borrowed<'_, '_, PyType> = unsafe {
-                subtype
-                    .cast::<ffi::PyObject>()
-                    .assume_borrowed_unchecked(py)
-                    .downcast_unchecked()
+            let tp_new = if let Some(tp_new) = override_tp_new {
+                tp_new
+            } else {
+                unsafe {
+                    native_base_type
+                        .cast::<ffi::PyObject>()
+                        .assume_borrowed_unchecked(py)
+                        .downcast_unchecked::<PyType>()
+                        .get_slot(TP_NEW)
+                        .ok_or_else(|| {
+                            PyTypeError::new_err(
+                                "cannot construct type that does not define __new__",
+                            )
+                        })?
+                }
             };
 
-            if is_base_object {
-                let alloc = subtype_borrowed
-                    .get_slot(TP_ALLOC)
-                    .unwrap_or(ffi::PyType_GenericAlloc);
-
-                let obj = unsafe { alloc(subtype, 0) };
-                return if obj.is_null() {
-                    Err(PyErr::fetch(py))
-                } else {
-                    Ok(obj)
-                };
-            }
-
-            #[cfg(Py_LIMITED_API)]
-            unreachable!("subclassing native types is not possible with the `abi3` feature");
+            let obj = unsafe {
+                tp_new(
+                    subtype,
+                    args.as_ptr(),
+                    kwargs
+                        .map(|obj| obj.as_ptr())
+                        .unwrap_or(std::ptr::null_mut()),
+                )
+            };
 
-            #[cfg(not(Py_LIMITED_API))]
-            {
-                match unsafe { (*type_object).tp_new } {
-                    // FIXME: Call __new__ with actual arguments
-                    Some(newfunc) => {
-                        let obj =
-                            unsafe { newfunc(subtype, std::ptr::null_mut(), std::ptr::null_mut()) };
-                        if obj.is_null() {
-                            Err(PyErr::fetch(py))
-                        } else {
-                            Ok(obj)
-                        }
-                    }
-                    None => Err(crate::exceptions::PyTypeError::new_err(
-                        "base type without tp_new",
-                    )),
-                }
+            if obj.is_null() {
+                Err(PyErr::fetch(py))
+            } else {
+                Ok(obj)
             }
         }
-        let type_object = T::type_object_raw(py);
-        unsafe { inner(py, type_object, subtype) }
+        inner(
+            py,
+            T::type_object_raw(py),
+            subtype,
+            args,
+            kwargs,
+            T::OVERRIDE_TP_NEW,
+        )
     }
 
     #[inline]

src/impl_/pymethods.rs

@@ -8,10 +8,10 @@ use crate::pycell::impl_::PyClassBorrowChecker as _;
 use crate::pycell::{PyBorrowError, PyBorrowMutError};
 use crate::pyclass::boolean_struct::False;
 use crate::types::any::PyAnyMethods;
-use crate::types::PyType;
+use crate::types::{PyDict, PyTuple, PyType};
 use crate::{
-    ffi, Bound, DowncastError, Py, PyAny, PyClass, PyClassInitializer, PyErr, PyObject, PyRef,
-    PyRefMut, PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python,
+    ffi, Borrowed, Bound, DowncastError, Py, PyAny, PyClass, PyClassInitializer, PyErr, PyObject,
+    PyRef, PyRefMut, PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python,
 };
 use std::ffi::CStr;
 use std::fmt;
@@ -699,14 +699,25 @@ impl<'py, T> std::ops::Deref for BoundRef<'_, 'py, T> {
     }
 }
 
-pub unsafe fn tp_new_impl<T: PyClass>(
-    py: Python<'_>,
+pub unsafe fn tp_new_impl<'py, T: PyClass>(
+    py: Python<'py>,
     initializer: PyClassInitializer<T>,
-    target_type: *mut ffi::PyTypeObject,
+    most_derived_type: *mut ffi::PyTypeObject,
+    args: *mut ffi::PyObject,
+    kwargs: *mut ffi::PyObject,
 ) -> PyResult<*mut ffi::PyObject> {
+    // Safety:
+    //  - `args` is known to be a tuple
+    //  - `kwargs` is known to be a dict or null
+    //  - we both have the GIL and can borrow these input references for the `'py` lifetime.
     unsafe {
+        let args: Borrowed<'py, 'py, PyTuple> =
+            Borrowed::from_ptr(py, args).downcast_unchecked::<PyTuple>();
+        let kwargs: Option<Borrowed<'py, 'py, PyDict>> =
+            Borrowed::from_ptr_or_opt(py, kwargs).map(|kwargs| kwargs.downcast_unchecked());
+
         initializer
-            .create_class_object_of_type(py, target_type)
+            .create_class_object_of_type(py, most_derived_type, &args, kwargs.as_deref())
             .map(Bound::into_ptr)
     }
 }

src/internal/get_slot.rs

@@ -124,11 +124,11 @@ macro_rules! impl_slots {
 
 // Slots are implemented on-demand as needed.)
 impl_slots! {
-    TP_ALLOC: (Py_tp_alloc, tp_alloc) -> Option<ffi::allocfunc>,
     TP_BASE: (Py_tp_base, tp_base) -> *mut ffi::PyTypeObject,
     TP_CLEAR: (Py_tp_clear, tp_clear) -> Option<ffi::inquiry>,
     TP_DESCR_GET: (Py_tp_descr_get, tp_descr_get) -> Option<ffi::descrgetfunc>,
     TP_FREE: (Py_tp_free, tp_free) -> Option<ffi::freefunc>,
+    TP_NEW: (Py_tp_new, tp_new) -> Option<ffi::newfunc>,
     TP_TRAVERSE: (Py_tp_traverse, tp_traverse) -> Option<ffi::traverseproc>,
 }