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

Jump to bottom

pymethods: prevent methods sharing the same name #2399

Merged
merged 1 commit into from May 24, 2022
Merged

pymethods: prevent methods sharing the same name #2399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -23,6 +23,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
Open in desktop
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
Open in desktop
Expand Up @@ -6,9 +6,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 @@ -418,60 +422,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 @@ -496,17 +496,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 @@ -519,13 +522,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 @@ -548,13 +574,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
Open in desktop
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