diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 3e40977e4af..1e70f387d75 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1415,12 +1415,14 @@ pub fn gen_complex_enum_variant_attr( }; let method_def = quote! { - #pyo3_path::class::PyMethodDefType::ClassAttribute({ - #pyo3_path::class::PyClassAttributeDef::new( - #python_name, - #cls_type::#wrapper_ident - ) - }) + #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static( + #pyo3_path::class::PyMethodDefType::ClassAttribute({ + #pyo3_path::class::PyClassAttributeDef::new( + #python_name, + #cls_type::#wrapper_ident + ) + }) + ) }; MethodAndMethodDef { diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs index a1242d49f10..c131fc05131 100644 --- a/pyo3-macros-backend/src/pyimpl.rs +++ b/pyo3-macros-backend/src/pyimpl.rs @@ -197,12 +197,14 @@ pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec<'_>, ctx: &Ctx) -> MethodA }; let method_def = quote! { - #pyo3_path::class::PyMethodDefType::ClassAttribute({ - #pyo3_path::class::PyClassAttributeDef::new( - #python_name, - #cls::#wrapper_ident - ) - }) + #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static( + #pyo3_path::class::PyMethodDefType::ClassAttribute({ + #pyo3_path::class::PyClassAttributeDef::new( + #python_name, + #cls::#wrapper_ident + ) + }) + ) }; MethodAndMethodDef { diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 06db0f8d410..34185248849 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -329,7 +329,9 @@ pub fn impl_py_method_def( }; let methoddef = spec.get_methoddef(quote! { #cls::#wrapper_ident }, doc, ctx); let method_def = quote! { - #pyo3_path::class::PyMethodDefType::#methoddef_type(#methoddef #add_flags) + #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static( + #pyo3_path::class::PyMethodDefType::#methoddef_type(#methoddef #add_flags) + ) }; Ok(MethodAndMethodDef { associated_method, @@ -510,12 +512,14 @@ fn impl_py_class_attribute( }; let method_def = quote! { - #pyo3_path::class::PyMethodDefType::ClassAttribute({ - #pyo3_path::class::PyClassAttributeDef::new( - #python_name, - #cls::#wrapper_ident - ) - }) + #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static( + #pyo3_path::class::PyMethodDefType::ClassAttribute({ + #pyo3_path::class::PyClassAttributeDef::new( + #python_name, + #cls::#wrapper_ident + ) + }) + ) }; Ok(MethodAndMethodDef { @@ -700,11 +704,13 @@ pub fn impl_py_setter_def( let method_def = quote! { #cfg_attrs - #pyo3_path::class::PyMethodDefType::Setter( - #pyo3_path::class::PySetterDef::new( - #python_name, - #cls::#wrapper_ident, - #doc + #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static( + #pyo3_path::class::PyMethodDefType::Setter( + #pyo3_path::class::PySetterDef::new( + #python_name, + #cls::#wrapper_ident, + #doc + ) ) ) }; @@ -776,15 +782,25 @@ pub fn impl_py_getter_def( #cfg_attrs { use #pyo3_path::impl_::pyclass::Tester; - const OFFSET: usize = ::std::mem::offset_of!(#cls, #field); + + struct Offset; + unsafe impl #pyo3_path::impl_::pyclass::OffsetCalculator<#cls, #ty> for Offset { + fn offset() -> usize { + #pyo3_path::impl_::pyclass::class_offset::<#cls>() + + #pyo3_path::impl_::pyclass::offset_of!(#cls, #field) + } + } + const GENERATOR: #pyo3_path::impl_::pyclass::PyClassGetterGenerator::< #cls, #ty, - OFFSET, + Offset, { #pyo3_path::impl_::pyclass::IsPyT::<#ty>::VALUE }, { #pyo3_path::impl_::pyclass::IsToPyObject::<#ty>::VALUE }, > = unsafe { #pyo3_path::impl_::pyclass::PyClassGetterGenerator::new() }; - GENERATOR.generate(#python_name, #doc) + #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Runtime( + || GENERATOR.generate(#python_name, #doc) + ) } }; @@ -820,11 +836,13 @@ pub fn impl_py_getter_def( let method_def = quote! { #cfg_attrs - #pyo3_path::class::PyMethodDefType::Getter( - #pyo3_path::class::PyGetterDef::new( - #python_name, - #cls::#wrapper_ident, - #doc + #pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static( + #pyo3_path::class::PyMethodDefType::Getter( + #pyo3_path::class::PyGetterDef::new( + #python_name, + #cls::#wrapper_ident, + #doc + ) ) ) }; diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 3c682f28a72..01981e4630d 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -131,8 +131,15 @@ impl Clone for PyClassImplCollector { impl Copy for PyClassImplCollector {} +pub enum MaybeRuntimePyMethodDef { + /// Used in cases where const functionality is not sufficient to define the method + /// purely at compile time. + Runtime(fn() -> PyMethodDefType), + Static(PyMethodDefType), +} + pub struct PyClassItems { - pub methods: &'static [PyMethodDefType], + pub methods: &'static [MaybeRuntimePyMethodDef], pub slots: &'static [ffi::PyType_Slot], } @@ -1172,6 +1179,23 @@ pub(crate) unsafe extern "C" fn assign_sequence_item_from_mapping( result } +// Below MSRV 1.77 we can't use `std::mem::offset_of!`, and the replacement in +// `memoffset::offset_of` doesn't work in const contexts for types containing `UnsafeCell`. +pub unsafe trait OffsetCalculator { + /// Offset to the field within a PyClassObject, in bytes. + /// + /// The trait is unsafe to implement because producing an incorrect offset will lead to UB. + fn offset() -> usize; +} + +// Used in generated implementations of OffsetCalculator +pub fn class_offset() -> usize { + offset_of!(PyClassObject, contents) +} + +// Used in generated implementations of OffsetCalculator +pub use memoffset::offset_of; + /// Type which uses specialization on impl blocks to determine how to read a field from a Rust pyclass /// as part of a `#[pyo3(get)]` annotation. pub struct PyClassGetterGenerator< @@ -1179,51 +1203,54 @@ pub struct PyClassGetterGenerator< // class struct ClassT: PyClass, FieldT, - const OFFSET: usize, + Offset: OffsetCalculator, // on Rust 1.77+ this could be a const OFFSET: usize // additional metadata about the field which is used to switch between different implementations // at compile time const IS_PY_T: bool, const IMPLEMENTS_TOPYOBJECT: bool, ->(PhantomData, PhantomData); +>(PhantomData<(ClassT, FieldT, Offset)>); impl< ClassT: PyClass, FieldT, - const OFFSET: usize, + Offset: OffsetCalculator, const IS_PY_T: bool, const IMPLEMENTS_TOPYOBJECT: bool, - > PyClassGetterGenerator + > PyClassGetterGenerator { - /// Safety: constructing this type requires that there exists a value of type X - /// at offset OFFSET within the type T. + /// Safety: constructing this type requires that there exists a value of type FieldT + /// at the calculated offset within the type ClassT. pub const unsafe fn new() -> Self { - Self(PhantomData, PhantomData) + Self(PhantomData) } } -impl - PyClassGetterGenerator, OFFSET, true, IMPLEMENTS_TOPYOBJECT> +impl< + ClassT: PyClass, + U, + Offset: OffsetCalculator>, + const IMPLEMENTS_TOPYOBJECT: bool, + > PyClassGetterGenerator, Offset, true, IMPLEMENTS_TOPYOBJECT> { /// Py fields have a potential optimization to use Python's "struct members" to read /// the field directly from the struct, rather than using a getter function. /// /// This is the most efficient operation the Python interpreter could possibly do to /// read a field, but it's only possible for us to allow this for frozen classes. - pub const fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType { + pub fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType { use crate::pyclass::boolean_struct::private::Boolean; if ClassT::Frozen::VALUE { PyMethodDefType::StructMember(ffi::PyMemberDef { name: name.as_ptr(), type_code: ffi::Py_T_OBJECT_EX, - offset: (std::mem::offset_of!(PyClassObject::, contents) + OFFSET) - as ffi::Py_ssize_t, + offset: Offset::offset() as ffi::Py_ssize_t, flags: ffi::Py_READONLY, doc: doc.as_ptr(), }) } else { PyMethodDefType::Getter(crate::PyGetterDef { name, - meth: pyo3_get_value_topyobject::, OFFSET>, + meth: pyo3_get_value_topyobject::, Offset>, doc, }) } @@ -1231,14 +1258,13 @@ impl } /// Field is not Py; try to use `ToPyObject` to avoid potentially expensive clones of containers like `Vec` -impl - PyClassGetterGenerator +impl> + PyClassGetterGenerator { pub const fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType { PyMethodDefType::Getter(crate::PyGetterDef { - // TODO: store &CStr in PyGetterDef etc name, - meth: pyo3_get_value_topyobject::, + meth: pyo3_get_value_topyobject::, doc, }) } @@ -1257,8 +1283,8 @@ pub trait PyO3GetField: IntoPy> + Clone {} impl> + Clone> PyO3GetField for T {} /// Base case attempts to use IntoPy + Clone, which was the only behaviour before PyO3 0.22. -impl - PyClassGetterGenerator +impl> + PyClassGetterGenerator { pub const fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType // The bound goes here rather than on the block so that this impl is always available @@ -1267,9 +1293,8 @@ impl FieldT: PyO3GetField, { PyMethodDefType::Getter(crate::PyGetterDef { - // TODO: store &CStr in PyGetterDef etc name, - meth: pyo3_get_value::, + meth: pyo3_get_value::, doc, }) } @@ -1307,7 +1332,11 @@ impl IsToPyObject { pub const VALUE: bool = true; } -fn pyo3_get_value_topyobject( +fn pyo3_get_value_topyobject< + ClassT: PyClass, + FieldT: ToPyObject, + Offset: OffsetCalculator, +>( py: Python<'_>, obj: *mut ffi::PyObject, ) -> PyResult<*mut ffi::PyObject> { @@ -1318,18 +1347,18 @@ fn pyo3_get_value_topyobject() - .offset((std::mem::offset_of!(PyClassObject::, contents) + OFFSET) as isize) - .cast::() - }; + let value = unsafe { obj.cast::().add(Offset::offset()).cast::() }; - // SAFETY: OFFSET is known to describe the location of the value, and + // SAFETY: Offset is known to describe the location of the value, and // _holder is preventing mutable aliasing Ok((unsafe { &*value }).to_object(py).into_ptr()) } -fn pyo3_get_value> + Clone, const OFFSET: usize>( +fn pyo3_get_value< + ClassT: PyClass, + FieldT: IntoPy> + Clone, + Offset: OffsetCalculator, +>( py: Python<'_>, obj: *mut ffi::PyObject, ) -> PyResult<*mut ffi::PyObject> { @@ -1340,13 +1369,9 @@ fn pyo3_get_value> + Clone, const OFFS .try_borrow()? }; - let value = unsafe { - obj.cast::() - .offset((std::mem::offset_of!(PyClassObject::, contents) + OFFSET) as isize) - .cast::() - }; + let value = unsafe { obj.cast::().add(Offset::offset()).cast::() }; - // SAFETY: OFFSET is known to describe the location of the value, and + // 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()) } diff --git a/src/impl_/pyclass/lazy_type_object.rs b/src/impl_/pyclass/lazy_type_object.rs index 7afaec8a99b..08a5f17d4bd 100644 --- a/src/impl_/pyclass/lazy_type_object.rs +++ b/src/impl_/pyclass/lazy_type_object.rs @@ -8,6 +8,7 @@ use std::{ use crate::{ exceptions::PyRuntimeError, ffi, + impl_::pyclass::MaybeRuntimePyMethodDef, pyclass::{create_type_object, PyClassTypeObject}, sync::{GILOnceCell, GILProtected}, types::PyType, @@ -149,7 +150,15 @@ impl LazyTypeObjectInner { let mut items = vec![]; for class_items in items_iter { for def in class_items.methods { - if let PyMethodDefType::ClassAttribute(attr) = def { + let built_method; + let method = match def { + MaybeRuntimePyMethodDef::Runtime(builder) => { + built_method = builder(); + &built_method + } + MaybeRuntimePyMethodDef::Static(method) => method, + }; + if let PyMethodDefType::ClassAttribute(attr) = method { match (attr.meth)(py) { Ok(val) => items.push((attr.name, val)), Err(err) => { diff --git a/src/pyclass/create_type_object.rs b/src/pyclass/create_type_object.rs index cdc25f6e433..00dc7ee36f8 100644 --- a/src/pyclass/create_type_object.rs +++ b/src/pyclass/create_type_object.rs @@ -5,7 +5,7 @@ use crate::{ pycell::PyClassObject, pyclass::{ assign_sequence_item_from_mapping, get_sequence_item_from_mapping, tp_dealloc, - tp_dealloc_with_gc, PyClassItemsIter, + tp_dealloc_with_gc, MaybeRuntimePyMethodDef, PyClassItemsIter, }, pymethods::{Getter, Setter}, trampoline::trampoline, @@ -317,6 +317,14 @@ impl PyTypeBuilder { self.push_slot(slot.slot, slot.pfunc); } for method in items.methods { + let built_method; + let method = match method { + MaybeRuntimePyMethodDef::Runtime(builder) => { + built_method = builder(); + &built_method + } + MaybeRuntimePyMethodDef::Static(method) => method, + }; self.pymethod_def(method); } } diff --git a/tests/ui/invalid_property_args.stderr b/tests/ui/invalid_property_args.stderr index 201a0d5fa88..64f5b62d57c 100644 --- a/tests/ui/invalid_property_args.stderr +++ b/tests/ui/invalid_property_args.stderr @@ -56,11 +56,11 @@ error[E0277]: `PhantomData` cannot be converted to a Python object = note: `Py` fields are always converible to Python objects = note: implement `ToPyObject` or `IntoPy + Clone` for `PhantomData` to define the conversion = note: required for `PhantomData` to implement `PyO3GetField` -note: required by a bound in `PyClassGetterGenerator::::generate` +note: required by a bound in `PyClassGetterGenerator::::generate` --> src/impl_/pyclass.rs | | pub const fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType | -------- required by a bound in this associated function ... | FieldT: PyO3GetField, - | ^^^^^^^^^^^^ required by this bound in `PyClassGetterGenerator::::generate` + | ^^^^^^^^^^^^ required by this bound in `PyClassGetterGenerator::::generate`