Skip to content

Commit 0b967d4

Browse files
davidhewittdavidhewitt
authored
use ffi::MemberGef for #[pyo3(get)] fields of Py<T> (#4254)
* use `ffi::MemberGef` for `#[pyo3(get)]` fields of `Py<T>` * tidy up implementation * make it work on MSRV :( * fix docs and newsfragment * clippy * internal docs and coverage * review: mejrs
1 parent 41fb957 commit 0b967d4

18 files changed

+498
-140
lines changed

‎newsfragments/4254.changed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Optimize code generated by `#[pyo3(get)]` on `#[pyclass]` fields.

‎pyo3-macros-backend/src/pyclass.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1415,12 +1415,14 @@ pub fn gen_complex_enum_variant_attr(
14151415
};
14161416

14171417
let method_def = quote! {
1418-
#pyo3_path::class::PyMethodDefType::ClassAttribute({
1419-
#pyo3_path::class::PyClassAttributeDef::new(
1420-
#python_name,
1421-
#cls_type::#wrapper_ident
1422-
)
1423-
})
1418+
#pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static(
1419+
#pyo3_path::class::PyMethodDefType::ClassAttribute({
1420+
#pyo3_path::class::PyClassAttributeDef::new(
1421+
#python_name,
1422+
#cls_type::#wrapper_ident
1423+
)
1424+
})
1425+
)
14241426
};
14251427

14261428
MethodAndMethodDef {

‎pyo3-macros-backend/src/pyimpl.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,12 +197,14 @@ pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec<'_>, ctx: &Ctx) -> MethodA
197197
};
198198

199199
let method_def = quote! {
200-
#pyo3_path::class::PyMethodDefType::ClassAttribute({
201-
#pyo3_path::class::PyClassAttributeDef::new(
202-
#python_name,
203-
#cls::#wrapper_ident
204-
)
205-
})
200+
#pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static(
201+
#pyo3_path::class::PyMethodDefType::ClassAttribute({
202+
#pyo3_path::class::PyClassAttributeDef::new(
203+
#python_name,
204+
#cls::#wrapper_ident
205+
)
206+
})
207+
)
206208
};
207209

208210
MethodAndMethodDef {

‎pyo3-macros-backend/src/pymethod.rs

Lines changed: 100 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,9 @@ pub fn impl_py_method_def(
330330
};
331331
let methoddef = spec.get_methoddef(quote! { #cls::#wrapper_ident }, doc, ctx);
332332
let method_def = quote! {
333-
#pyo3_path::class::PyMethodDefType::#methoddef_type(#methoddef #add_flags)
333+
#pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static(
334+
#pyo3_path::class::PyMethodDefType::#methoddef_type(#methoddef #add_flags)
335+
)
334336
};
335337
Ok(MethodAndMethodDef {
336338
associated_method,
@@ -511,12 +513,14 @@ fn impl_py_class_attribute(
511513
};
512514

513515
let method_def = quote! {
514-
#pyo3_path::class::PyMethodDefType::ClassAttribute({
515-
#pyo3_path::class::PyClassAttributeDef::new(
516-
#python_name,
517-
#cls::#wrapper_ident
518-
)
519-
})
516+
#pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static(
517+
#pyo3_path::class::PyMethodDefType::ClassAttribute({
518+
#pyo3_path::class::PyClassAttributeDef::new(
519+
#python_name,
520+
#cls::#wrapper_ident
521+
)
522+
})
523+
)
520524
};
521525

