Merge pull request #751 from davidhewitt/remove-static-mut

Remove static mut from PyTypeInfo implementation
This commit is contained in:
Yuji Kanagawa 2020-02-03 17:39:25 +09:00 committed by GitHub
commit 8fea23a0b8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 143 additions and 117 deletions

View file

@ -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]

View file

@ -29,6 +29,7 @@ parking_lot = { version = "0.10", features = ["nightly"] }
paste = "0.1.6"
pyo3cls = { path = "pyo3cls", version = "=0.9.0-alpha.1" }
unindent = "0.1.4"
once_cell = "1.3.1"
[dev-dependencies]
assert_approx_eq = "1.1.0"

View file

@ -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<Self>;
@ -43,9 +43,10 @@ impl pyo3::PyTypeInfo for MyClass {
const FLAGS: usize = 0;
#[inline]
unsafe fn type_object() -> &'static mut pyo3::ffi::PyTypeObject {
static mut TYPE_OBJECT: pyo3::ffi::PyTypeObject = pyo3::ffi::PyTypeObject_INIT;
&mut TYPE_OBJECT
fn type_object() -> std::ptr::NonNull<pyo3::ffi::PyTypeObject> {
use pyo3::type_object::LazyTypeObject;
static TYPE_OBJECT: LazyTypeObject = LazyTypeObject::new();
TYPE_OBJECT.get_pyclass_type::<Self>()
}
}

View file

@ -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<Self>;
@ -384,9 +382,10 @@ fn impl_class(
const FLAGS: usize = #(#flags)|* | #extended;
#[inline]
unsafe fn type_object() -> &'static mut pyo3::ffi::PyTypeObject {
static mut TYPE_OBJECT: pyo3::ffi::PyTypeObject = pyo3::ffi::PyTypeObject_INIT;
&mut TYPE_OBJECT
fn type_object() -> std::ptr::NonNull<pyo3::ffi::PyTypeObject> {
use pyo3::type_object::LazyTypeObject;
static TYPE_OBJECT: LazyTypeObject = LazyTypeObject::new();
TYPE_OBJECT.get_pyclass_type::<Self>()
}
}

View file

@ -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<PyObject>,
) -> *mut ffi::PyTypeObject {
) -> NonNull<ffi::PyTypeObject> {
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)
}
}

View file

@ -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) }
}
}
);

View file

@ -74,7 +74,7 @@ where
{
unsafe fn alloc(_py: Python) -> *mut Self::ConcreteLayout {
if let Some(obj) = <Self as PyClassWithFreeList>::get_free_list().pop() {
ffi::PyObject_Init(obj, <Self as PyTypeInfo>::type_object());
ffi::PyObject_Init(obj, <Self as PyTypeInfo>::type_object().as_ptr() as *mut _);
obj as _
} else {
crate::pyclass::default_alloc::<Self>() as _
@ -90,7 +90,7 @@ where
}
if let Some(obj) = <Self as PyClassWithFreeList>::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),
}

View file

