diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a5859f03af..0d16f3cdb59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * `PyRef`, `PyRefMut`, `PyRawObject`. [#683](https://github.com/PyO3/pyo3/pull/683) * `PyNoArgsFunction`. [#741](https://github.com/PyO3/pyo3/pull/741) +* `initialize_type()`. To set the module name for a `#[pyclass]`, use the `module` argument to the macro. #[751](https://github.com/PyO3/pyo3/pull/751) ## [0.8.5] diff --git a/guide/src/class.md b/guide/src/class.md index eb00b2e59ba..ef37bd46343 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -31,7 +31,7 @@ struct MyClass { impl pyo3::pyclass::PyClassAlloc for MyClass {} -impl pyo3::PyTypeInfo for MyClass { +unsafe impl pyo3::PyTypeInfo for MyClass { type Type = MyClass; type BaseType = pyo3::types::PyAny; type ConcreteLayout = pyo3::PyClassShell; @@ -43,10 +43,23 @@ impl pyo3::PyTypeInfo for MyClass { const FLAGS: usize = 0; #[inline] - fn type_object() -> *mut pyo3::ffi::PyTypeObject { - static TYPE_OBJECT: pyo3::derive_utils::LazyTypeObject = - pyo3::derive_utils::LazyTypeObject::new(); - TYPE_OBJECT.get() + fn type_object() -> std::ptr::NonNull { + use std::ptr::NonNull; + use pyo3::type_object::LazyTypeObject; + static TYPE_OBJECT: LazyTypeObject = LazyTypeObject::new(); + TYPE_OBJECT.get_or_init(|| { + // automatically initialize the class on-demand + let gil = pyo3::Python::acquire_gil(); + let py = gil.python(); + let boxed = pyo3::pyclass::create_type_object::(py, Self::MODULE)?; + Ok(unsafe { NonNull::new_unchecked(Box::into_raw(boxed)) }) + }) + .unwrap_or_else(|e| { + let gil = Python::acquire_gil(); + let py = gil.python(); + e.print(py); + panic!("An error occurred while initializing class {}", Self::NAME) + }) } } diff --git a/pyo3-derive-backend/src/pyclass.rs b/pyo3-derive-backend/src/pyclass.rs index 15addd6f03c..51f63d1c3b5 100644 --- a/pyo3-derive-backend/src/pyclass.rs +++ b/pyo3-derive-backend/src/pyclass.rs @@ -273,8 +273,6 @@ fn impl_class( if FREELIST.is_null() { FREELIST = Box::into_raw(Box::new( pyo3::freelist::FreeList::with_capacity(#freelist))); - - <#cls as pyo3::type_object::PyTypeObject>::init_type(); } &mut *FREELIST } @@ -372,7 +370,7 @@ fn impl_class( }; Ok(quote! { - impl pyo3::type_object::PyTypeInfo for #cls { + unsafe impl pyo3::type_object::PyTypeInfo for #cls { type Type = #cls; type BaseType = #base; type ConcreteLayout = pyo3::pyclass::PyClassShell; @@ -384,10 +382,23 @@ fn impl_class( const FLAGS: usize = #(#flags)|* | #extended; #[inline] - fn type_object() -> *mut pyo3::ffi::PyTypeObject { - static TYPE_OBJECT: pyo3::derive_utils::LazyTypeObject = - pyo3::derive_utils::LazyTypeObject::new(); - TYPE_OBJECT.get() + fn type_object() -> std::ptr::NonNull { + use std::ptr::NonNull; + use pyo3::type_object::LazyTypeObject; + static TYPE_OBJECT: LazyTypeObject = LazyTypeObject::new(); + TYPE_OBJECT.get_or_init(|| { + // automatically initialize the class on-demand + let gil = pyo3::Python::acquire_gil(); + let py = gil.python(); + let boxed = pyo3::pyclass::create_type_object::(py, Self::MODULE)?; + Ok(unsafe { NonNull::new_unchecked(Box::into_raw(boxed)) }) + }) + .unwrap_or_else(|e| { + let gil = Python::acquire_gil(); + let py = gil.python(); + e.print(py); + panic!("An error occurred while initializing class {}", Self::NAME) + }) } } diff --git a/src/derive_utils.rs b/src/derive_utils.rs index d136db4ccae..a5e284b5f9a 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -12,8 +12,6 @@ use crate::pyclass::PyClass; use crate::pyclass_init::PyClassInitializer; use crate::types::{PyAny, PyDict, PyModule, PyTuple}; use crate::{ffi, GILPool, IntoPy, PyObject, Python}; -use once_cell::sync::OnceCell; -use std::cell::UnsafeCell; use std::ptr; /// Description of a python parameter; used for `parse_args()`. @@ -201,28 +199,3 @@ impl>> IntoPyNewResult for PyRes self } } - -/// Type used to store type objects -pub struct LazyTypeObject { - cell: OnceCell>, -} - -impl LazyTypeObject { - pub const fn new() -> Self { - Self { - cell: OnceCell::new(), - } - } - - pub fn get(&self) -> *mut ffi::PyTypeObject { - self.cell - .get_or_init(|| UnsafeCell::new(ffi::PyTypeObject_INIT)) - .get() - } -} - -// This is necessary for making static `LazyTypeObject`s -// -// Type objects are shared between threads by the Python interpreter anyway, so it is no worse -// to allow sharing on the Rust side too. -unsafe impl Sync for LazyTypeObject {} diff --git a/src/err.rs b/src/err.rs index 7b884b21a0a..337d9a1e718 100644 --- a/src/err.rs +++ b/src/err.rs @@ -14,6 +14,7 @@ use libc::c_int; use std::ffi::CString; use std::io; use std::os::raw::c_char; +use std::ptr::NonNull; /// Represents a `PyErr` value /// @@ -179,7 +180,7 @@ impl PyErr { name: &str, base: Option<&PyType>, dict: Option, - ) -> *mut ffi::PyTypeObject { + ) -> NonNull { let base: *mut ffi::PyObject = match base { None => std::ptr::null_mut(), Some(obj) => obj.as_ptr(), @@ -193,8 +194,12 @@ impl PyErr { unsafe { let null_terminated_name = CString::new(name).expect("Failed to initialize nul terminated exception name"); - ffi::PyErr_NewException(null_terminated_name.as_ptr() as *mut c_char, base, dict) - as *mut ffi::PyTypeObject + + NonNull::new_unchecked(ffi::PyErr_NewException( + null_terminated_name.as_ptr() as *mut c_char, + base, + dict, + ) as *mut ffi::PyTypeObject) } } diff --git a/src/exceptions.rs b/src/exceptions.rs index 3dc253a90bc..e42d265bfaf 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -89,16 +89,15 @@ macro_rules! import_exception { macro_rules! import_exception_type_object { ($module: expr, $name: ident) => { unsafe impl $crate::type_object::PyTypeObject for $name { - fn init_type() -> std::ptr::NonNull<$crate::ffi::PyTypeObject> { - // We can't use lazy_static here because raw pointers aren't Send - static TYPE_OBJECT_ONCE: ::std::sync::Once = ::std::sync::Once::new(); - static mut TYPE_OBJECT: *mut $crate::ffi::PyTypeObject = ::std::ptr::null_mut(); + fn type_object() -> $crate::Py<$crate::types::PyType> { + use $crate::type_object::LazyTypeObject; + static TYPE_OBJECT: LazyTypeObject = LazyTypeObject::new(); - TYPE_OBJECT_ONCE.call_once(|| { - let gil = $crate::Python::acquire_gil(); - let py = gil.python(); + let ptr = TYPE_OBJECT + .get_or_init(|| { + let gil = $crate::Python::acquire_gil(); + let py = gil.python(); - unsafe { let imp = py .import(stringify!($module)) .expect(concat!("Can not import module: ", stringify!($module))); @@ -108,12 +107,16 @@ macro_rules! import_exception_type_object { ".", stringify!($name) )); - TYPE_OBJECT = - $crate::IntoPyPointer::into_ptr(cls) as *mut $crate::ffi::PyTypeObject; - } - }); - unsafe { std::ptr::NonNull::new_unchecked(TYPE_OBJECT) } + unsafe { + Ok(std::ptr::NonNull::new_unchecked( + $crate::IntoPyPointer::into_ptr(cls) as *mut _, + )) + } + }) + .unwrap(); + + unsafe { $crate::Py::from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) } } } }; @@ -174,26 +177,25 @@ macro_rules! create_exception { macro_rules! create_exception_type_object { ($module: ident, $name: ident, $base: ty) => { unsafe impl $crate::type_object::PyTypeObject for $name { - fn init_type() -> std::ptr::NonNull<$crate::ffi::PyTypeObject> { - // We can't use lazy_static here because raw pointers aren't Send - static TYPE_OBJECT_ONCE: ::std::sync::Once = ::std::sync::Once::new(); - static mut TYPE_OBJECT: *mut $crate::ffi::PyTypeObject = ::std::ptr::null_mut(); + fn type_object() -> $crate::Py<$crate::types::PyType> { + use $crate::type_object::LazyTypeObject; + static TYPE_OBJECT: LazyTypeObject = LazyTypeObject::new(); - TYPE_OBJECT_ONCE.call_once(|| { - let gil = $crate::Python::acquire_gil(); - let py = gil.python(); + let ptr = TYPE_OBJECT + .get_or_init(|| { + let gil = $crate::Python::acquire_gil(); + let py = gil.python(); - unsafe { - TYPE_OBJECT = $crate::PyErr::new_type( + Ok($crate::PyErr::new_type( py, concat!(stringify!($module), ".", stringify!($name)), Some(py.get_type::<$base>()), None, - ); - } - }); + )) + }) + .unwrap(); - unsafe { std::ptr::NonNull::new_unchecked(TYPE_OBJECT) } + unsafe { $crate::Py::from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) } } } }; @@ -222,8 +224,8 @@ macro_rules! impl_native_exception ( } } unsafe impl PyTypeObject for $name { - fn init_type() -> std::ptr::NonNull<$crate::ffi::PyTypeObject> { - unsafe { std::ptr::NonNull::new_unchecked(ffi::$exc_name as *mut _) } + fn type_object() -> $crate::Py<$crate::types::PyType> { + unsafe { $crate::Py::from_borrowed_ptr(ffi::$exc_name) } } } ); diff --git a/src/freelist.rs b/src/freelist.rs index 939c3d1690b..4217839d4c3 100644 --- a/src/freelist.rs +++ b/src/freelist.rs @@ -74,7 +74,7 @@ where { unsafe fn alloc(_py: Python) -> *mut Self::ConcreteLayout { if let Some(obj) = ::get_free_list().pop() { - ffi::PyObject_Init(obj, ::type_object()); + ffi::PyObject_Init(obj, ::type_object().as_ptr() as *mut _); obj as _ } else { crate::pyclass::default_alloc::() as _ @@ -90,7 +90,7 @@ where } if let Some(obj) = ::get_free_list().insert(obj) { - match (*Self::type_object()).tp_free { + match Self::type_object().as_ref().tp_free { Some(free) => free(obj as *mut c_void), None => tp_free_fallback(obj), } diff --git a/src/pyclass.rs b/src/pyclass.rs index a01585a48f4..5984251d151 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -3,7 +3,7 @@ use crate::class::methods::{PyMethodDefType, PyMethodsProtocol}; use crate::conversion::{AsPyPointer, FromPyPointer, ToPyObject}; use crate::pyclass_init::PyClassInitializer; use crate::pyclass_slots::{PyClassDict, PyClassWeakRef}; -use crate::type_object::{type_flags, PyObjectLayout, PyObjectSizedLayout, PyTypeObject}; +use crate::type_object::{type_flags, PyObjectLayout, PyObjectSizedLayout}; use crate::types::PyAny; use crate::{class, ffi, gil, PyErr, PyObject, PyResult, PyTypeInfo, Python}; use std::ffi::CString; @@ -13,12 +13,12 @@ use std::ptr::{self, NonNull}; #[inline] pub(crate) unsafe fn default_alloc() -> *mut ffi::PyObject { - let tp_ptr = T::type_object(); + let tp_ptr = T::type_object().as_ptr(); if T::FLAGS & type_flags::EXTENDED != 0 && ::ConcreteLayout::IS_NATIVE_TYPE { - let base_tp_ptr = ::type_object(); - if let Some(base_new) = (*base_tp_ptr).tp_new { + let base_tp = ::type_object(); + if let Some(base_new) = base_tp.as_ref().tp_new { return base_new(tp_ptr, ptr::null_mut(), ptr::null_mut()); } } @@ -47,7 +47,7 @@ pub trait PyClassAlloc: PyTypeInfo + Sized { return; } - match (*Self::type_object()).tp_free { + match Self::type_object().as_ref().tp_free { Some(free) => free(obj as *mut c_void), None => tp_free_fallback(obj), } @@ -82,30 +82,6 @@ pub trait PyClass: type WeakRef: PyClassWeakRef; } -unsafe impl PyTypeObject for T -where - T: PyClass, -{ - fn init_type() -> NonNull { - ::init_type(); - let type_object = ::type_object(); - let type_flags = unsafe { (*type_object).tp_flags }; - - if (type_flags & ffi::Py_TPFLAGS_READY) == 0 { - // automatically initialize the class on-demand - let gil = Python::acquire_gil(); - let py = gil.python(); - - initialize_type::(py, ::MODULE).unwrap_or_else(|e| { - e.print(py); - panic!("An error occurred while initializing class {}", Self::NAME) - }); - } - - unsafe { NonNull::new_unchecked(type_object) } - } -} - /// `PyClassShell` represents the concrete layout of `T: PyClass` when it is converted /// to a Python class. /// @@ -179,7 +155,6 @@ impl PyClassShell { ::ConcreteLayout: crate::type_object::PyObjectSizedLayout, { - T::init_type(); let base = T::alloc(py); if base.is_null() { return Err(PyErr::fetch(py)); @@ -277,14 +252,17 @@ where } } -/// Register `T: PyClass` to Python interpreter. #[cfg(not(Py_LIMITED_API))] -pub fn initialize_type(py: Python, module_name: Option<&str>) -> PyResult<*mut ffi::PyTypeObject> +pub fn create_type_object( + py: Python, + module_name: Option<&str>, +) -> PyResult> where T: PyClass, { - let type_object: &mut ffi::PyTypeObject = unsafe { &mut *T::type_object() }; - let base_type_object = ::type_object(); + let mut boxed = Box::new(ffi::PyTypeObject_INIT); + let mut type_object = boxed.as_mut(); + let base_type_object = ::type_object().as_ptr(); // PyPy will segfault if passed only a nul terminator as `tp_doc`. // ptr::null() is OK though. @@ -391,7 +369,7 @@ where // register type object unsafe { if ffi::PyType_Ready(type_object) == 0 { - Ok(type_object as *mut ffi::PyTypeObject) + Ok(boxed) } else { PyErr::fetch(py).into() } diff --git a/src/type_object.rs b/src/type_object.rs index d61dcb5bfc6..fbe46f68ebb 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -1,10 +1,12 @@ // Copyright (c) 2017-present PyO3 Project and Contributors //! Python type object information +use crate::err::PyResult; use crate::instance::Py; use crate::pyclass_init::PyObjectInit; use crate::types::{PyAny, PyType}; use crate::{ffi, AsPyPointer, Python}; +use once_cell::sync::OnceCell; use std::ptr::NonNull; /// `T: PyObjectLayout` represents that `T` is a concrete representaion of `U` in Python heap. @@ -60,7 +62,11 @@ pub mod type_flags { /// Python type information. /// All Python native types(e.g., `PyDict`) and `#[pyclass]` structs implement this trait. -pub trait PyTypeInfo: Sized { +/// +/// This trait is marked unsafe because: +/// - specifying the incorrect layout can lead to memory errors +/// - the return value of type_object must always point to the same PyTypeObject instance +pub unsafe trait PyTypeInfo: Sized { /// Type of objects to store in PyObject struct type Type; @@ -85,32 +91,64 @@ pub trait PyTypeInfo: Sized { /// Initializer for layout type Initializer: PyObjectInit; - /// PyTypeObject instance for this type, which might still need to - /// be initialized - fn type_object() -> *mut ffi::PyTypeObject; + /// PyTypeObject instance for this type, guaranteed to be global and initialized. + fn type_object() -> NonNull; /// Check if `*mut ffi::PyObject` is instance of this type fn is_instance(object: &PyAny) -> bool { - unsafe { ffi::PyObject_TypeCheck(object.as_ptr(), Self::type_object()) != 0 } + unsafe { + ffi::PyObject_TypeCheck(object.as_ptr(), Self::type_object().as_ptr() as *mut _) != 0 + } } /// Check if `*mut ffi::PyObject` is exact instance of this type fn is_exact_instance(object: &PyAny) -> bool { - unsafe { (*object.as_ptr()).ob_type == Self::type_object() } + unsafe { (*object.as_ptr()).ob_type == Self::type_object().as_ptr() as *mut _ } } } /// Python object types that have a corresponding type object. /// -/// This trait is marked unsafe because not fulfilling the contract for [PyTypeObject::init_type] -/// leads to UB +/// This trait is marked unsafe because not fulfilling the contract for type_object +/// leads to UB. +/// +/// See [PyTypeInfo::type_object] pub unsafe trait PyTypeObject { - /// This function must make sure that the corresponding type object gets - /// initialized exactly once and return it. - fn init_type() -> NonNull; + /// Returns the safe abstraction over the type object. + fn type_object() -> Py; +} - /// Returns the safe abstraction over the type object from [PyTypeObject::init_type] +unsafe impl PyTypeObject for T +where + T: PyTypeInfo, +{ fn type_object() -> Py { - unsafe { Py::from_borrowed_ptr(Self::init_type().as_ptr() as *mut ffi::PyObject) } + unsafe { Py::from_borrowed_ptr(::type_object().as_ptr() as *mut _) } } } + +/// Type used to store type objects +pub struct LazyTypeObject { + cell: OnceCell>, +} + +impl LazyTypeObject { + pub const fn new() -> Self { + Self { + cell: OnceCell::new(), + } + } + + pub fn get_or_init(&self, constructor: F) -> PyResult> + where + F: Fn() -> PyResult>, + { + Ok(*self.cell.get_or_try_init(constructor)?) + } +} + +// This is necessary for making static `LazyTypeObject`s +// +// Type objects are shared between threads by the Python interpreter anyway, so it is no worse +// to allow sharing on the Rust side too. +unsafe impl Sync for LazyTypeObject {} diff --git a/src/types/mod.rs b/src/types/mod.rs index fddc470dec4..4b4a8b99c36 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -101,7 +101,7 @@ macro_rules! pyobject_native_var_type { macro_rules! pyobject_native_type_convert( ($name: ty, $layout: path, $typeobject: expr, $module: expr, $checkfunction: path $(,$type_param: ident)*) => { - impl<$($type_param,)*> $crate::type_object::PyTypeInfo for $name { + unsafe impl<$($type_param,)*> $crate::type_object::PyTypeInfo for $name { type Type = (); type BaseType = $crate::types::PyAny; type ConcreteLayout = $layout; @@ -111,28 +111,17 @@ macro_rules! pyobject_native_type_convert( const MODULE: Option<&'static str> = $module; #[inline] - fn type_object() -> *mut $crate::ffi::PyTypeObject { - unsafe { &mut $typeobject as *mut _ } + fn type_object() -> std::ptr::NonNull<$crate::ffi::PyTypeObject> { + unsafe { std::ptr::NonNull::new_unchecked(&mut $typeobject as *mut _) } } #[allow(unused_unsafe)] fn is_instance(ptr: &$crate::types::PyAny) -> bool { use $crate::AsPyPointer; - unsafe { $checkfunction(ptr.as_ptr()) > 0 } } } - unsafe impl<$($type_param,)*> $crate::type_object::PyTypeObject for $name { - fn init_type() -> std::ptr::NonNull<$crate::ffi::PyTypeObject> { - unsafe { - std::ptr::NonNull::new_unchecked( - ::type_object() - ) - } - } - } - impl<$($type_param,)*> $crate::ToPyObject for $name { #[inline] diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index ae18a3b1d48..b8420a486eb 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -7,7 +7,7 @@ use crate::ffi; use crate::instance::{Py, PyNativeType}; use crate::internal_tricks::Unsendable; use crate::object::PyObject; -use crate::type_object::{PyTypeInfo, PyTypeObject}; +use crate::type_object::PyTypeObject; use crate::AsPyPointer; use crate::Python; use std::borrow::Cow; @@ -21,8 +21,8 @@ pyobject_native_var_type!(PyType, ffi::PyType_Type, ffi::PyType_Check); impl PyType { #[inline] - pub fn new() -> Py { - unsafe { Py::from_borrowed_ptr(T::type_object() as *const _ as *mut ffi::PyObject) } + pub fn new() -> Py { + T::type_object() } /// Retrieves the underlying FFI pointer associated with this Python object. diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs index c026bfd17ad..8298b761f9e 100644 --- a/tests/test_class_basics.rs +++ b/tests/test_class_basics.rs @@ -1,6 +1,5 @@ use pyo3::prelude::*; use pyo3::py_run; -use pyo3::pyclass::initialize_type; mod common; @@ -111,9 +110,4 @@ fn empty_class_in_module() { // We currently have no way of determining a canonical module, so builtins is better // than using whatever calls init first. assert_eq!(module, "builtins"); - - // The module name can also be set manually by calling `initialize_type`. - initialize_type::(py, Some("test_module.nested")).unwrap(); - let module: String = ty.getattr("__module__").unwrap().extract().unwrap(); - assert_eq!(module, "test_module.nested"); } diff --git a/tests/test_various.rs b/tests/test_various.rs index c4b8e6c878a..e24db369606 100644 --- a/tests/test_various.rs +++ b/tests/test_various.rs @@ -1,5 +1,4 @@ use pyo3::prelude::*; -use pyo3::pyclass::initialize_type; use pyo3::types::IntoPyDict; use pyo3::types::{PyDict, PyTuple}; use pyo3::{py_run, wrap_pyfunction, AsPyRef, PyClassShell}; @@ -117,7 +116,7 @@ fn pytuple_pyclass_iter() { py_assert!(py, tup, "tup[0] != tup[1]"); } -#[pyclass(dict)] +#[pyclass(dict, module = "test_module")] struct PickleSupport {} #[pymethods] @@ -153,7 +152,6 @@ fn test_pickle() { let module = PyModule::new(py, "test_module").unwrap(); module.add_class::().unwrap(); add_module(py, module).unwrap(); - initialize_type::(py, Some("test_module")).unwrap(); let inst = PyClassShell::new_ref(py, PickleSupport {}).unwrap(); py_run!( py,