Skip to content

Commit

Permalink
Merge pull request #751 from davidhewitt/remove-static-mut
Browse files Browse the repository at this point in the history 
Remove static mut from PyTypeInfo implementation
  • Loading branch information
@kngwyu
kngwyu authored Feb 3, 2020
2 parents d75f072 + 04f30c5 commit 8fea23a
Show file tree
Hide file tree
Showing 13 changed files with 143 additions and 117 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
9 changes: 5 additions & 4 deletions guide/src/class.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand All @@ -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>()
}
}

Expand Down
11 changes: 5 additions & 6 deletions pyo3-derive-backend/src/pyclass.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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>;
Expand All @@ -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>()
}
}

Expand Down
11 changes: 8 additions & 3 deletions src/err.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
Expand Down Expand Up @@ -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(),
Expand All @@ -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)
}
}

Expand Down
58 changes: 30 additions & 28 deletions src/exceptions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand All @@ -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) }
}
}
};
Expand Down Expand Up @@ -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) }
}
}
};
Expand Down Expand Up @@ -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) }
}
}
);
Expand Down
4 changes: 2 additions & 2 deletions src/freelist.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 _
Expand All @@ -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),
}
Expand Down
50 changes: 15 additions & 35 deletions src/pyclass.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
}
}
Expand Down Expand Up @@ -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),
}
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()
}
Expand Down
Loading

0 comments on commit 8fea23a

Please sign in to comment.