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.

Sign up for GitHub

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

Remove static mut from PyTypeInfo implementation #751

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5cbdef6
Remove static mut from PyTypeInfo implementation
davidhewitt Jan 30, 2020
7531b9f
Remove even more uses of `static mut`
davidhewitt Jan 30, 2020
04f30c5
Revisions from PR#751
davidhewitt Feb 3, 2020
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
View file
Open in desktop
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
Open in desktop
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
22 changes: 18 additions & 4 deletions guide/src/class.md
View file
Open in desktop
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,23 @@ 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 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::<Self>(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)
})
}
}

Expand Down
24 changes: 18 additions & 6 deletions pyo3-derive-backend/src/pyclass.rs
View file
Open in desktop
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,23 @@ 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 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::<Self>(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)
})
}
}

Expand Down
11 changes: 8 additions & 3 deletions src/err.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
48 changes: 13 additions & 35 deletions src/pyclass.rs
View file
Open in desktop
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,17 @@ 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 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() };
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 +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()
}
Expand Down
Loading