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

use ffi::MemberGef for #[pyo3(get)] fields of Py<T> #4254

Merged
merged 8 commits into from
Jun 22, 2024
Merged

use ffi::MemberGef for #[pyo3(get)] fields of Py<T> #4254

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dda52e2
use `ffi::MemberGef` for `#[pyo3(get)]` fields of `Py<T>`
davidhewitt Jun 15, 2024
b68c2b3
Merge branch 'main' into py-struct-member
davidhewitt Jun 18, 2024
9ea2842
tidy up implementation
davidhewitt Jun 18, 2024
547e18f
make it work on MSRV :(
davidhewitt Jun 19, 2024
7aa721b
fix docs and newsfragment
davidhewitt Jun 20, 2024
fec85ec
clippy
davidhewitt Jun 20, 2024
49aad6c
internal docs and coverage
davidhewitt Jun 20, 2024
b862d1b
review: mejrs
davidhewitt Jun 21, 2024
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 newsfragments/4254.changed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Optimize code generated by `#[pyo3(get)]` on `#[pyclass]` fields.
14 changes: 8 additions & 6 deletions pyo3-macros-backend/src/pyclass.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 8 additions & 6 deletions pyo3-macros-backend/src/pyimpl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
188 changes: 99 additions & 89 deletions pyo3-macros-backend/src/pymethod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
)
)
)
};
Expand Down Expand Up @@ -749,102 +755,106 @@ pub fn impl_py_getter_def(
let python_name = property_type.null_terminated_python_name(ctx)?;
let doc = property_type.doc(ctx);

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,
)

// TODO: on MSRV 1.77+, we can use `::std::mem::offset_of!` here, and it should
// make it possible for the `MaybeRuntimePyMethodDef` to be a `Static` variant.
let method_def = quote_spanned! {ty.span()=>
#cfg_attrs
{
use #pyo3_path::impl_::pyclass::Tester;

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,
{ #pyo3_path::impl_::pyclass::IsPyT::<#ty>::VALUE },
{ #pyo3_path::impl_::pyclass::IsToPyObject::<#ty>::VALUE },
> = unsafe { #pyo3_path::impl_::pyclass::PyClassGetterGenerator::new() };
#pyo3_path::impl_::pyclass::MaybeRuntimePyMethodDef::Runtime(
|| GENERATOR.generate(#python_name, #doc)
)
}
};

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::impl_::pyclass::MaybeRuntimePyMethodDef::Static(
#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
Loading
Loading