Skip to content

Commit

Permalink
Merge pull request #2458 from davidhewitt/pymethods-order
Browse files Browse the repository at this point in the history
macros: emit pymethod associated methods as a single block
  • Loading branch information
davidhewitt committed Jun 16, 2022
2 parents 1978712 + f81a01b commit 7fb9f32
Show file tree
Hide file tree
Showing 7 changed files with 275 additions and 200 deletions.
37 changes: 14 additions & 23 deletions pyo3-macros-backend/src/method.rs
Expand Up @@ -7,7 +7,7 @@ use crate::deprecations::Deprecation;
use crate::params::{accept_args_kwargs, impl_arg_params};
use crate::pyfunction::PyFunctionOptions;
use crate::pyfunction::{PyFunctionArgPyO3Attributes, PyFunctionSignature};
use crate::utils::{self, get_pyo3_crate, PythonDoc};
use crate::utils::{self, PythonDoc};
use crate::{deprecations::Deprecations, pyfunction::Argument};
use proc_macro2::{Span, TokenStream};
use quote::ToTokens;
Expand Down Expand Up @@ -224,7 +224,6 @@ pub struct FnSpec<'a> {
pub deprecations: Deprecations,
pub convention: CallingConvention,
pub text_signature: Option<TextSignatureAttribute>,
pub krate: syn::Path,
pub unsafety: Option<syn::Token![unsafe]>,
}