522526
Ok(MethodAndMethodDef {
@@ -701,11 +705,13 @@ pub fn impl_py_setter_def(
701705

702706
let method_def = quote! {
703707
#cfg_attrs
704-
#pyo3_path::class::PyMethodDefType::Setter(
705-
#pyo3_path::class::PySetterDef::new(
706-
#python_name,
707-
#cls::#wrapper_ident,
708-
#doc
708+
#pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static(
709+
#pyo3_path::class::PyMethodDefType::Setter(
710+
#pyo3_path::class::PySetterDef::new(
711+
#python_name,
712+
#cls::#wrapper_ident,
713+
#doc
714+
)
709715
)
710716
)
711717
};
@@ -750,102 +756,107 @@ pub fn impl_py_getter_def(
750756
let python_name = property_type.null_terminated_python_name(ctx)?;
751757
let doc = property_type.doc(ctx);
752758

759+
let mut cfg_attrs = TokenStream::new();
760+
if let PropertyType::Descriptor { field, .. } = &property_type {
761+
for attr in field
762+
.attrs
763+
.iter()
764+
.filter(|attr| attr.path().is_ident("cfg"))
765+
{
766+
attr.to_tokens(&mut cfg_attrs);
767+
}
768+
}
769+
753770
let mut holders = Holders::new();
754-
let body = match property_type {
771+
match property_type {
755772
PropertyType::Descriptor {
756773
field_index, field, ..
757774
} => {
758-
let slf = SelfType::Receiver {
759-
mutable: false,
760-
span: Span::call_site(),
761-
}
762-
.receiver(cls, ExtractErrorMode::Raise, &mut holders, ctx);
763-
let field_token = if let Some(ident) = &field.ident {
764-
// named struct field
775+
let ty = &field.ty;
776+
let field = if let Some(ident) = &field.ident {
765777
ident.to_token_stream()
766778
} else {
767-
// tuple struct field
768779
syn::Index::from(field_index).to_token_stream()
769780
};
770-
quotes::map_result_into_ptr(
771-
quotes::ok_wrap(
772-
quote! {
773-
::std::clone::Clone::clone(&(#slf.#field_token))
774-
},
775-
ctx,
776-
),
777-
ctx,
778-
)
781+
782+
// TODO: on MSRV 1.77+, we can use `::std::mem::offset_of!` here, and it should
783+
// make it possible for the `MaybeRuntimePyMethodDef` to be a `Static` variant.
784+
let method_def = quote_spanned! {ty.span()=>
785+
#cfg_attrs
786+
{
787+
#[allow(unused_imports)] // might not be used if all probes are positve
788+
use #pyo3_path::impl_::pyclass::Probe;
789+
790+
struct Offset;
791+
unsafe impl #pyo3_path::impl_::pyclass::OffsetCalculator<#cls, #ty> for Offset {
792+
fn offset() -> usize {
793+
#pyo3_path::impl_::pyclass::class_offset::<#cls>() +
794+
#pyo3_path::impl_::pyclass::offset_of!(#cls, #field)
795+
}
796+
}
797+
798+
const GENERATOR: #pyo3_path::impl_::pyclass::PyClassGetterGenerator::<
799+
#cls,
800+
#ty,
801+
Offset,
802+
{ #pyo3_path::impl_::pyclass::IsPyT::<#ty>::VALUE },
803+
{ #pyo3_path::impl_::pyclass::IsToPyObject::<#ty>::VALUE },
804+
> = unsafe { #pyo3_path::impl_::pyclass::PyClassGetterGenerator::new() };
805+
#pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Runtime(
806+
|| GENERATOR.generate(#python_name, #doc)
807+
)
808+
}
809+
};
810+
811+
Ok(MethodAndMethodDef {
812+
associated_method: quote! {},
813+
method_def,
814+
})
779815
}
780816
// Forward to `IntoPyCallbackOutput`, to handle `#[getter]`s returning results.
781817
PropertyType::Function {
782818
spec, self_type, ..
783819
} => {
820+
let wrapper_ident = format_ident!("__pymethod_get_{}__", spec.name);
784821
let call = impl_call_getter(cls, spec, self_type, &mut holders, ctx)?;
785-
quote! {
822+
let body = quote! {
786823
#pyo3_path::callback::convert(py, #call)
787-
}
788-
}
789-
};
824+
};
790825

791-
let wrapper_ident = match property_type {
792-
PropertyType::Descriptor {
793-
field: syn::Field {
794-
ident: Some(ident), ..
795-
},
796-
..
797-
} => {
798-
format_ident!("__pymethod_get_{}__", ident)
799-
}
800-
PropertyType::Descriptor { field_index, .. } => {
801-
format_ident!("__pymethod_get_field_{}__", field_index)
802-
}
803-
PropertyType::Function { spec, .. } => {
804-
format_ident!("__pymethod_get_{}__", spec.name)
805-
}
806-
};
826+
let init_holders = holders.init_holders(ctx);
827+
let check_gil_refs = holders.check_gil_refs();
828+
let associated_method = quote! {
829+
#cfg_attrs
830+
unsafe fn #wrapper_ident(
831+
py: #pyo3_path::Python<'_>,
832+
_slf: *mut #pyo3_path::ffi::PyObject
833+
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
834+
#init_holders
835+
let result = #body;
836+
#check_gil_refs
837+
result
838+
}
839+
};
807840

808-
let mut cfg_attrs = TokenStream::new();
809-
if let PropertyType::Descriptor { field, .. } = &property_type {
810-
for attr in field
811-
.attrs
812-
.iter()
813-
.filter(|attr| attr.path().is_ident("cfg"))
814-
{
815-
attr.to_tokens(&mut cfg_attrs);
816-
}
817-
}
841+
let method_def = quote! {
842+
#cfg_attrs
843+
#pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Static(
844+
#pyo3_path::class::PyMethodDefType::Getter(
845+
#pyo3_path::class::PyGetterDef::new(
846+
#python_name,
847+
#cls::#wrapper_ident,
848+
#doc
849+
)
850+
)
851+
)
852+
};
818853

819-
let init_holders = holders.init_holders(ctx);
820-
let check_gil_refs = holders.check_gil_refs();
821-
let associated_method = quote! {
822-
#cfg_attrs
823-
unsafe fn #wrapper_ident(
824-
py: #pyo3_path::Python<'_>,
825-
_slf: *mut #pyo3_path::ffi::PyObject
826-
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
827-
#init_holders
828-
let result = #body;
829-
#check_gil_refs
830-
result
854+
Ok(MethodAndMethodDef {
855+
associated_method,
856+
method_def,
857+
})
831858
}
832-
};
833-
834-
let method_def = quote! {
835-
#cfg_attrs
836-
#pyo3_path::class::PyMethodDefType::Getter(
837-
#pyo3_path::class::PyGetterDef::new(
838-
#python_name,
839-
#cls::#wrapper_ident,
840-
#doc
841-
)
842-
)
843-
};
844-
845-
Ok(MethodAndMethodDef {
846-
associated_method,
847-
method_def,
848-
})
859+
}
849860
}
850861

851862
/// Split an argument of pyo3::Python from the front of the arg list, if present

0 commit comments

Comments
 (0)