Skip to content

Commit

Permalink
use PyO3 types within LazyTypeObject
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Feb 18, 2023
1 parent 00ddd21 commit c7cc48f
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 41 deletions.
1 change: 1 addition & 0 deletions guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,7 @@ unsafe impl pyo3::type_object::PyTypeInfo for MyClass {
fn type_object_raw(py: pyo3::Python<'_>) -> *mut pyo3::ffi::PyTypeObject {
<Self as pyo3::impl_::pyclass::PyClassImpl>::lazy_type_object()
.get_or_init(py)
.as_type_ptr()
}
}

Expand Down
1 change: 1 addition & 0 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ fn impl_pytypeinfo(

<#cls as _pyo3::impl_::pyclass::PyClassImpl>::lazy_type_object()
.get_or_init(py)
.as_type_ptr()
}
}
}
Expand Down
47 changes: 22 additions & 25 deletions src/impl_/pyclass/lazy_type_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -20,11 +21,11 @@ pub struct LazyTypeObject<T>(LazyTypeObjectInner, PhantomData<T>);

// Non-generic inner of LazyTypeObject to keep code size down
struct LazyTypeObjectInner {
value: GILOnceCell<*mut ffi::PyTypeObject>,
value: GILOnceCell<Py<PyType>>,
// Threads which have begun initialization of the `tp_dict`. Used for
// reentrant initialization detection.
initializing_threads: Mutex<Vec<ThreadId>>,
tp_dict_filled: GILOnceCell<PyResult<()>>,
tp_dict_filled: GILOnceCell<()>,
}

impl<T> LazyTypeObject<T> {
Expand All @@ -43,39 +44,34 @@ impl<T> LazyTypeObject<T> {

impl<T: PyClass> LazyTypeObject<T> {
/// 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)
})
}

/// Fallible version of the above.
pub(crate) fn get_or_try_init(&self, py: Python<'_>) -> PyResult<*mut ffi::PyTypeObject> {
fn inner<T: PyClass>() -> 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::<T>(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>, T::NAME, T::items_iter())
.get_or_try_init(py, create_type_object::<T>, 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<Py<PyType>
// so that this code is only instantiated once, instead of for every T
// like the generic LazyTypeObject<T> methods above.
fn get_or_try_init<'py>(
&'py self,
py: Python<'py>,
init: fn(Python<'py>) -> PyResult<Py<PyType>>,
name: &str,
items_iter: PyClassItemsIter,
) -> PyResult<*mut ffi::PyTypeObject> {
) -> PyResult<&'py PyType> {
(|| -> PyResult<_> {
// Uses explicit GILOnceCell::get_or_init::<fn() -> *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| {
Expand All @@ -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`.
Expand Down Expand Up @@ -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.)
Expand Down
21 changes: 10 additions & 11 deletions src/pyclass/create_type_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -15,7 +16,7 @@ use std::{
ptr,
};

pub(crate) fn create_type_object<T>(py: Python<'_>) -> PyResult<*mut ffi::PyTypeObject>
pub(crate) fn create_type_object<T>(py: Python<'_>) -> PyResult<Py<PyType>>
where
T: PyClass,
{
Expand Down Expand Up @@ -322,7 +323,7 @@ impl PyTypeBuilder {
name: &'static str,
module_name: Option<&'static str>,
basicsize: usize,
) -> PyResult<*mut ffi::PyTypeObject> {
) -> PyResult<Py<PyType>> {
// `c_ulong` and `c_uint` have the same size
// on some platforms (like windows)
#![allow(clippy::useless_conversion)]
Expand Down Expand Up @@ -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<PyType> =
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)
}
}

Expand Down
6 changes: 2 additions & 4 deletions src/types/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion tests/test_class_attributes.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![cfg(feature = "macros")]

use pyo3::{exceptions::PyValueError, prelude::*, types::PyString};
use pyo3::{prelude::*, types::PyString};

mod common;

Expand Down Expand Up @@ -98,6 +98,8 @@ fn recursive_class_attributes() {
#[test]
#[cfg(panic = "unwind")]
fn test_fallible_class_attribute() {
use pyo3::exceptions::PyValueError;

#[pyclass]
struct BrokenClass;

Expand Down

0 comments on commit c7cc48f

Please sign in to comment.