Skip to content

Commit

Permalink
Merge pull request #2399 from davidhewitt/avoid-duplicate-pymethods
Browse files Browse the repository at this point in the history 
pymethods: prevent methods sharing the same name
  • Loading branch information
@davidhewitt
davidhewitt committed May 24, 2022
2 parents 879eb14 + a306365 commit eafbbc5
Show file tree
Hide file tree
Showing 9 changed files with 263 additions and 200 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `impl<T, const N: usize> IntoPy<PyObject> for [T; N]` now requires `T: IntoPy` rather than `T: ToPyObject`. [#2326](https://github.com/PyO3/pyo3/pull/2326)
- Correct `wrap_pymodule` to match normal namespacing rules: it no longer "sees through" glob imports of `use submodule::*` when `submodule::submodule` is a `#[pymodule]`. [#2363](https://github.com/PyO3/pyo3/pull/2363)
- Allow `#[classattr]` methods to be fallible. [#2385](https://github.com/PyO3/pyo3/pull/2385)
- Prevent multiple `#[pymethods]` with the same name for a single `#[pyclass]`. [#2399](https://github.com/PyO3/pyo3/pull/2399)

### Fixed

Expand Down
11 changes: 3 additions & 8 deletions pyo3-macros-backend/src/params.rs
View file
Expand Up @@ -4,7 +4,7 @@ use crate::{
attributes::FromPyWithAttribute,
method::{FnArg, FnSpec},
pyfunction::Argument,
utils::{remove_lifetime, replace_self, unwrap_ty_group},
utils::{remove_lifetime, unwrap_ty_group},
};
use proc_macro2::{Span, TokenStream};
use quote::{quote, quote_spanned};
Expand Down Expand Up @@ -67,7 +67,7 @@ pub fn impl_arg_params(
// is (*args, **kwds).
let mut arg_convert = vec![];
for (i, arg) in spec.args.iter().enumerate() {
arg_convert.push(impl_arg_param(arg, spec, i, None, &mut 0, py, &args_array)?);
arg_convert.push(impl_arg_param(arg, spec, i, &mut 0, py, &args_array)?);
}
return Ok(quote! {
let _args = #py.from_borrowed_ptr::<_pyo3::types::PyTuple>(_args);
Expand Down Expand Up @@ -118,7 +118,6 @@ pub fn impl_arg_params(
arg,
spec,
idx,
self_,
&mut option_pos,
py,
&args_array,
Expand Down Expand Up @@ -189,7 +188,6 @@ fn impl_arg_param(
arg: &FnArg<'_>,
spec: &FnSpec<'_>,
idx: usize,
self_: Option<&syn::Type>,
option_pos: &mut usize,
py: &syn::Ident,
args_array: &syn::Ident,
Expand Down Expand Up @@ -290,10 +288,7 @@ fn impl_arg_param(
};

return if let syn::Type::Reference(tref) = unwrap_ty_group(arg.optional.unwrap_or(ty)) {
let mut tref = remove_lifetime(tref);
if let Some(cls) = self_ {
replace_self(&mut tref.elem, cls);
}
let tref = remove_lifetime(tref);
let mut_ = tref.mutability;
let (target_ty, borrow_tmp) = if arg.optional.is_some() {
// Get Option<&T> from Option<PyRef<T>>
Expand Down
79 changes: 49 additions & 30 deletions pyo3-macros-backend/src/pyclass.rs
View file
Expand Up @@ -8,9 +8,13 @@ use crate::attributes::{
};
use crate::deprecations::{Deprecation, Deprecations};
use crate::konst::{ConstAttributes, ConstSpec};
use crate::pyimpl::{gen_default_items, gen_py_const, PyClassMethodsType};
use crate::pymethod::{impl_py_getter_def, impl_py_setter_def, PropertyType};
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__,
};
use crate::utils::{self, get_pyo3_crate, PythonDoc};
use crate::PyFunctionOptions;
use proc_macro2::{Span, TokenStream};
use quote::quote;
use syn::ext::IdentExt;
Expand Down Expand Up @@ -420,60 +424,56 @@ fn impl_enum_class(
krate: syn::Path,
) -> TokenStream {
let cls = enum_.ident;
let ty: syn::Type = syn::parse_quote!(#cls);
let variants = enum_.variants;
let pytypeinfo = impl_pytypeinfo(cls, args, None);

let default_repr_impl: syn::ImplItemMethod = {
let (default_repr, default_repr_slot) = {
let variants_repr = variants.iter().map(|variant| {
let variant_name = variant.ident;
// Assuming all variants are unit variants because they are the only type we support.
let repr = format!("{}.{}", cls, variant_name);
quote! { #cls::#variant_name => #repr, }
});
syn::parse_quote! {
#[doc(hidden)]
#[allow(non_snake_case)]
#[pyo3(name = "__repr__")]
let mut repr_impl: syn::ImplItemMethod = syn::parse_quote! {
fn __pyo3__repr__(&self) -> &'static str {
match self {
#(#variants_repr)*
}
}
}
};
let repr_slot = generate_default_protocol_slot(&ty, &mut repr_impl, &__REPR__).unwrap();
(repr_impl, repr_slot)
};

let repr_type = &enum_.repr_type;

let default_int = {
let (default_int, default_int_slot) = {
// This implementation allows us to convert &T to #repr_type without implementing `Copy`
let variants_to_int = variants.iter().map(|variant| {
let variant_name = variant.ident;
quote! { #cls::#variant_name => #cls::#variant_name as #repr_type, }
});
syn::parse_quote! {
#[doc(hidden)]
#[allow(non_snake_case)]
#[pyo3(name = "__int__")]
let mut int_impl: syn::ImplItemMethod = syn::parse_quote! {
fn __pyo3__int__(&self) -> #repr_type {
match self {
#(#variants_to_int)*
}
}
}
};
let int_slot = generate_default_protocol_slot(&ty, &mut int_impl, &__INT__).unwrap();
(int_impl, int_slot)
};

let default_richcmp = {
let (default_richcmp, default_richcmp_slot) = {
let variants_eq = variants.iter().map(|variant| {
let variant_name = variant.ident;
quote! {
(#cls::#variant_name, #cls::#variant_name) =>
Ok(true.to_object(py)),
}
});
syn::parse_quote! {
#[doc(hidden)]
#[allow(non_snake_case)]
#[pyo3(name = "__richcmp__")]
let mut richcmp_impl: syn::ImplItemMethod = syn::parse_quote! {
fn __pyo3__richcmp__(
&self,
py: _pyo3::Python,
Expand All @@ -498,17 +498,20 @@ fn impl_enum_class(
_ => Ok(py.NotImplemented()),
}
}
}
};
let richcmp_slot =
generate_default_protocol_slot(&ty, &mut richcmp_impl, &__RICHCMP__).unwrap();
(richcmp_impl, richcmp_slot)
};

let mut default_methods = vec![default_repr_impl, default_richcmp, default_int];
let default_slots = vec![default_repr_slot, default_int_slot, default_richcmp_slot];

let pyclass_impls = PyClassImplsBuilder::new(
cls,
args,
methods_type,
enum_default_methods(cls, variants.iter().map(|v| v.ident)),
enum_default_slots(cls, &mut default_methods),
default_slots,
)
.doc(doc)
.impl_all();
Expand All @@ -521,13 +524,36 @@ fn impl_enum_class(

#pyclass_impls

#[doc(hidden)]
#[allow(non_snake_case)]
impl #cls {
#(#default_methods)*
#default_repr
#default_int
#default_richcmp
}
};
}
}

fn generate_default_protocol_slot(
cls: &syn::Type,
method: &mut syn::ImplItemMethod,
slot: &SlotDef,
) -> syn::Result<TokenStream> {
let spec = FnSpec::parse(
&mut method.sig,
&mut Vec::new(),
PyFunctionOptions::default(),
)
.unwrap();
let name = spec.name.to_string();
slot.generate_type_slot(
&syn::parse_quote!(#cls),
&spec,
&format!("__default_{}__", name),
)
}

fn enum_default_methods<'a>(
cls: &'a syn::Ident,
unit_variant_names: impl IntoIterator<Item = &'a syn::Ident>,
Expand All @@ -550,13 +576,6 @@ fn enum_default_methods<'a>(
.collect()
}

fn enum_default_slots(
cls: &syn::Ident,
default_items: &mut [syn::ImplItemMethod],
) -> Vec<TokenStream> {
gen_default_items(cls, default_items).collect()
}

fn extract_variant_data(variant: &syn::Variant) -> syn::Result<PyClassEnumVariant<'_>> {
use syn::Fields;
let ident = match variant.fields {
Expand Down
40 changes: 10 additions & 30 deletions pyo3-macros-backend/src/pyimpl.rs
View file
Expand Up @@ -11,7 +11,7 @@ use crate::{
};
use proc_macro2::TokenStream;
use pymethod::GeneratedPyMethod;
use quote::quote;
use quote::{format_ident, quote};
use syn::{
parse::{Parse, ParseStream},
spanned::Spanned,
Expand Down Expand Up @@ -164,49 +164,29 @@ pub fn impl_methods(

pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream {
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! {
_pyo3::class::PyMethodDefType::ClassAttribute({
_pyo3::class::PyClassAttributeDef::new(
#python_name,
_pyo3::impl_::pymethods::PyClassAttributeFactory({
fn __wrap(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> {
#deprecations
::std::result::Result::Ok(_pyo3::IntoPy::into_py(#cls::#member, py))
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))
}
}
__wrap
#cls::#wrapper_ident
})
)
})
}
}

pub fn gen_default_items<'a>(
cls: &syn::Ident,
method_defs: &'a mut [syn::ImplItemMethod],
) -> impl Iterator<Item = TokenStream> + 'a {
// This function uses a lot of `unwrap()`; since method_defs are provided by us, they should
// all succeed.
let ty: syn::Type = syn::parse_quote!(#cls);

method_defs.iter_mut().map(move |meth| {
let options = PyFunctionOptions::from_attrs(&mut meth.attrs).unwrap();
match pymethod::gen_py_method(&ty, &mut meth.sig, &mut meth.attrs, options).unwrap() {
GeneratedPyMethod::Proto(token_stream) => {
let attrs = get_cfg_attributes(&meth.attrs);
quote!(#(#attrs)* #token_stream)
}
GeneratedPyMethod::SlotTraitImpl(..) => {
panic!("SlotFragment methods cannot have default implementation!")
}
GeneratedPyMethod::Method(_) => {
panic!("Only protocol methods can have default implementation!")
}
}
})
}

fn impl_py_methods(
ty: &syn::Type,
methods: Vec<TokenStream>,
Expand Down

0 comments on commit eafbbc5

Please sign in to comment.