diff --git a/guide/src/migration.md b/guide/src/migration.md index 70463842334..d5494bc08c8 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -59,6 +59,35 @@ let _: Bound<'_, PyNone> = unsafe { Bound::from_owned_ptr(py, raw_ptr).cast_into # }) ``` +### Removal of `From` and `From> for PyClassInitializer` + +As part of refactoring the initialization code these impls were removed and its functionality was moved into the generated code for `#[new]`. +As a small side side effect the following pattern will not be accepted anymore: + +```rust,ignore +# use pyo3::prelude::*; +# Python::attach(|py| { +# let existing_py: Py = py.None(); +let obj_1 = Py::new(py, existing_py); + +# let existing_bound: Bound<'_, PyAny> = py.None().into_bound(py); +let obj_2 = Bound::new(py, existing_bound); +# }) +``` + +To migrate use `clone` or `clone_ref`: + +```rust +# use pyo3::prelude::*; +# Python::attach(|py| { +# let existing_py: Py = py.None(); +let obj_1 = existing_py.clone_ref(py); + +# let existing_bound: Bound<'_, PyAny> = py.None().into_bound(py); +let obj_2 = existing_bound.clone(); +# }) +``` + ### Internal change to use multi-phase initialization [PEP 489](https://peps.python.org/pep-0489/) introduced "multi-phase initialization" for extension modules which provides ways to allocate and clean up per-module state. diff --git a/newsfragments/5739.changed.md b/newsfragments/5739.changed.md new file mode 100644 index 00000000000..b8ff3b33578 --- /dev/null +++ b/newsfragments/5739.changed.md @@ -0,0 +1 @@ +`#[new]` can now return arbitrary Python objects \ No newline at end of file diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index ca0deba089a..b9f208a9e89 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -427,7 +427,6 @@ pub struct FnSpec<'a> { pub asyncness: Option, pub unsafety: Option, pub warnings: Vec, - #[cfg(feature = "experimental-inspect")] pub output: syn::ReturnType, } @@ -503,7 +502,6 @@ impl<'a> FnSpec<'a> { asyncness: sig.asyncness, unsafety: sig.unsafety, warnings, - #[cfg(feature = "experimental-inspect")] output: sig.output.clone(), }) } diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 08cc044d293..6cffc3ac532 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1817,7 +1817,6 @@ fn complex_enum_struct_variant_new<'a>( asyncness: None, unsafety: None, warnings: vec![], - #[cfg(feature = "experimental-inspect")] output: syn::ReturnType::Default, }; @@ -1875,7 +1874,6 @@ fn complex_enum_tuple_variant_new<'a>( asyncness: None, unsafety: None, warnings: vec![], - #[cfg(feature = "experimental-inspect")] output: syn::ReturnType::Default, }; @@ -1903,7 +1901,6 @@ fn complex_enum_variant_field_getter<'a>( asyncness: None, unsafety: None, warnings: vec![], - #[cfg(feature = "experimental-inspect")] output: parse_quote!(-> #variant_cls_type), }; diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs index a860837d0ec..55be364363b 100644 --- a/pyo3-macros-backend/src/pyfunction.rs +++ b/pyo3-macros-backend/src/pyfunction.rs @@ -415,7 +415,6 @@ pub fn impl_wrap_pyfunction( asyncness: func.sig.asyncness, unsafety: func.sig.unsafety, warnings, - #[cfg(feature = "experimental-inspect")] output: func.sig.output.clone(), }; diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index b460c5f8bcd..8f5f639adc4 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -16,8 +16,8 @@ use crate::{ use crate::{quotes, utils}; use proc_macro2::{Span, TokenStream}; use quote::{format_ident, quote, quote_spanned, ToTokens}; -use syn::LitCStr; use syn::{ext::IdentExt, spanned::Spanned, Field, Ident, Result}; +use syn::{parse_quote, LitCStr}; /// Generated code for a single pymethod item. pub struct MethodAndMethodDef { @@ -1460,15 +1460,23 @@ fn generate_method_body( } }); + let output = if let syn::ReturnType::Type(_, ty) = &spec.output { + ty + } else { + &parse_quote!(()) + }; let body = quote! { #text_signature_impl - use #pyo3_path::impl_::callback::IntoPyCallbackOutput; + use #pyo3_path::impl_::pyclass::Probe as _; #warnings #arg_convert 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::< + _, + { #pyo3_path::impl_::pyclass::IsPyClass::<#output>::VALUE }, + { #pyo3_path::impl_::pyclass::IsInitializerTuple::<#output>::VALUE } + >(py, result, _slf) }; (arg_idents, arg_types, body) } diff --git a/src/impl_/pyclass/probes.rs b/src/impl_/pyclass/probes.rs index 5de0c6a22cb..a16fc2f689c 100644 --- a/src/impl_/pyclass/probes.rs +++ b/src/impl_/pyclass/probes.rs @@ -1,6 +1,9 @@ use std::marker::PhantomData; -use crate::{conversion::IntoPyObject, FromPyObject, Py}; +use crate::conversion::IntoPyObject; +use crate::impl_::pyclass::PyClassBaseType; +use crate::impl_::pyclass_init::PyNativeTypeInitializer; +use crate::{FromPyObject, Py, PyClass, PyClassInitializer}; /// Trait used to combine with zero-sized types to calculate at compile time /// some property of a type. @@ -89,6 +92,39 @@ impl IsReturningEmptyTuple> { pub const VALUE: bool = true; } +probe!(IsPyClass); +impl IsPyClass +where + T: PyClass, +{ + pub const VALUE: bool = true; +} + +impl IsPyClass> +where + T: PyClass, +{ + pub const VALUE: bool = true; +} + +probe!(IsInitializerTuple); +impl IsInitializerTuple<(S, B)> +where + S: PyClass, + B: PyClass + PyClassBaseType>, + B::BaseType: PyClassBaseType>, +{ + pub const VALUE: bool = true; +} +impl IsInitializerTuple> +where + S: PyClass, + B: PyClass + PyClassBaseType>, + B::BaseType: PyClassBaseType>, +{ + pub const VALUE: bool = true; +} + #[cfg(test)] macro_rules! value_of { ($probe:ident, $ty:ty) => {{ diff --git a/src/impl_/pyclass_init.rs b/src/impl_/pyclass_init.rs index 5e4903d1b2d..a3c9607a83d 100644 --- a/src/impl_/pyclass_init.rs +++ b/src/impl_/pyclass_init.rs @@ -1,9 +1,10 @@ //! Contains initialization utilities for `#[pyclass]`. use crate::exceptions::PyTypeError; use crate::ffi_ptr_ext::FfiPtrExt; +use crate::impl_::pyclass::PyClassBaseType; use crate::internal::get_slot::TP_NEW; use crate::types::{PyTuple, PyType}; -use crate::{ffi, PyErr, PyResult, Python}; +use crate::{ffi, PyClass, PyClassInitializer, PyErr, PyResult, Python}; use crate::{ffi::PyTypeObject, sealed::Sealed, type_object::PyTypeInfo}; use std::marker::PhantomData; @@ -19,9 +20,6 @@ pub trait PyObjectInit: Sized + Sealed { py: Python<'_>, subtype: *mut PyTypeObject, ) -> PyResult<*mut ffi::PyObject>; - - #[doc(hidden)] - fn can_be_subclassed(&self) -> bool; } /// Initializer for Python native types, like `PyDict`. @@ -57,9 +55,84 @@ impl PyObjectInit for PyNativeTypeInitializer { } unsafe { inner(py, T::type_object_raw(py), subtype) } } +} + +pub trait PyClassInit<'py, const IS_PYCLASS: bool, const IS_INITIALIZER_TUPLE: bool> { + fn init( + self, + cls: crate::Borrowed<'_, 'py, crate::types::PyType>, + ) -> PyResult>; +} + +impl<'py, T> PyClassInit<'py, false, false> for T +where + T: crate::IntoPyObject<'py>, +{ + fn init( + self, + cls: crate::Borrowed<'_, 'py, crate::types::PyType>, + ) -> PyResult> { + self.into_pyobject(cls.py()) + .map(crate::BoundObject::into_any) + .map(crate::BoundObject::into_bound) + .map_err(Into::into) + } +} - #[inline] - fn can_be_subclassed(&self) -> bool { - true +impl<'py, T> PyClassInit<'py, true, false> for T +where + T: crate::PyClass, + T::BaseType: + super::pyclass::PyClassBaseType>, +{ + fn init( + self, + cls: crate::Borrowed<'_, 'py, crate::types::PyType>, + ) -> PyResult> { + PyClassInitializer::from(self).init(cls) + } +} + +impl<'py, T, E, const IS_PYCLASS: bool, const IS_INITIALIZER_TUPLE: bool> + PyClassInit<'py, IS_PYCLASS, IS_INITIALIZER_TUPLE> for Result +where + T: PyClassInit<'py, IS_PYCLASS, IS_INITIALIZER_TUPLE>, + E: Into, +{ + fn init( + self, + cls: crate::Borrowed<'_, 'py, crate::types::PyType>, + ) -> PyResult> { + self.map_err(Into::into)?.init(cls) + } +} + +impl<'py, T> PyClassInit<'py, false, false> for PyClassInitializer +where + T: PyClass, +{ + fn init( + self, + cls: crate::Borrowed<'_, 'py, crate::types::PyType>, + ) -> PyResult> { + unsafe { + self.create_class_object_of_type(cls.py(), cls.as_ptr().cast()) + .map(crate::Bound::into_any) + } + } +} + +impl<'py, S, B> PyClassInit<'py, false, true> for (S, B) +where + S: PyClass, + B: PyClass + PyClassBaseType>, + B::BaseType: PyClassBaseType>, +{ + fn init( + self, + cls: crate::Borrowed<'_, 'py, crate::types::PyType>, + ) -> PyResult> { + let (sub, base) = self; + PyClassInitializer::from(base).add_subclass(sub).init(cls) } } diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index 233bb88f857..b4a46b5cc41 100644 --- a/src/impl_/pymethods.rs +++ b/src/impl_/pymethods.rs @@ -9,8 +9,8 @@ use crate::pycell::{PyBorrowError, PyBorrowMutError}; use crate::pyclass::boolean_struct::False; use crate::types::PyType; use crate::{ - ffi, Bound, CastError, Py, PyAny, PyClass, PyClassGuard, PyClassGuardMut, PyClassInitializer, - PyErr, PyRef, PyRefMut, PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python, + ffi, Bound, CastError, Py, PyAny, PyClass, PyClassGuard, PyClassGuardMut, PyErr, PyRef, + PyRefMut, PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python, }; use std::ffi::CStr; use std::ffi::{c_int, c_void}; @@ -724,14 +724,16 @@ impl<'py, T> std::ops::Deref for BoundRef<'_, 'py, T> { } } -pub unsafe fn tp_new_impl( - py: Python<'_>, - initializer: PyClassInitializer, - target_type: *mut ffi::PyTypeObject, -) -> PyResult<*mut ffi::PyObject> { +pub unsafe fn tp_new_impl<'py, T, const IS_PYCLASS: bool, const IS_INITIALIZER_TUPLE: bool>( + py: Python<'py>, + obj: T, + cls: *mut ffi::PyTypeObject, +) -> PyResult<*mut ffi::PyObject> +where + T: super::pyclass_init::PyClassInit<'py, IS_PYCLASS, IS_INITIALIZER_TUPLE>, +{ unsafe { - initializer - .create_class_object_of_type(py, target_type) + obj.init(crate::Borrowed::from_ptr_unchecked(py, cls.cast()).cast_unchecked()) .map(Bound::into_ptr) } } diff --git a/src/pyclass_init.rs b/src/pyclass_init.rs index 121c070a694..9ac1e9aa261 100644 --- a/src/pyclass_init.rs +++ b/src/pyclass_init.rs @@ -1,10 +1,9 @@ //! Contains initialization utilities for `#[pyclass]`. use crate::ffi_ptr_ext::FfiPtrExt; -use crate::impl_::callback::IntoPyCallbackOutput; use crate::impl_::pyclass::{PyClassBaseType, PyClassImpl}; use crate::impl_::pyclass_init::{PyNativeTypeInitializer, PyObjectInit}; use crate::pycell::impl_::PyClassObjectLayout; -use crate::{ffi, Bound, Py, PyClass, PyResult, Python}; +use crate::{ffi, Bound, PyClass, PyResult, Python}; use crate::{ffi::PyTypeObject, pycell::impl_::PyClassObjectContents}; use std::marker::PhantomData; @@ -57,14 +56,9 @@ use std::marker::PhantomData; /// ); /// }); /// ``` -pub struct PyClassInitializer(PyClassInitializerImpl); - -enum PyClassInitializerImpl { - Existing(Py), - New { - init: T, - super_init: ::Initializer, - }, +pub struct PyClassInitializer { + init: T, + super_init: ::Initializer, } impl PyClassInitializer { @@ -74,12 +68,7 @@ impl PyClassInitializer { #[track_caller] #[inline] pub fn new(init: T, super_init: ::Initializer) -> Self { - // This is unsound; see https://github.com/PyO3/pyo3/issues/4452. - assert!( - super_init.can_be_subclassed(), - "you cannot add a subclass to an existing value", - ); - Self(PyClassInitializerImpl::New { init, super_init }) + Self { init, super_init } } /// Constructs a new initializer from an initializer for the base class. @@ -158,18 +147,13 @@ impl PyClassInitializer { where T: PyClass, { - let (init, super_init) = match self.0 { - PyClassInitializerImpl::Existing(value) => return Ok(value.into_bound(py)), - PyClassInitializerImpl::New { init, super_init } => (init, super_init), - }; - - let obj = unsafe { super_init.into_new_object(py, target_type)? }; + let obj = unsafe { self.super_init.into_new_object(py, target_type)? }; // SAFETY: `obj` is constructed using `T::Layout` but has not been initialized yet let contents = unsafe { ::Layout::contents_uninit(obj) }; // SAFETY: `contents` is a non-null pointer to the space allocated for our // `PyClassObjectContents` (either statically in Rust or dynamically by Python) - unsafe { (*contents).write(PyClassObjectContents::new(init)) }; + unsafe { (*contents).write(PyClassObjectContents::new(self.init)) }; // Safety: obj is a valid pointer to an object of type `target_type`, which` is a known // subclass of `T` @@ -188,11 +172,6 @@ impl PyObjectInit for PyClassInitializer { .map(Bound::into_ptr) } } - - #[inline] - fn can_be_subclassed(&self) -> bool { - !matches!(self.0, PyClassInitializerImpl::Existing(..)) - } } impl From for PyClassInitializer @@ -219,54 +198,3 @@ where PyClassInitializer::from(base).add_subclass(sub) } } - -impl From> for PyClassInitializer { - #[inline] - fn from(value: Py) -> PyClassInitializer { - PyClassInitializer(PyClassInitializerImpl::Existing(value)) - } -} - -impl<'py, T: PyClass> From> for PyClassInitializer { - #[inline] - fn from(value: Bound<'py, T>) -> PyClassInitializer { - PyClassInitializer::from(value.unbind()) - } -} - -// Implementation used by proc macros to allow anything convertible to PyClassInitializer to be -// the return value of pyclass #[new] method (optionally wrapped in `Result`). -impl IntoPyCallbackOutput<'_, PyClassInitializer> for U -where - T: PyClass, - U: Into>, -{ - #[inline] - fn convert(self, _py: Python<'_>) -> PyResult> { - Ok(self.into()) - } -} - -#[cfg(all(test, feature = "macros"))] -mod tests { - //! See https://github.com/PyO3/pyo3/issues/4452. - - use crate::prelude::*; - - #[pyclass(crate = "crate", subclass)] - struct BaseClass {} - - #[pyclass(crate = "crate", extends=BaseClass)] - struct SubClass { - _data: i32, - } - - #[test] - #[should_panic] - fn add_subclass_to_py_is_unsound() { - Python::attach(|py| { - let base = Py::new(py, BaseClass {}).unwrap(); - let _subclass = PyClassInitializer::from(base).add_subclass(SubClass { _data: 42 }); - }); - } -}