Skip to content

Commit

Permalink
use ffi::MemberGef for #[pyo3(get)] fields of Py<T>
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Jun 15, 2024
1 parent 5749a08 commit dda52e2
Show file tree
Hide file tree
Showing 11 changed files with 232 additions and 111 deletions.
147 changes: 70 additions & 77 deletions pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -749,102 +749,95 @@ pub fn impl_py_getter_def(
let python_name = property_type.null_terminated_python_name()?;
let doc = property_type.doc();

let mut cfg_attrs = TokenStream::new();
if let PropertyType::Descriptor { field, .. } = &property_type {
for attr in field
.attrs
.iter()
.filter(|attr| attr.path().is_ident("cfg"))
{
attr.to_tokens(&mut cfg_attrs);
}
}

let mut holders = Holders::new();
let body = match property_type {
match property_type {
PropertyType::Descriptor {
field_index, field, ..
} => {
let slf = SelfType::Receiver {
mutable: false,
span: Span::call_site(),
}
.receiver(cls, ExtractErrorMode::Raise, &mut holders, ctx);
let field_token = if let Some(ident) = &field.ident {
// named struct field
let ty = &field.ty;
let field = if let Some(ident) = &field.ident {
ident.to_token_stream()
} else {
// tuple struct field
syn::Index::from(field_index).to_token_stream()
};
quotes::map_result_into_ptr(
quotes::ok_wrap(
quote! {
::std::clone::Clone::clone(&(#slf.#field_token))
},
ctx,
),
ctx,
)

let method_def = quote! {
#cfg_attrs
{
const OFFSET: usize = ::std::mem::offset_of!(#cls, #field);
unsafe { #pyo3_path::impl_::pyclass::PyClassGetterGenerator::<
#cls,
#ty,
OFFSET,
{
use #pyo3_path::impl_::pyclass::Tester;
#pyo3_path::impl_::pyclass::IsPyT::<#ty>::VALUE
}
>::new() }.generate(
unsafe { ::std::ffi::CStr::from_bytes_with_nul_unchecked(#python_name.as_bytes()) },
unsafe { ::std::ffi::CStr::from_bytes_with_nul_unchecked(#doc.as_bytes()) },
)
}
};

Ok(MethodAndMethodDef {
associated_method: quote! {},
method_def,
})
}
// Forward to `IntoPyCallbackOutput`, to handle `#[getter]`s returning results.
PropertyType::Function {
spec, self_type, ..
} => {
let wrapper_ident = format_ident!("__pymethod_get_{}__", spec.name);
let call = impl_call_getter(cls, spec, self_type, &mut holders, ctx)?;
quote! {
let body = quote! {
#pyo3_path::callback::convert(py, #call)
}
}
};
};

let wrapper_ident = match property_type {
PropertyType::Descriptor {
field: syn::Field {
ident: Some(ident), ..
},
..
} => {
format_ident!("__pymethod_get_{}__", ident)
}
PropertyType::Descriptor { field_index, .. } => {
format_ident!("__pymethod_get_field_{}__", field_index)
}
PropertyType::Function { spec, .. } => {
format_ident!("__pymethod_get_{}__", spec.name)
}
};
let init_holders = holders.init_holders(ctx);
let check_gil_refs = holders.check_gil_refs();
let associated_method = quote! {
#cfg_attrs
unsafe fn #wrapper_ident(
py: #pyo3_path::Python<'_>,
_slf: *mut #pyo3_path::ffi::PyObject
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
#init_holders
let result = #body;
#check_gil_refs
result
}
};

let mut cfg_attrs = TokenStream::new();
if let PropertyType::Descriptor { field, .. } = &property_type {
for attr in field
.attrs
.iter()
.filter(|attr| attr.path().is_ident("cfg"))
{
attr.to_tokens(&mut cfg_attrs);
}
}
let method_def = quote! {
#cfg_attrs
#pyo3_path::class::PyMethodDefType::Getter(
#pyo3_path::class::PyGetterDef::new(
#python_name,
#cls::#wrapper_ident,
#doc
)
)
};

let init_holders = holders.init_holders(ctx);
let check_gil_refs = holders.check_gil_refs();
let associated_method = quote! {
#cfg_attrs
unsafe fn #wrapper_ident(
py: #pyo3_path::Python<'_>,
_slf: *mut #pyo3_path::ffi::PyObject
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
#init_holders
let result = #body;
#check_gil_refs
result
Ok(MethodAndMethodDef {
associated_method,
method_def,
})
}
};

let method_def = quote! {
#cfg_attrs
#pyo3_path::class::PyMethodDefType::Getter(
#pyo3_path::class::PyGetterDef::new(
#python_name,
#cls::#wrapper_ident,
#doc
)
)
};

Ok(MethodAndMethodDef {
associated_method,
method_def,
})
}
}

/// Split an argument of pyo3::Python from the front of the arg list, if present
Expand Down
144 changes: 138 additions & 6 deletions src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ use crate::PyNativeType;
use crate::{
exceptions::{PyAttributeError, PyNotImplementedError, PyRuntimeError, PyValueError},
ffi,
impl_::freelist::FreeList,
impl_::pycell::{GetBorrowChecker, PyClassMutability, PyClassObjectLayout},
impl_::{
freelist::FreeList,
pycell::{GetBorrowChecker, PyClassMutability, PyClassObjectLayout},
},
internal_tricks::extract_c_string,
pyclass_init::PyObjectInit,
types::any::PyAnyMethods,
types::PyBool,
Borrowed, Py, PyAny, PyClass, PyErr, PyMethodDefType, PyResult, PyTypeInfo, Python,
types::{any::PyAnyMethods, PyBool},
Borrowed, IntoPy, Py, PyAny, PyClass, PyErr, PyMethodDefType, PyResult, PyTypeInfo, Python,
ToPyObject,
};
use std::{
borrow::Cow,
Expand Down Expand Up @@ -891,7 +893,7 @@ macro_rules! generate_pyclass_richcompare_slot {
}
pub use generate_pyclass_richcompare_slot;

use super::pycell::PyClassObject;
use super::{pycell::PyClassObject, pymethods::BoundRef};

/// Implements a freelist.
///
Expand Down Expand Up @@ -1172,3 +1174,133 @@ pub(crate) unsafe extern "C" fn assign_sequence_item_from_mapping(
ffi::Py_DECREF(index);
result
}

/// Type which uses deref specialization to determine how to read a value from a Rust pyclass
/// as part of a `#[pyo3(get)]` annotation.
pub struct PyClassGetterGenerator<T: PyClass, X, const OFFSET: usize, const IS_PY_T: bool>(
PhantomData<T>,
PhantomData<X>,
);

impl<T: PyClass, X, const OFFSET: usize, const IS_PY_T: bool>
PyClassGetterGenerator<T, X, OFFSET, IS_PY_T>
{
/// Safety: constructing this type requires that there exists a value of type X
/// at offset OFFSET within the type T.
pub const unsafe fn new() -> Self {
Self(PhantomData, PhantomData)
}
}

impl<T: PyClass, X, const OFFSET: usize> PyClassGetterGenerator<T, Py<X>, OFFSET, true> {
/// Base case attempts to use IntoPy + Clone, which was the only behaviour before PyO3 0.22.
pub const fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType {
use crate::pyclass::boolean_struct::private::Boolean;
if T::Frozen::VALUE {
PyMethodDefType::StructMember(ffi::PyMemberDef {
name: name.as_ptr(),
type_code: ffi::Py_T_OBJECT_EX,
offset: (std::mem::offset_of!(PyClassObject::<T>, contents) + OFFSET)
as ffi::Py_ssize_t,
flags: ffi::Py_READONLY,
doc: doc.as_ptr(),
})
} else {
PyMethodDefType::Getter(crate::PyGetterDef {
// TODO: store &CStr in PyGetterDef etc
name: unsafe { std::str::from_utf8_unchecked(name.to_bytes_with_nul()) },
meth: pyo3_get_py_t::<T, Py<X>, OFFSET>,
doc: unsafe { std::str::from_utf8_unchecked(doc.to_bytes_with_nul()) },
})
}
}
}

impl<T: PyClass, X: IntoPy<Py<PyAny>> + Clone, const OFFSET: usize>
PyClassGetterGenerator<T, X, OFFSET, false>
{
/// Base case attempts to use IntoPy + Clone, which was the only behaviour before PyO3 0.22.
pub const fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType {
PyMethodDefType::Getter(crate::PyGetterDef {
// TODO: store &CStr in PyGetterDef etc
name: unsafe { std::str::from_utf8_unchecked(name.to_bytes_with_nul()) },
meth: pyo3_get_value::<T, X, OFFSET>,
doc: unsafe { std::str::from_utf8_unchecked(doc.to_bytes_with_nul()) },
})
}
}

pub trait Tester {
const VALUE: bool = false;
}

macro_rules! tester {
($name:ident) => {
pub struct $name<T>(PhantomData<T>);
impl<T> Tester for $name<T> {}
};
}

tester!(IsPyT);

impl<T> IsPyT<Py<T>> {
pub const VALUE: bool = true;
}

macro_rules! trait_tester {
($name:ident, $($trait:tt)+) => {
tester!($name);

impl<T: $($trait)+> $name<T> {
pub const VALUE: bool = true;
}
}
}

trait_tester!(IsIntoPyAndClone, IntoPy<Py<PyAny>> + Clone);
trait_tester!(IsToPyObject, ToPyObject);

fn pyo3_get_py_t<T: PyClass, U, const OFFSET: usize>(
py: Python<'_>,
obj: *mut ffi::PyObject,
) -> PyResult<*mut ffi::PyObject> {
// Check for mutable aliasing
let _holder = unsafe {
BoundRef::ref_from_ptr(py, &obj)
.downcast_unchecked::<T>()
.try_borrow()?
};

let value = unsafe {
obj.cast::<u8>()
.offset((std::mem::offset_of!(PyClassObject::<T>, contents) + OFFSET) as isize)
.cast::<Py<U>>()
};

// SAFETY: OFFSET is known to describe the location of the value, and
// _holder is preventing mutable aliasing
Ok((unsafe { &*value }).clone_ref(py).into_py(py).into_ptr())
}

fn pyo3_get_value<T: PyClass, X: IntoPy<Py<PyAny>> + Clone, const OFFSET: usize>(
py: Python<'_>,
obj: *mut ffi::PyObject,
) -> PyResult<*mut ffi::PyObject> {
assert!(IsIntoPyAndClone::<X>::VALUE);
// Check for mutable aliasing
let _holder = unsafe {
BoundRef::ref_from_ptr(py, &obj)
.downcast_unchecked::<T>()
.try_borrow()?
};

let value = unsafe {
obj.cast::<u8>()
.offset((std::mem::offset_of!(PyClassObject::<T>, contents) + OFFSET) as isize)
.cast::<X>()
};

// SAFETY: OFFSET is known to describe the location of the value, and
// _holder is preventing mutable aliasing
Ok((unsafe { &*value }).clone().into_py(py).into_ptr())
}
2 changes: 2 additions & 0 deletions src/impl_/pymethods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ pub enum PyMethodDefType {
Getter(PyGetterDef),
/// Represents setter descriptor, used by `#[setter]`
Setter(PySetterDef),
/// Represents a struct member
StructMember(ffi::PyMemberDef),
}

#[derive(Copy, Clone, Debug)]
Expand Down
2 changes: 1 addition & 1 deletion src/pycell/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ where
#[repr(C)]
pub struct PyClassObject<T: PyClassImpl> {
pub(crate) ob_base: <T::BaseType as PyClassBaseType>::LayoutAsBase,
contents: PyClassObjectContents<T>,
pub(crate) contents: PyClassObjectContents<T>,
}

#[repr(C)]
Expand Down
12 changes: 9 additions & 3 deletions src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,16 @@ pub mod boolean_struct {
use super::*;

/// A way to "seal" the boolean traits.
pub trait Boolean {}
pub trait Boolean {
const VALUE: bool;
}

impl Boolean for True {}
impl Boolean for False {}
impl Boolean for True {
const VALUE: bool = true;
}
impl Boolean for False {
const VALUE: bool = false;
}
}

pub struct True(());
Expand Down
Loading

0 comments on commit dda52e2

Please sign in to comment.