From c7cc48f8e4e07e6fc70b7958488050fd287c6fae Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 14 Feb 2023 08:21:04 +0000 Subject: [PATCH] use PyO3 types within LazyTypeObject --- guide/src/class.md | 1 + pyo3-macros-backend/src/pyclass.rs | 1 + src/impl_/pyclass/lazy_type_object.rs | 47 +++++++++++++-------------- src/pyclass/create_type_object.rs | 21 ++++++------ src/types/module.rs | 6 ++-- tests/test_class_attributes.rs | 4 ++- 6 files changed, 39 insertions(+), 41 deletions(-) diff --git a/guide/src/class.md b/guide/src/class.md index e030213c43b..5220ea94d0b 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -981,6 +981,7 @@ unsafe impl pyo3::type_object::PyTypeInfo for MyClass { fn type_object_raw(py: pyo3::Python<'_>) -> *mut pyo3::ffi::PyTypeObject { ::lazy_type_object() .get_or_init(py) + .as_type_ptr() } } diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 181ea94ac48..bb8bd7f0cf3 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -758,6 +758,7 @@ fn impl_pytypeinfo( <#cls as _pyo3::impl_::pyclass::PyClassImpl>::lazy_type_object() .get_or_init(py) + .as_type_ptr() } } } diff --git a/src/impl_/pyclass/lazy_type_object.rs b/src/impl_/pyclass/lazy_type_object.rs index b0cc272116b..8990bba3717 100644 --- a/src/impl_/pyclass/lazy_type_object.rs +++ b/src/impl_/pyclass/lazy_type_object.rs @@ -9,7 +9,8 @@ use parking_lot::{const_mutex, Mutex}; use crate::{ exceptions::PyRuntimeError, ffi, once_cell::GILOnceCell, pyclass::create_type_object, - IntoPyPointer, PyClass, PyErr, PyMethodDefType, PyObject, PyResult, Python, + types::PyType, AsPyPointer, IntoPyPointer, Py, PyClass, PyErr, PyMethodDefType, PyObject, + PyResult, Python, }; use super::PyClassItemsIter; @@ -20,11 +21,11 @@ pub struct LazyTypeObject(LazyTypeObjectInner, PhantomData); // Non-generic inner of LazyTypeObject to keep code size down struct LazyTypeObjectInner { - value: GILOnceCell<*mut ffi::PyTypeObject>, + value: GILOnceCell>, // Threads which have begun initialization of the `tp_dict`. Used for // reentrant initialization detection. initializing_threads: Mutex>, - tp_dict_filled: GILOnceCell>, + tp_dict_filled: GILOnceCell<()>, } impl LazyTypeObject { @@ -43,7 +44,7 @@ impl LazyTypeObject { impl LazyTypeObject { /// Gets the type object contained by this `LazyTypeObject`, initializing it if needed. - pub fn get_or_init(&self, py: Python<'_>) -> *mut ffi::PyTypeObject { + pub fn get_or_init<'py>(&'py self, py: Python<'py>) -> &'py PyType { self.get_or_try_init(py).unwrap_or_else(|err| { err.print(py); panic!("failed to create type object for {}", T::NAME) @@ -51,31 +52,26 @@ impl LazyTypeObject { } /// Fallible version of the above. - pub(crate) fn get_or_try_init(&self, py: Python<'_>) -> PyResult<*mut ffi::PyTypeObject> { - fn inner() -> PyResult<*mut ffi::PyTypeObject> { - // Safety: `py` is held by the caller of `get_or_init`. - let py = unsafe { Python::assume_gil_acquired() }; - create_type_object::(py) - } - + pub(crate) fn get_or_try_init<'py>(&'py self, py: Python<'py>) -> PyResult<&'py PyType> { self.0 - .get_or_try_init(py, inner::, T::NAME, T::items_iter()) + .get_or_try_init(py, create_type_object::, T::NAME, T::items_iter()) } } impl LazyTypeObjectInner { - fn get_or_try_init( - &self, - py: Python<'_>, - init: fn() -> PyResult<*mut ffi::PyTypeObject>, + // Uses dynamically dispatched fn(Python<'py>) -> PyResult + // so that this code is only instantiated once, instead of for every T + // like the generic LazyTypeObject methods above. + fn get_or_try_init<'py>( + &'py self, + py: Python<'py>, + init: fn(Python<'py>) -> PyResult>, name: &str, items_iter: PyClassItemsIter, - ) -> PyResult<*mut ffi::PyTypeObject> { + ) -> PyResult<&'py PyType> { (|| -> PyResult<_> { - // Uses explicit GILOnceCell::get_or_init:: *mut ffi::PyTypeObject> monomorphization - // so that this code is only monomorphized once, instead of for every T. - let type_object = *self.value.get_or_try_init(py, init)?; - self.ensure_init(py, type_object, name, items_iter)?; + let type_object = self.value.get_or_try_init(py, || init(py))?.as_ref(py); + self.ensure_init(type_object, name, items_iter)?; Ok(type_object) })() .map_err(|err| { @@ -89,11 +85,12 @@ impl LazyTypeObjectInner { fn ensure_init( &self, - py: Python<'_>, - type_object: *mut ffi::PyTypeObject, + type_object: &PyType, name: &str, items_iter: PyClassItemsIter, ) -> PyResult<()> { + let py = type_object.py(); + // We might want to fill the `tp_dict` with python instances of `T` // itself. In order to do so, we must first initialize the type object // with an empty `tp_dict`: now we can create instances of `T`. @@ -167,8 +164,8 @@ impl LazyTypeObjectInner { // Now we hold the GIL and we can assume it won't be released until we // return from the function. - let result = self.tp_dict_filled.get_or_init(py, move || { - let result = initialize_tp_dict(py, type_object as *mut ffi::PyObject, items); + let result = self.tp_dict_filled.get_or_try_init(py, move || { + let result = initialize_tp_dict(py, type_object.as_ptr(), items); // Initialization successfully complete, can clear the thread list. // (No further calls to get_or_init() will try to init, on any thread.) diff --git a/src/pyclass/create_type_object.rs b/src/pyclass/create_type_object.rs index 97f44ef7bdf..2409a0f1ff2 100644 --- a/src/pyclass/create_type_object.rs +++ b/src/pyclass/create_type_object.rs @@ -5,7 +5,8 @@ use crate::{ assign_sequence_item_from_mapping, get_sequence_item_from_mapping, tp_dealloc, PyClassItemsIter, }, - PyClass, PyErr, PyMethodDefType, PyResult, PyTypeInfo, Python, + types::PyType, + Py, PyClass, PyMethodDefType, PyResult, PyTypeInfo, Python, }; use std::{ collections::HashMap, @@ -15,7 +16,7 @@ use std::{ ptr, }; -pub(crate) fn create_type_object(py: Python<'_>) -> PyResult<*mut ffi::PyTypeObject> +pub(crate) fn create_type_object(py: Python<'_>) -> PyResult> where T: PyClass, { @@ -322,7 +323,7 @@ impl PyTypeBuilder { name: &'static str, module_name: Option<&'static str>, basicsize: usize, - ) -> PyResult<*mut ffi::PyTypeObject> { + ) -> PyResult> { // `c_ulong` and `c_uint` have the same size // on some platforms (like windows) #![allow(clippy::useless_conversion)] @@ -370,16 +371,14 @@ impl PyTypeBuilder { }; // Safety: We've correctly setup the PyType_Spec at this point - let type_object = unsafe { ffi::PyType_FromSpec(&mut spec) }; - if type_object.is_null() { - Err(PyErr::fetch(py)) - } else { - for cleanup in std::mem::take(&mut self.cleanup) { - cleanup(&self, type_object as _); - } + let type_object: Py = + unsafe { Py::from_owned_ptr_or_err(py, ffi::PyType_FromSpec(&mut spec))? }; - Ok(type_object as _) + for cleanup in std::mem::take(&mut self.cleanup) { + cleanup(&self, type_object.as_ref(py).as_type_ptr()); } + + Ok(type_object) } } diff --git a/src/types/module.rs b/src/types/module.rs index ecec49b97fc..970a22ff0ce 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -7,7 +7,7 @@ use crate::err::{PyErr, PyResult}; use crate::exceptions; use crate::ffi; use crate::pyclass::PyClass; -use crate::types::{PyAny, PyCFunction, PyDict, PyList, PyString, PyType}; +use crate::types::{PyAny, PyCFunction, PyDict, PyList, PyString}; use crate::{AsPyPointer, IntoPy, Py, PyObject, Python}; use std::ffi::{CStr, CString}; use std::str; @@ -295,9 +295,7 @@ impl PyModule { T: PyClass, { let py = self.py(); - self.add(T::NAME, unsafe { - PyType::from_type_ptr(py, T::lazy_type_object().get_or_try_init(py)?) - }) + self.add(T::NAME, T::lazy_type_object().get_or_try_init(py)?) } /// Adds a function or a (sub)module to a module, using the functions name as name. diff --git a/tests/test_class_attributes.rs b/tests/test_class_attributes.rs index 5070338df71..1d5d3f20de7 100644 --- a/tests/test_class_attributes.rs +++ b/tests/test_class_attributes.rs @@ -1,6 +1,6 @@ #![cfg(feature = "macros")] -use pyo3::{exceptions::PyValueError, prelude::*, types::PyString}; +use pyo3::{prelude::*, types::PyString}; mod common; @@ -98,6 +98,8 @@ fn recursive_class_attributes() { #[test] #[cfg(panic = "unwind")] fn test_fallible_class_attribute() { + use pyo3::exceptions::PyValueError; + #[pyclass] struct BrokenClass;