@ -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<T: PyTypeInfo>() -> *mut ffi::PyObject {
let tp_ptr = T::type_object();
let tp_ptr = T::type_object().as_ptr();
if T::FLAGS & type_flags::EXTENDED != 0
&& <T::BaseType as PyTypeInfo>::ConcreteLayout::IS_NATIVE_TYPE
{
let base_tp_ptr = <T::BaseType as PyTypeInfo>::type_object();
if let Some(base_new) = (*base_tp_ptr).tp_new {
let base_tp = <T::BaseType as PyTypeInfo>::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,29 +82,6 @@ pub trait PyClass:
type WeakRef: PyClassWeakRef;
}
unsafe impl<T> PyTypeObject for T
where
T: PyClass,
{
fn init_type() -> NonNull<ffi::PyTypeObject> {
<T::BaseType as PyTypeObject>::init_type();
let type_object = unsafe { <Self as PyTypeInfo>::type_object() };
if (type_object.tp_flags & ffi::Py_TPFLAGS_READY) == 0 {
// automatically initialize the class on-demand
let gil = Python::acquire_gil();
let py = gil.python();
initialize_type::<Self>(py, <Self as PyTypeInfo>::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.
///
@ -178,7 +155,6 @@ impl<T: PyClass> PyClassShell<T> {
<T::BaseType as PyTypeInfo>::ConcreteLayout:
crate::type_object::PyObjectSizedLayout<T::BaseType>,
{
T::init_type();
let base = T::alloc(py);
if base.is_null() {
return Err(PyErr::fetch(py));
@ -276,15 +252,19 @@ where
}
}
/// Register `T: PyClass` to Python interpreter.
#[cfg(not(Py_LIMITED_API))]
pub fn initialize_type<T>(py: Python, module_name: Option<&str>) -> PyResult<*mut ffi::PyTypeObject>
pub(crate) fn create_type_object<T>(
py: Python,
module_name: Option<&str>,
) -> PyResult<Box<ffi::PyTypeObject>>
where
T: PyClass,
{
let type_object: &mut ffi::PyTypeObject = unsafe { T::type_object() };
let base_type_object: &mut ffi::PyTypeObject =
unsafe { <T::BaseType as PyTypeInfo>::type_object() };
// Box (or some other heap allocation) is needed because PyType_Ready expects the type object
// to have a permanent memory address.
let mut boxed = Box::new(ffi::PyTypeObject_INIT);
let mut type_object = boxed.as_mut();
let base_type_object = <T::BaseType as PyTypeInfo>::type_object().as_ptr();
// PyPy will segfault if passed only a nul terminator as `tp_doc`.
// ptr::null() is OK though.
@ -391,7 +371,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()
}

View file

@ -1,10 +1,13 @@
// Copyright (c) 2017-present PyO3 Project and Contributors
//! Python type object information
use crate::err::PyResult;
use crate::instance::Py;
use crate::pyclass::{create_type_object, PyClass};
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<U>` represents that `T` is a concrete representaion of `U` in Python heap.
@ -60,7 +63,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 +92,81 @@ pub trait PyTypeInfo: Sized {
/// Initializer for layout
type Initializer: PyObjectInit<Self>;
/// PyTypeObject instance for this type, which might still need to
/// be initialized
unsafe fn type_object() -> &'static mut ffi::PyTypeObject;
/// PyTypeObject instance for this type, guaranteed to be global and initialized.
fn type_object() -> NonNull<ffi::PyTypeObject>;
/// 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<ffi::PyTypeObject>;
/// Returns the safe abstraction over the type object.
fn type_object() -> Py<PyType>;
}
/// Returns the safe abstraction over the type object from [PyTypeObject::init_type]
unsafe impl<T> PyTypeObject for T
where
T: PyTypeInfo,
{
fn type_object() -> Py<PyType> {
unsafe { Py::from_borrowed_ptr(Self::init_type().as_ptr() as *mut ffi::PyObject) }
unsafe { Py::from_borrowed_ptr(<Self as PyTypeInfo>::type_object().as_ptr() as *mut _) }
}
}
/// Type used to store static type objects
#[doc(hidden)]
pub struct LazyTypeObject {
cell: OnceCell<NonNull<ffi::PyTypeObject>>,
}
impl LazyTypeObject {
pub const fn new() -> Self {
Self {
cell: OnceCell::new(),
}
}
pub fn get_or_init<F>(&self, constructor: F) -> PyResult<NonNull<ffi::PyTypeObject>>
where
F: Fn() -> PyResult<NonNull<ffi::PyTypeObject>>,
{
Ok(*self.cell.get_or_try_init(constructor)?)
}
pub fn get_pyclass_type<T: PyClass>(&self) -> NonNull<ffi::PyTypeObject> {
self.get_or_init(|| {
// automatically initialize the class on-demand
let gil = Python::acquire_gil();
let py = gil.python();
let boxed = create_type_object::<T>(py, T::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 {}", T::NAME)
})
}
}
// 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 {}

View file

@ -114,7 +114,7 @@ macro_rules! pyobject_native_type_extract {
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;
@ -124,28 +124,17 @@ macro_rules! pyobject_native_type_convert(
const MODULE: Option<&'static str> = $module;
#[inline]
unsafe fn type_object() -> &'static mut $crate::ffi::PyTypeObject {
&mut $typeobject
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(
<Self as $crate::type_object::PyTypeInfo>::type_object() as *mut _
)
}
}
}
impl<$($type_param,)*> $crate::ToPyObject for $name
{
#[inline]

View file

@ -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<T: PyTypeInfo>() -> Py<PyType> {
unsafe { Py::from_borrowed_ptr(T::type_object() as *const _ as *mut ffi::PyObject) }
pub fn new<T: PyTypeObject>() -> Py<PyType> {
T::type_object()
}
/// Retrieves the underlying FFI pointer associated with this Python object.

View file

@ -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::<EmptyClassInModule>(py, Some("test_module.nested")).unwrap();
let module: String = ty.getattr("__module__").unwrap().extract().unwrap();
assert_eq!(module, "test_module.nested");
}

View file

@ -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::<PickleSupport>().unwrap();
add_module(py, module).unwrap();
initialize_type::<PickleSupport>(py, Some("test_module")).unwrap();
let inst = PyClassShell::new_ref(py, PickleSupport {}).unwrap();
py_run!(
py,