Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove static mut from PyTypeInfo implementation #751

Merged
merged 3 commits into from
Feb 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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
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
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> {
davidhewitt marked this conversation as resolved.
Show resolved Hide resolved
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
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
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
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
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);
davidhewitt marked this conversation as resolved.
Show resolved Hide resolved
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