Skip to content

Commit

Permalink
Improve native object initialization
Browse files Browse the repository at this point in the history
Before this change, classes inheriting the base object was a special
case where `object.__new__` is not called, and inheriting from other
base classes requires use of the unlimited API.

Previously this was not very limiting, but with <#4678>
it will be possible to inherit from native base classes with the
limited API and possibly from dynamically imported native base classes
which may require `__new__` arguments to reach them.
  • Loading branch information
mbway committed Dec 14, 2024
1 parent 603a55f commit 1e0745f
Show file tree
Hide file tree
Showing 9 changed files with 220 additions and 64 deletions.
2 changes: 2 additions & 0 deletions newsfragments/4797.fixed.md
Original file line number Diff line number Diff line change
@@ -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__`.
4 changes: 3 additions & 1 deletion pyo3-macros-backend/src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,12 +830,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)
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1138,6 +1138,9 @@ pub trait PyClassBaseType: Sized {
type BaseNativeType;
type Initializer: PyObjectInit<Self>;
type PyClassMutability: PyClassMutability;
/// Whether `__new__` ([ffi::PyTypeObject::tp_new]) for this type accepts arguments other
/// than the type of object to create.
const NEW_ACCEPTS_ARGUMENTS: bool = true;
}

/// Implementation of tp_dealloc for pyclasses without gc
Expand Down
89 changes: 46 additions & 43 deletions src/impl_/pyclass_init.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
//! Contains initialization utilities for `#[pyclass]`.
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, Borrowed, Bound, PyErr, PyResult, Python};
use crate::{ffi::PyTypeObject, sealed::Sealed, type_object::PyTypeInfo};
use std::marker::PhantomData;

use super::pyclass::PyClassBaseType;

/// Initializer for Python types.
///
/// This trait is intended to use internally for distinguishing `#[pyclass]` and
Expand All @@ -17,69 +19,70 @@ 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(
py: Python<'_>,
type_object: *mut PyTypeObject,
native_base_type: *mut PyTypeObject,
subtype: *mut PyTypeObject,
args: &Bound<'_, PyTuple>,
kwargs: Option<&Bound<'_, PyDict>>,
new_accepts_arguments: bool,
) -> PyResult<*mut ffi::PyObject> {
// HACK (due to FIXME below): PyBaseObject_Type's tp_new isn't happy with NULL arguments
let is_base_object = type_object == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type);
let subtype_borrowed: Borrowed<'_, '_, PyType> = subtype
let native_base_type_borrowed: Borrowed<'_, '_, PyType> = native_base_type
.cast::<ffi::PyObject>()
.assume_borrowed_unchecked(py)
.downcast_unchecked();
let tp_new = native_base_type_borrowed
.get_slot(TP_NEW)
.unwrap_or(ffi::PyType_GenericNew);

if is_base_object {
let alloc = subtype_borrowed
.get_slot(TP_ALLOC)
.unwrap_or(ffi::PyType_GenericAlloc);

let obj = 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 = if new_accepts_arguments {
tp_new(
subtype,
args.as_ptr(),
kwargs
.map(|obj| obj.as_ptr())
.unwrap_or(std::ptr::null_mut()),
)
} else {
let args = PyTuple::empty(py);
tp_new(subtype, args.as_ptr(), std::ptr::null_mut())
};

#[cfg(not(Py_LIMITED_API))]
{
match (*type_object).tp_new {
// FIXME: Call __new__ with actual arguments
Some(newfunc) => {
let obj = 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);
inner(py, type_object, subtype)
inner(
py,
T::type_object_raw(py),
subtype,
args,
kwargs,
T::NEW_ACCEPTS_ARGUMENTS,
)
}

#[inline]
Expand Down
25 changes: 18 additions & 7 deletions src/impl_/pymethods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -696,13 +696,24 @@ 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.
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)
}

Expand Down
2 changes: 1 addition & 1 deletion src/internal/get_slot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,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>,
}

Expand Down
Loading

0 comments on commit 1e0745f

Please sign in to comment.