Expand Down Expand Up @@ -266,7 +265,6 @@ impl<'a> FnSpec<'a> {
) -> Result<FnSpec<'a>> {
let PyFunctionOptions {
text_signature,
krate,
name,
mut deprecations,
..
Expand All @@ -285,7 +283,6 @@ impl<'a> FnSpec<'a> {
let name = &sig.ident;
let ty = get_return_info(&sig.output);
let python_name = python_name.as_ref().unwrap_or(name).unraw();
let krate = get_pyo3_crate(&krate);

let doc = utils::get_doc(
meth_attrs,
Expand Down Expand Up @@ -321,7 +318,6 @@ impl<'a> FnSpec<'a> {
doc,
deprecations,
text_signature,
krate,
unsafety: sig.unsafety,
})
}
Expand Down Expand Up @@ -489,16 +485,14 @@ impl<'a> FnSpec<'a> {
_pyo3::callback::convert(#py, ret)
};

let krate = &self.krate;
Ok(match self.convention {
CallingConvention::Noargs => {
quote! {
unsafe extern "C" fn #ident (
_slf: *mut #krate::ffi::PyObject,
_args: *mut #krate::ffi::PyObject,
) -> *mut #krate::ffi::PyObject
_slf: *mut _pyo3::ffi::PyObject,
_args: *mut _pyo3::ffi::PyObject,
) -> *mut _pyo3::ffi::PyObject
{
use #krate as _pyo3;
#deprecations
let gil = _pyo3::GILPool::new();
let #py = gil.python();
Expand All @@ -513,12 +507,11 @@ impl<'a> FnSpec<'a> {
let arg_convert = impl_arg_params(self, cls, &py, true)?;
quote! {
unsafe extern "C" fn #ident (
_slf: *mut #krate::ffi::PyObject,
_args: *const *mut #krate::ffi::PyObject,
_nargs: #krate::ffi::Py_ssize_t,
_kwnames: *mut #krate::ffi::PyObject) -> *mut #krate::ffi::PyObject
_slf: *mut _pyo3::ffi::PyObject,
_args: *const *mut _pyo3::ffi::PyObject,
_nargs: _pyo3::ffi::Py_ssize_t,
_kwnames: *mut _pyo3::ffi::PyObject) -> *mut _pyo3::ffi::PyObject
{
use #krate as _pyo3;
#deprecations
let gil = _pyo3::GILPool::new();
let #py = gil.python();
Expand All @@ -534,11 +527,10 @@ impl<'a> FnSpec<'a> {
let arg_convert = impl_arg_params(self, cls, &py, false)?;
quote! {
unsafe extern "C" fn #ident (
_slf: *mut #krate::ffi::PyObject,
_args: *mut #krate::ffi::PyObject,
_kwargs: *mut #krate::ffi::PyObject) -> *mut #krate::ffi::PyObject
_slf: *mut _pyo3::ffi::PyObject,
_args: *mut _pyo3::ffi::PyObject,
_kwargs: *mut _pyo3::ffi::PyObject) -> *mut _pyo3::ffi::PyObject
{
use #krate as _pyo3;
#deprecations
let gil = _pyo3::GILPool::new();
let #py = gil.python();
Expand All @@ -555,11 +547,10 @@ impl<'a> FnSpec<'a> {
let arg_convert = impl_arg_params(self, cls, &py, false)?;
quote! {
unsafe extern "C" fn #ident (
subtype: *mut #krate::ffi::PyTypeObject,
_args: *mut #krate::ffi::PyObject,
_kwargs: *mut #krate::ffi::PyObject) -> *mut #krate::ffi::PyObject
subtype: *mut _pyo3::ffi::PyTypeObject,
_args: *mut _pyo3::ffi::PyObject,
_kwargs: *mut _pyo3::ffi::PyObject) -> *mut _pyo3::ffi::PyObject
{
use #krate as _pyo3;
#deprecations
use _pyo3::callback::IntoPyCallbackOutput;
let gil = _pyo3::GILPool::new();
Expand Down
41 changes: 29 additions & 12 deletions pyo3-macros-backend/src/pyclass.rs
Expand Up @@ -11,7 +11,8 @@ use crate::konst::{ConstAttributes, ConstSpec};
use crate::method::FnSpec;
use crate::pyimpl::{gen_py_const, PyClassMethodsType};
use crate::pymethod::{
impl_py_getter_def, impl_py_setter_def, PropertyType, SlotDef, __INT__, __REPR__, __RICHCMP__,
impl_py_getter_def, impl_py_setter_def, MethodAndMethodDef, MethodAndSlotDef, PropertyType,
SlotDef, __INT__, __REPR__, __RICHCMP__,
};
use crate::utils::{self, get_pyo3_crate, PythonDoc};
use crate::PyFunctionOptions;
Expand Down Expand Up @@ -539,7 +540,7 @@ fn generate_default_protocol_slot(
cls: &syn::Type,
method: &mut syn::ImplItemMethod,
slot: &SlotDef,
) -> syn::Result<TokenStream> {
) -> syn::Result<MethodAndSlotDef> {
let spec = FnSpec::parse(
&mut method.sig,
&mut Vec::new(),
Expand All @@ -557,7 +558,7 @@ fn generate_default_protocol_slot(
fn enum_default_methods<'a>(
cls: &'a syn::Ident,
unit_variant_names: impl IntoIterator<Item = &'a syn::Ident>,
) -> Vec<TokenStream> {
) -> Vec<MethodAndMethodDef> {
let cls_type = syn::parse_quote!(#cls);
let variant_to_attribute = |ident: &syn::Ident| ConstSpec {
rust_ident: ident.clone(),
Expand Down Expand Up @@ -588,7 +589,7 @@ fn extract_variant_data(variant: &syn::Variant) -> syn::Result<PyClassEnumVarian
fn descriptors_to_items(
cls: &syn::Ident,
field_options: Vec<(&syn::Field, FieldPyO3Options)>,
) -> syn::Result<Vec<TokenStream>> {
) -> syn::Result<Vec<MethodAndMethodDef>> {
let ty = syn::parse_quote!(#cls);
field_options
.into_iter()
Expand Down Expand Up @@ -666,8 +667,8 @@ struct PyClassImplsBuilder<'a> {
cls: &'a syn::Ident,
attr: &'a PyClassArgs,
methods_type: PyClassMethodsType,
default_methods: Vec<TokenStream>,
default_slots: Vec<TokenStream>,
default_methods: Vec<MethodAndMethodDef>,
default_slots: Vec<MethodAndSlotDef>,
doc: Option<PythonDoc>,
}

Expand All @@ -676,8 +677,8 @@ impl<'a> PyClassImplsBuilder<'a> {
cls: &'a syn::Ident,
attr: &'a PyClassArgs,
methods_type: PyClassMethodsType,
default_methods: Vec<TokenStream>,
default_slots: Vec<TokenStream>,
default_methods: Vec<MethodAndMethodDef>,
default_slots: Vec<MethodAndSlotDef>,
) -> Self {
Self {
cls,
Expand Down Expand Up @@ -838,8 +839,18 @@ impl<'a> PyClassImplsBuilder<'a> {
None
};

let default_methods = &self.default_methods;
let default_slots = &self.default_slots;
let default_methods = self
.default_methods
.iter()
.map(|meth| &meth.associated_method)
.chain(
self.default_slots
.iter()
.map(|meth| &meth.associated_method),
);

let default_method_defs = self.default_methods.iter().map(|meth| &meth.method_def);
let default_slot_defs = self.default_slots.iter().map(|slot| &slot.slot_def);
let freelist_slots = self.freelist_slots();

let deprecations = &self.attr.deprecations;
Expand Down Expand Up @@ -907,8 +918,8 @@ impl<'a> PyClassImplsBuilder<'a> {
let collector = PyClassImplCollector::<Self>::new();
#deprecations;
static INTRINSIC_ITEMS: PyClassItems = PyClassItems {
methods: &[#(#default_methods),*],
slots: &[#(#default_slots),* #(#freelist_slots),*],
methods: &[#(#default_method_defs),*],
slots: &[#(#default_slot_defs),* #(#freelist_slots),*],
};
visitor(&INTRINSIC_ITEMS);
#pymethods_items
Expand All @@ -920,6 +931,12 @@ impl<'a> PyClassImplsBuilder<'a> {
#weaklist_offset
}

#[doc(hidden)]
#[allow(non_snake_case)]
impl #cls {
#(#default_methods)*
}

#inventory_class
}
}
Expand Down
4 changes: 2 additions & 2 deletions pyo3-macros-backend/src/pyfunction.rs
Expand Up @@ -430,7 +430,6 @@ pub fn impl_wrap_pyfunction(
doc,
deprecations: options.deprecations,
text_signature: options.text_signature,
krate: krate.clone(),
unsafety: func.sig.unsafety,
};

Expand All @@ -442,7 +441,6 @@ pub fn impl_wrap_pyfunction(
let methoddef = spec.get_methoddef(wrapper_ident);

let wrapped_pyfunction = quote! {
#wrapper

// Create a module with the same name as the `#[pyfunction]` - this way `use <the function>`
// will actually bring both the module and the function into scope.
Expand All @@ -461,6 +459,8 @@ pub fn impl_wrap_pyfunction(
impl #name::MakeDef {
const DEF: #krate::impl_::pyfunction::PyMethodDef = #methoddef;
}

#wrapper
};
};
Ok(wrapped_pyfunction)
Expand Down
62 changes: 42 additions & 20 deletions pyo3-macros-backend/src/pyimpl.rs
Expand Up @@ -6,7 +6,7 @@ use crate::{
attributes::{take_pyo3_options, CrateAttribute},
konst::{ConstAttributes, ConstSpec},
pyfunction::PyFunctionOptions,
pymethod::{self, is_proto_method},
pymethod::{self, is_proto_method, MethodAndMethodDef, MethodAndSlotDef},
utils::get_pyo3_crate,
};
use proc_macro2::TokenStream;
Expand Down Expand Up @@ -95,6 +95,7 @@ pub fn impl_methods(
let mut trait_impls = Vec::new();
let mut proto_impls = Vec::new();
let mut methods = Vec::new();
let mut associated_methods = Vec::new();

let mut implemented_proto_fragments = HashSet::new();

Expand All @@ -104,18 +105,26 @@ pub fn impl_methods(
let mut fun_options = PyFunctionOptions::from_attrs(&mut meth.attrs)?;
fun_options.krate = fun_options.krate.or_else(|| options.krate.clone());
match pymethod::gen_py_method(ty, &mut meth.sig, &mut meth.attrs, fun_options)? {
GeneratedPyMethod::Method(token_stream) => {
GeneratedPyMethod::Method(MethodAndMethodDef {
associated_method,
method_def,
}) => {
let attrs = get_cfg_attributes(&meth.attrs);
methods.push(quote!(#(#attrs)* #token_stream));
associated_methods.push(quote!(#(#attrs)* #associated_method));
methods.push(quote!(#(#attrs)* #method_def));
}
GeneratedPyMethod::SlotTraitImpl(method_name, token_stream) => {
implemented_proto_fragments.insert(method_name);
let attrs = get_cfg_attributes(&meth.attrs);
trait_impls.push(quote!(#(#attrs)* #token_stream));
}
GeneratedPyMethod::Proto(token_stream) => {
GeneratedPyMethod::Proto(MethodAndSlotDef {
associated_method,
slot_def,
}) => {
let attrs = get_cfg_attributes(&meth.attrs);
proto_impls.push(quote!(#(#attrs)* #token_stream))
proto_impls.push(quote!(#(#attrs)* #slot_def));
associated_methods.push(quote!(#(#attrs)* #associated_method));
}
}
}
Expand All @@ -127,8 +136,12 @@ pub fn impl_methods(
attributes,
};
let attrs = get_cfg_attributes(&konst.attrs);
let meth = gen_py_const(ty, &spec);
methods.push(quote!(#(#attrs)* #meth));
let MethodAndMethodDef {
associated_method,
method_def,
} = gen_py_const(ty, &spec);
methods.push(quote!(#(#attrs)* #method_def));
associated_methods.push(quote!(#(#attrs)* #associated_method));
if is_proto_method(&spec.python_name().to_string()) {
// If this is a known protocol method e.g. __contains__, then allow this
// symbol even though it's not an uppercase constant.
Expand Down Expand Up @@ -158,32 +171,41 @@ pub fn impl_methods(
#(#trait_impls)*

#items

#[doc(hidden)]
#[allow(non_snake_case)]
impl #ty {
#(#associated_methods)*
}
};
})
}

pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream {
pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> MethodAndMethodDef {
let member = &spec.rust_ident;
let wrapper_ident = format_ident!("__pymethod_{}__", member);
let deprecations = &spec.attributes.deprecations;
let python_name = &spec.null_terminated_python_name();
quote! {

let associated_method = quote! {
fn #wrapper_ident(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> {
#deprecations
::std::result::Result::Ok(_pyo3::IntoPy::into_py(#cls::#member, py))
}
};

let method_def = quote! {
_pyo3::class::PyMethodDefType::ClassAttribute({
_pyo3::class::PyClassAttributeDef::new(
#python_name,
_pyo3::impl_::pymethods::PyClassAttributeFactory({
impl #cls {
#[doc(hidden)]
#[allow(non_snake_case)]
fn #wrapper_ident(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> {
#deprecations
::std::result::Result::Ok(_pyo3::IntoPy::into_py(#cls::#member, py))
}
}
#cls::#wrapper_ident
})
_pyo3::impl_::pymethods::PyClassAttributeFactory(#cls::#wrapper_ident)
)
})
};

MethodAndMethodDef {
associated_method,
method_def,
}
}

Expand Down

0 comments on commit 7fb9f32

Please sign in to comment.