From b86de9376d36b32596b5d96b649e7ba224898179 Mon Sep 17 00:00:00 2001 From: kngwyu Date: Sun, 15 Dec 2019 11:41:32 +0900 Subject: [PATCH] Introduce PyClassInitializer --- pyo3-derive-backend/src/pymethod.rs | 24 +++-- src/derive_utils.rs | 4 +- src/instance.rs | 9 +- src/lib.rs | 6 +- src/prelude.rs | 1 + src/pyclass.rs | 141 +++++++++++++++++++++++++--- src/type_object.rs | 11 ++- tests/test_dunder.rs | 2 +- tests/test_gc.rs | 9 +- tests/test_getter_setter.rs | 1 - tests/test_inheritance.rs | 50 +++++++++- 11 files changed, 217 insertions(+), 41 deletions(-) diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index e47edc04..355a1e2e 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -222,7 +222,11 @@ pub fn impl_proto_wrap(cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>) -> pub fn impl_wrap_new(cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>) -> TokenStream { let names: Vec = get_arg_names(&spec); let cb = quote! { #cls::#name(#(#names),*) }; - let body = impl_arg_params(spec, cb); + let body = impl_arg_params_( + spec, + cb, + quote! { pyo3::pyclass::IntoInitializer::into_initializer }, + ); quote! { #[allow(unused_mut)] @@ -239,9 +243,9 @@ pub fn impl_wrap_new(cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>) -> T let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); - # body + #body - match _result.and_then(|slf| pyo3::PyClassShell::new(_py, slf)) { + match _result.and_then(|init| init.create_shell(_py)) { Ok(slf) => slf as _, Err(e) => e.restore_and_null(_py), } @@ -409,11 +413,11 @@ fn bool_to_ident(condition: bool) -> syn::Ident { } } -pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream { +fn impl_arg_params_(spec: &FnSpec<'_>, body: TokenStream, into_result: TokenStream) -> TokenStream { if spec.args.is_empty() { return quote! { let _result = { - pyo3::derive_utils::IntoPyResult::into_py_result(#body) + #into_result (#body) }; }; } @@ -471,11 +475,19 @@ pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream { #(#param_conversion)* - pyo3::derive_utils::IntoPyResult::into_py_result(#body) + #into_result(#body) })(); } } +pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream { + impl_arg_params_( + spec, + body, + quote! { pyo3::derive_utils::IntoPyResult::into_py_result }, + ) +} + /// Re option_pos: The option slice doesn't contain the py: Python argument, so the argument /// index and the index in option diverge when using py: Python fn impl_arg_param( diff --git a/src/derive_utils.rs b/src/derive_utils.rs index c1a9ac32..f64cde0d 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -9,9 +9,7 @@ use crate::exceptions::TypeError; use crate::init_once; use crate::instance::PyNativeType; use crate::types::{PyAny, PyDict, PyModule, PyTuple}; -use crate::GILPool; -use crate::Python; -use crate::{ffi, IntoPy, PyObject}; +use crate::{ffi, GILPool, IntoPy, PyObject, Python}; use std::ptr; /// Description of a python parameter; used for `parse_args()`. diff --git a/src/instance.rs b/src/instance.rs index 9c728ecb..b91f2786 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -3,7 +3,7 @@ use crate::err::{PyErr, PyResult}; use crate::gil; use crate::object::PyObject; use crate::objectprotocol::ObjectProtocol; -use crate::pyclass::{PyClass, PyClassShell}; +use crate::pyclass::{IntoInitializer, PyClass, PyClassShell}; use crate::type_object::{PyConcreteObject, PyTypeInfo}; use crate::types::PyAny; use crate::{ffi, IntoPy}; @@ -35,11 +35,12 @@ unsafe impl Sync for Py {} impl Py { /// Create new instance of T and move it under python management - pub fn new(py: Python, value: T) -> PyResult> + pub fn new(py: Python, value: impl IntoInitializer) -> PyResult> where - T: PyClass, + T: PyClass + PyTypeInfo>, { - let obj = unsafe { PyClassShell::new(py, value)? }; + let initializer = value.into_initializer()?; + let obj = unsafe { initializer.create_shell(py)? }; let ob = unsafe { Py::from_owned_ptr(obj as _) }; Ok(ob) } diff --git a/src/lib.rs b/src/lib.rs index 88d62b69..383f753f 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -127,7 +127,7 @@ pub use crate::gil::{init_once, GILGuard, GILPool}; pub use crate::instance::{AsPyRef, ManagedPyRef, Py, PyNativeType}; pub use crate::object::PyObject; pub use crate::objectprotocol::ObjectProtocol; -pub use crate::pyclass::{PyClass, PyClassShell}; +pub use crate::pyclass::{PyClass, PyClassInitializer, PyClassShell}; pub use crate::python::{prepare_freethreaded_python, Python}; pub use crate::type_object::{type_flags, PyConcreteObject, PyTypeInfo}; @@ -217,7 +217,7 @@ macro_rules! wrap_pymodule { /// /// # Example /// ``` -/// use pyo3::{prelude::*, py_run}; +/// use pyo3::{prelude::*, py_run, PyClassShell}; /// #[pyclass] /// #[derive(Debug)] /// struct Time { @@ -240,7 +240,7 @@ macro_rules! wrap_pymodule { /// } /// let gil = Python::acquire_gil(); /// let py = gil.python(); -/// let time = PyRef::new(py, Time {hour: 8, minute: 43, second: 16}).unwrap(); +/// let time = PyClassShell::new_ref(py, Time {hour: 8, minute: 43, second: 16}).unwrap(); /// let time_as_tuple = (8, 43, 16); /// py_run!(py, time time_as_tuple, r#" /// assert time.hour == 8 diff --git a/src/prelude.rs b/src/prelude.rs index 821bc939..4b4cd038 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -18,6 +18,7 @@ pub use crate::objectprotocol::ObjectProtocol; pub use crate::python::Python; pub use crate::{FromPy, FromPyObject, IntoPy, IntoPyPointer, PyTryFrom, PyTryInto, ToPyObject}; // This is only part of the prelude because we need it for the pymodule function +pub use crate::pyclass::PyClassInitializer; pub use crate::types::PyModule; pub use pyo3cls::pymodule; pub use pyo3cls::{pyclass, pyfunction, pymethods, pyproto}; diff --git a/src/pyclass.rs b/src/pyclass.rs index eba23e2c..074d13f7 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -1,6 +1,7 @@ //! An experiment module which has all codes related only to #[pyclass] use crate::class::methods::{PyMethodDefType, PyMethodsProtocol}; use crate::conversion::{AsPyPointer, FromPyPointer, ToPyObject}; +use crate::exceptions::RuntimeError; use crate::pyclass_slots::{PyClassDict, PyClassWeakRef}; use crate::type_object::{type_flags, PyConcreteObject, PyTypeObject}; use crate::types::PyAny; @@ -75,7 +76,7 @@ where } } -/// So this is a shell for our *sweet* pyclasses to survive in *harsh* Python world. +/// `PyClassShell` represents the concrete layout of our `#[pyclass]` in the Python heap. #[repr(C)] pub struct PyClassShell { ob_base: ::ConcreteLayout, @@ -85,28 +86,37 @@ pub struct PyClassShell { } impl PyClassShell { - pub fn new_ref(py: Python, value: T) -> PyResult<&Self> { + pub fn new_ref(py: Python, value: impl IntoInitializer) -> PyResult<&Self> + where + T: PyTypeInfo, + { unsafe { - let ptr = Self::new(py, value)?; - FromPyPointer::from_owned_ptr_or_err(py, ptr as _) + let initializer = value.into_initializer()?; + let self_ = initializer.create_shell(py)?; + FromPyPointer::from_owned_ptr_or_err(py, self_ as _) } } - pub fn new_mut(py: Python, value: T) -> PyResult<&mut Self> { + pub fn new_mut(py: Python, value: impl IntoInitializer) -> PyResult<&mut Self> + where + T: PyTypeInfo, + { unsafe { - let ptr = Self::new(py, value)?; - FromPyPointer::from_owned_ptr_or_err(py, ptr as _) + let initializer = value.into_initializer()?; + let self_ = initializer.create_shell(py)?; + FromPyPointer::from_owned_ptr_or_err(py, self_ as _) } } - pub unsafe fn new(py: Python, value: T) -> PyResult<*mut Self> { + #[doc(hidden)] + unsafe fn new(py: Python) -> PyResult<*mut Self> { + ::init_type(); T::init_type(); let base = T::alloc(py); if base.is_null() { return Err(PyErr::fetch(py)); } let self_ = base as *mut Self; - (*self_).pyclass = ManuallyDrop::new(value); (*self_).dict = T::Dict::new(); (*self_).weakref = T::WeakRef::new(); Ok(self_) @@ -114,13 +124,14 @@ impl PyClassShell { } impl PyConcreteObject for PyClassShell { + const NEED_INIT: bool = std::mem::size_of::() != 0; unsafe fn internal_ref_cast(obj: &PyAny) -> &T { let shell = obj.as_ptr() as *const PyClassShell; - &*(*shell).pyclass + &(*shell).pyclass } unsafe fn internal_mut_cast(obj: &PyAny) -> &mut T { let shell = obj.as_ptr() as *const PyClassShell as *mut PyClassShell; - &mut *(*shell).pyclass + &mut (*shell).pyclass } unsafe fn py_drop(&mut self, py: Python) { ManuallyDrop::drop(&mut self.pyclass); @@ -128,6 +139,12 @@ impl PyConcreteObject for PyClassShell { self.weakref.clear_weakrefs(self.as_ptr(), py); self.ob_base.py_drop(py); } + unsafe fn py_init(&mut self, value: T) { + self.pyclass = ManuallyDrop::new(value); + } + fn get_super(&mut self) -> Option<&mut ::ConcreteLayout> { + Some(&mut self.ob_base) + } } impl AsPyPointer for PyClassShell { @@ -190,6 +207,108 @@ where } } +/// An initializer for `PyClassShell`. +/// +/// **NOTE** If +pub struct PyClassInitializer { + init: Option, + super_init: Option<*mut PyClassInitializer>, +} + +impl PyClassInitializer { + pub fn from_value(value: T) -> Self { + PyClassInitializer { + init: Some(value), + super_init: None, + } + } + + pub fn new() -> Self { + PyClassInitializer { + init: None, + super_init: None, + } + } + + #[must_use] + #[doc(hiddden)] + pub fn init_class(self, shell: &mut T::ConcreteLayout) -> PyResult<()> { + macro_rules! raise_err { + ($name: path) => { + return Err(PyErr::new::(format!( + "Base class '{}' is not initialized", + $name + ))); + }; + } + let PyClassInitializer { init, super_init } = self; + if let Some(value) = init { + unsafe { shell.py_init(value) }; + } else if !T::ConcreteLayout::NEED_INIT { + raise_err!(T::NAME); + } + if let Some(super_init) = super_init { + let super_init = unsafe { Box::from_raw(super_init) }; + if let Some(super_obj) = shell.get_super() { + super_init.init_class(super_obj)?; + } + } else if ::ConcreteLayout::NEED_INIT { + raise_err!(T::BaseType::NAME) + } + Ok(()) + } + + pub fn init(&mut self, value: T) { + self.init = Some(value); + } + + pub fn get_super(&mut self) -> &mut PyClassInitializer { + if let Some(super_init) = self.super_init { + return unsafe { &mut *super_init }; + } + let super_init = Box::into_raw(Box::new(PyClassInitializer::new())); + self.super_init = Some(super_init); + return unsafe { &mut *super_init }; + } + + pub unsafe fn create_shell(self, py: Python) -> PyResult<*mut PyClassShell> + where + T: PyClass + PyTypeInfo>, + { + let shell = PyClassShell::new(py)?; + self.init_class(&mut *shell)?; + Ok(shell) + } +} + +pub trait IntoInitializer { + fn into_initializer(self) -> PyResult>; +} + +impl IntoInitializer for T { + fn into_initializer(self) -> PyResult> { + Ok(PyClassInitializer::from_value(self)) + } +} + +impl IntoInitializer for PyResult { + fn into_initializer(self) -> PyResult> { + self.map(PyClassInitializer::from_value) + } +} + +impl IntoInitializer for PyClassInitializer { + fn into_initializer(self) -> PyResult> { + Ok(self) + } +} + +impl IntoInitializer for PyResult> { + fn into_initializer(self) -> PyResult> { + self + } +} + /// Register new type in python object system. #[cfg(not(Py_LIMITED_API))] pub fn initialize_type(py: Python, module_name: Option<&str>) -> PyResult<*mut ffi::PyTypeObject> diff --git a/src/type_object.rs b/src/type_object.rs index 4b939716..39768602 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -12,7 +12,8 @@ use crate::Python; use std::ptr::NonNull; /// TODO: write document -pub trait PyConcreteObject: Sized { +pub trait PyConcreteObject: Sized { + const NEED_INIT: bool = false; unsafe fn internal_ref_cast(obj: &PyAny) -> &T { &*(obj as *const _ as *const T) } @@ -20,9 +21,13 @@ pub trait PyConcreteObject: Sized { &mut *(obj as *const _ as *const T as *mut T) } unsafe fn py_drop(&mut self, _py: Python) {} + unsafe fn py_init(&mut self, _value: T) {} + fn get_super(&mut self) -> Option<&mut ::ConcreteLayout> { + None + } } -impl PyConcreteObject for ffi::PyObject {} +impl PyConcreteObject for ffi::PyObject {} /// Our custom type flags pub mod type_flags { @@ -57,7 +62,7 @@ pub trait PyTypeInfo: Sized { const FLAGS: usize = 0; /// Base class - type BaseType: PyTypeInfo; + type BaseType: PyTypeInfo + PyTypeObject; /// Layout type ConcreteLayout: PyConcreteObject; diff --git a/tests/test_dunder.rs b/tests/test_dunder.rs index 6bb177cf..6fecc4c7 100755 --- a/tests/test_dunder.rs +++ b/tests/test_dunder.rs @@ -57,7 +57,7 @@ impl<'p> PyIterProtocol for Iterator { Ok(slf.into()) } - fn __next__(mut slf: &mut PyClassShell) -> PyResult> { + fn __next__(slf: &mut PyClassShell) -> PyResult> { Ok(slf.iter.next()) } } diff --git a/tests/test_gc.rs b/tests/test_gc.rs index 81fa8963..cac90a79 100644 --- a/tests/test_gc.rs +++ b/tests/test_gc.rs @@ -3,7 +3,7 @@ use pyo3::class::PyTraverseError; use pyo3::class::PyVisit; use pyo3::prelude::*; use pyo3::types::{PyAny, PyTuple}; -use pyo3::{ffi, py_run, AsPyPointer, PyClassShell}; +use pyo3::{ffi, py_run, AsPyPointer, PyClassInitializer, PyClassShell}; use std::cell::RefCell; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -235,10 +235,11 @@ struct SubClassWithDrop { #[pymethods] impl SubClassWithDrop { - // TODO(kngwyu): Implement baseclass initialization #[new] - fn new() -> SubClassWithDrop { - SubClassWithDrop { data: None } + fn new() -> PyClassInitializer { + let mut init = PyClassInitializer::from_value(SubClassWithDrop { data: None }); + init.get_super().init(BaseClassWithDrop { data: None }); + init } } diff --git a/tests/test_getter_setter.rs b/tests/test_getter_setter.rs index cacf5652..7610e81f 100644 --- a/tests/test_getter_setter.rs +++ b/tests/test_getter_setter.rs @@ -1,7 +1,6 @@ use pyo3::prelude::*; use pyo3::py_run; use pyo3::types::{IntoPyDict, PyList}; -use std::isize; mod common; diff --git a/tests/test_inheritance.rs b/tests/test_inheritance.rs index 462ff138..14aed197 100644 --- a/tests/test_inheritance.rs +++ b/tests/test_inheritance.rs @@ -1,5 +1,6 @@ use pyo3::prelude::*; use pyo3::py_run; + #[cfg(feature = "unsound-subclass")] use pyo3::types::IntoPyDict; @@ -48,19 +49,58 @@ struct SubClass { #[pymethods] impl SubClass { #[new] - fn new() -> Self { - SubClass { val2: 5 } + fn new() -> PyClassInitializer { + let mut init = PyClassInitializer::from_value(SubClass { val2: 5 }); + init.get_super().init(BaseClass { val1: 10 }); + init } } -// TODO(kngwyu): disable untill super().__init__ fixed #[test] -#[ignore] fn inheritance_with_new_methods() { let gil = Python::acquire_gil(); let py = gil.python(); - let _typebase = py.get_type::(); + let _baseobj = py.get_type::(); let typeobj = py.get_type::(); let inst = typeobj.call((), None).unwrap(); py_run!(py, inst, "assert inst.val1 == 10; assert inst.val2 == 5"); } + +#[pyclass(extends=BaseClass)] +struct InvalidSubClass { + #[pyo3(get)] + val2: usize, +} + +#[pymethods] +impl InvalidSubClass { + #[new] + fn new() -> PyClassInitializer { + PyClassInitializer::from_value(InvalidSubClass { val2: 5 }) + } +} + +#[test] +fn uninit_baseclass_raise_exception() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let _baseclass = py.get_type::(); + let subclass = py.get_type::(); + py_expect_exception!(py, subclass, "subclass()", RuntimeError); +} + +#[test] +fn uninit_baseclass_returns_err() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let subclass = pyo3::pyclass::PyClassShell::new_ref(py, InvalidSubClass { val2: 5 }); + if let Err(err) = subclass { + py_run!( + py, + err, + r#"str(err) == "Base class 'BaseClass' is not initialized""# + ) + } else { + panic!("Uninitialized class detection failed!!!") + } +}