Merge pull request #2157 from davidhewitt/simpler-default-items

refactor: simpler pyclass internals
This commit is contained in:
David Hewitt 2022-02-09 09:04:08 +00:00 committed by GitHub
commit be31ca96f7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 134 additions and 211 deletions

View file

@ -50,7 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Reduce generated LLVM code size (to improve compile times) for:
- internal `handle_panic` helper [#2074](https://github.com/PyO3/pyo3/pull/2074)
- `#[pyfunction]` and `#[pymethods]` argument extraction [#2075](https://github.com/PyO3/pyo3/pull/2075) [#2085](https://github.com/PyO3/pyo3/pull/2085)
- `#[pyclass]` type object creation [#2076](https://github.com/PyO3/pyo3/pull/2076) [#2081](https://github.com/PyO3/pyo3/pull/2081)
- `#[pyclass]` type object creation [#2076](https://github.com/PyO3/pyo3/pull/2076) [#2081](https://github.com/PyO3/pyo3/pull/2081) [#2157](https://github.com/PyO3/pyo3/pull/2157)
- `__ipow__` now supports modulo argument on Python 3.8+. [#2083](https://github.com/PyO3/pyo3/pull/2083)
- `pyo3-macros-backend` is now compiled with PyO3 cfgs to enable different magic method definitions based on version. [#2083](https://github.com/PyO3/pyo3/pull/2083)
- `_PyCFunctionFast` now correctly reflects the signature defined in the [Python docs](https://docs.python.org/3/c-api/structures.html#c._PyCFunctionFast). [#2126](https://github.com/PyO3/pyo3/pull/2126)

View file

@ -981,21 +981,6 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass {
visitor(&INTRINSIC_ITEMS);
visitor(collector.py_methods());
}
fn get_new() -> Option<pyo3::ffi::newfunc> {
use pyo3::impl_::pyclass::*;
let collector = PyClassImplCollector::<Self>::new();
collector.new_impl()
}
fn get_alloc() -> Option<pyo3::ffi::allocfunc> {
use pyo3::impl_::pyclass::*;
let collector = PyClassImplCollector::<Self>::new();
collector.alloc_impl()
}
fn get_free() -> Option<pyo3::ffi::freefunc> {
use pyo3::impl_::pyclass::*;
let collector = PyClassImplCollector::<Self>::new();
collector.free_impl()
}
}
# Python::with_gil(|py| {
# let cls = py.get_type::<MyClass>();

View file

@ -387,6 +387,7 @@ fn impl_class(
attr,
methods_type,
descriptors_to_items(cls, field_options)?,
vec![],
)
.doc(doc)
.impl_all();
@ -497,23 +498,15 @@ fn impl_enum_class(
let cls = enum_.ident;
let variants = enum_.variants;
let pytypeinfo = impl_pytypeinfo(cls, args, None);
let pyclass_impls = PyClassImplsBuilder::new(
cls,
args,
methods_type,
unit_variants_as_items(cls, variants.iter().map(|v| v.ident)),
)
.doc(doc)
.impl_all();
let default_repr_impl = {
let default_repr_impl: syn::ImplItemMethod = {
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, }
});
quote! {
syn::parse_quote! {
#[doc(hidden)]
#[allow(non_snake_case)]
#[pyo3(name = "__repr__")]
@ -533,7 +526,7 @@ fn impl_enum_class(
let variant_name = variant.ident;
quote! { #cls::#variant_name => #cls::#variant_name as #repr_type, }
});
quote! {
syn::parse_quote! {
#[doc(hidden)]
#[allow(non_snake_case)]
#[pyo3(name = "__int__")]
@ -553,7 +546,7 @@ fn impl_enum_class(
Ok(true.to_object(py)),
}
});
quote! {
syn::parse_quote! {
#[doc(hidden)]
#[allow(non_snake_case)]
#[pyo3(name = "__richcmp__")]
@ -584,8 +577,17 @@ fn impl_enum_class(
}
};
let default_items =
gen_default_items(cls, vec![default_repr_impl, default_richcmp, default_int]);
let mut default_methods = vec![default_repr_impl, default_richcmp, default_int];
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),
)
.doc(doc)
.impl_all();
Ok(quote! {
const _: () = {
@ -595,15 +597,17 @@ fn impl_enum_class(
#pyclass_impls
#default_items
impl #cls {
#(#default_methods)*
}
};
})
}
fn unit_variants_as_items<'a>(
fn enum_default_methods<'a>(
cls: &'a syn::Ident,
variant_names: impl IntoIterator<Item = &'a syn::Ident>,
) -> TokenStream {
unit_variant_names: impl IntoIterator<Item = &'a syn::Ident>,
) -> Vec<TokenStream> {
let cls_type = syn::parse_quote!(#cls);
let variant_to_attribute = |ident: &syn::Ident| ConstSpec {
rust_ident: ident.clone(),
@ -613,16 +617,17 @@ fn unit_variants_as_items<'a>(
deprecations: Default::default(),
},
};
let py_methods = variant_names
unit_variant_names
.into_iter()
.map(|var| gen_py_const(&cls_type, &variant_to_attribute(var)));
.map(|var| gen_py_const(&cls_type, &variant_to_attribute(var)))
.collect()
}
quote! {
_pyo3::impl_::pyclass::PyClassItems {
methods: &[#(#py_methods),*],
slots: &[]
}
}
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> {
@ -637,9 +642,9 @@ 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<TokenStream> {
) -> syn::Result<Vec<TokenStream>> {
let ty = syn::parse_quote!(#cls);
let py_methods: Vec<TokenStream> = field_options
field_options
.into_iter()
.enumerate()
.flat_map(|(field_index, (field, options))| {
@ -671,14 +676,7 @@ fn descriptors_to_items(
name_err.into_iter().chain(getter).chain(setter)
})
.collect::<syn::Result<_>>()?;
Ok(quote! {
_pyo3::impl_::pyclass::PyClassItems {
methods: &[#(#py_methods),*],
slots: &[]
}
})
.collect::<syn::Result<_>>()
}
fn impl_pytypeinfo(
@ -722,7 +720,8 @@ struct PyClassImplsBuilder<'a> {
cls: &'a syn::Ident,
attr: &'a PyClassArgs,
methods_type: PyClassMethodsType,
default_items: TokenStream,
default_methods: Vec<TokenStream>,
default_slots: Vec<TokenStream>,
doc: Option<PythonDoc>,
}
@ -731,13 +730,15 @@ impl<'a> PyClassImplsBuilder<'a> {
cls: &'a syn::Ident,
attr: &'a PyClassArgs,
methods_type: PyClassMethodsType,
default_items: TokenStream,
default_methods: Vec<TokenStream>,
default_slots: Vec<TokenStream>,
) -> Self {
Self {
cls,
attr,
methods_type,
default_items,
default_methods,
default_slots,
doc: None,
}
}
@ -899,7 +900,9 @@ impl<'a> PyClassImplsBuilder<'a> {
None
};
let default_items = &self.default_items;
let default_methods = &self.default_methods;
let default_slots = &self.default_slots;
let freelist_slots = self.freelist_slots();
quote! {
impl _pyo3::impl_::pyclass::PyClassImpl for #cls {
@ -916,29 +919,14 @@ impl<'a> PyClassImplsBuilder<'a> {
fn for_all_items(visitor: &mut dyn ::std::ops::FnMut(& _pyo3::impl_::pyclass::PyClassItems)) {
use _pyo3::impl_::pyclass::*;
let collector = PyClassImplCollector::<Self>::new();
static INTRINSIC_ITEMS: PyClassItems = #default_items;
static INTRINSIC_ITEMS: PyClassItems = PyClassItems {
methods: &[#(#default_methods),*],
slots: &[#(#default_slots),* #(#freelist_slots),*],
};
visitor(&INTRINSIC_ITEMS);
// This depends on Python implementation detail;
// an old slot entry will be overriden by newer ones.
visitor(collector.pyclass_default_items());
#pymethods_items
#pyproto_items
}
fn get_new() -> ::std::option::Option<_pyo3::ffi::newfunc> {
use _pyo3::impl_::pyclass::*;
let collector = PyClassImplCollector::<Self>::new();
collector.new_impl()
}
fn get_alloc() -> ::std::option::Option<_pyo3::ffi::allocfunc> {
use _pyo3::impl_::pyclass::*;
let collector = PyClassImplCollector::<Self>::new();
collector.alloc_impl()
}
fn get_free() -> ::std::option::Option<_pyo3::ffi::freefunc> {
use _pyo3::impl_::pyclass::*;
let collector = PyClassImplCollector::<Self>::new();
collector.free_impl()
}
#dict_offset
@ -967,23 +955,33 @@ impl<'a> PyClassImplsBuilder<'a> {
}
}
}
impl _pyo3::impl_::pyclass::PyClassAllocImpl<#cls> for _pyo3::impl_::pyclass::PyClassImplCollector<#cls> {
#[inline]
fn alloc_impl(self) -> ::std::option::Option<_pyo3::ffi::allocfunc> {
::std::option::Option::Some(_pyo3::impl_::pyclass::alloc_with_freelist::<#cls>)
}
}
impl _pyo3::impl_::pyclass::PyClassFreeImpl<#cls> for _pyo3::impl_::pyclass::PyClassImplCollector<#cls> {
#[inline]
fn free_impl(self) -> ::std::option::Option<_pyo3::ffi::freefunc> {
::std::option::Option::Some(_pyo3::impl_::pyclass::free_with_freelist::<#cls>)
}
}
}
})
}
fn freelist_slots(&self) -> Vec<TokenStream> {
let cls = self.cls;
if self.attr.freelist.is_some() {
vec![
quote! {
_pyo3::ffi::PyType_Slot {
slot: _pyo3::ffi::Py_tp_alloc,
pfunc: _pyo3::impl_::pyclass::alloc_with_freelist::<#cls> as *mut _,
}
},
quote! {
_pyo3::ffi::PyType_Slot {
slot: _pyo3::ffi::Py_tp_free,
pfunc: _pyo3::impl_::pyclass::free_with_freelist::<#cls> as *mut _,
}
},
]
} else {
Vec::new()
}
}
/// Enforce at compile time that PyGCProtocol is implemented
fn impl_gc(&self) -> TokenStream {
let cls = self.cls;

View file

@ -108,10 +108,6 @@ pub fn impl_methods(
let attrs = get_cfg_attributes(&meth.attrs);
methods.push(quote!(#(#attrs)* #token_stream));
}
GeneratedPyMethod::TraitImpl(token_stream) => {
let attrs = get_cfg_attributes(&meth.attrs);
trait_impls.push(quote!(#(#attrs)* #token_stream));
}
GeneratedPyMethod::SlotTraitImpl(method_name, token_stream) => {
implemented_proto_fragments.insert(method_name);
let attrs = get_cfg_attributes(&meth.attrs);
@ -186,49 +182,29 @@ pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream {
}
}
pub fn gen_default_items(cls: &syn::Ident, method_defs: Vec<TokenStream>) -> TokenStream {
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);
let mut method_defs: Vec<_> = method_defs
.into_iter()
.map(|token| syn::parse2::<syn::ImplItemMethod>(token).unwrap())
.collect();
let mut proto_impls = Vec::new();
for meth in &mut method_defs {
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);
proto_impls.push(quote!(#(#attrs)* #token_stream))
quote!(#(#attrs)* #token_stream)
}
GeneratedPyMethod::SlotTraitImpl(..) => {
panic!("SlotFragment methods cannot have default implementation!")
}
GeneratedPyMethod::Method(_) | GeneratedPyMethod::TraitImpl(_) => {
GeneratedPyMethod::Method(_) => {
panic!("Only protocol methods can have default implementation!")
}
}
}
quote! {
impl #cls {
#(#method_defs)*
}
impl _pyo3::impl_::pyclass::PyClassDefaultItems<#cls>
for _pyo3::impl_::pyclass::PyClassImplCollector<#cls> {
fn pyclass_default_items(self) -> &'static _pyo3::impl_::pyclass::PyClassItems {
static ITEMS: _pyo3::impl_::pyclass::PyClassItems = _pyo3::impl_::pyclass::PyClassItems {
methods: &[],
slots: &[#(#proto_impls),*]
};
&ITEMS
}
}
}
})
}
fn impl_py_methods(

View file

@ -20,7 +20,6 @@ use syn::{ext::IdentExt, spanned::Spanned, Result};
pub enum GeneratedPyMethod {
Method(TokenStream),
Proto(TokenStream),
TraitImpl(TokenStream),
SlotTraitImpl(String, TokenStream),
}
@ -128,7 +127,7 @@ pub fn gen_py_method(
Some(quote!(_pyo3::ffi::METH_STATIC)),
)?),
// special prototypes
(_, FnType::FnNew) => GeneratedPyMethod::TraitImpl(impl_py_method_def_new(cls, spec)?),
(_, FnType::FnNew) => GeneratedPyMethod::Proto(impl_py_method_def_new(cls, spec)?),
(_, FnType::Getter(self_type)) => GeneratedPyMethod::Method(impl_py_getter_def(
cls,
@ -191,18 +190,18 @@ pub fn impl_py_method_def(
}
fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec) -> Result<TokenStream> {
let wrapper_ident = syn::Ident::new("__wrap", Span::call_site());
let wrapper_ident = syn::Ident::new("__pymethod__new__", Span::call_site());
let wrapper = spec.get_wrapper_function(&wrapper_ident, Some(cls))?;
Ok(quote! {
impl _pyo3::impl_::pyclass::PyClassNewImpl<#cls> for _pyo3::impl_::pyclass::PyClassImplCollector<#cls> {
fn new_impl(self) -> ::std::option::Option<_pyo3::ffi::newfunc> {
::std::option::Option::Some({
#wrapper
#wrapper_ident
})
}
Ok(quote! {{
impl #cls {
#[doc(hidden)]
#wrapper
}
})
_pyo3::ffi::PyType_Slot {
slot: _pyo3::ffi::Py_tp_new,
pfunc: #cls::#wrapper_ident as _pyo3::ffi::newfunc as _
}
}})
}
fn impl_call_slot(cls: &syn::Type, mut spec: FnSpec) -> Result<TokenStream> {

View file

@ -178,18 +178,6 @@ pub trait PyClassImpl: Sized {
fn for_all_items(visitor: &mut dyn FnMut(&PyClassItems));
#[inline]
fn get_new() -> Option<ffi::newfunc> {
None
}
#[inline]
fn get_alloc() -> Option<ffi::allocfunc> {
None
}
#[inline]
fn get_free() -> Option<ffi::freefunc> {
None
}
#[inline]
fn dict_offset() -> Option<ffi::Py_ssize_t> {
None
@ -202,16 +190,6 @@ pub trait PyClassImpl: Sized {
// Traits describing known special methods.
pub trait PyClassNewImpl<T> {
fn new_impl(self) -> Option<ffi::newfunc>;
}
impl<T> PyClassNewImpl<T> for &'_ PyClassImplCollector<T> {
fn new_impl(self) -> Option<ffi::newfunc> {
None
}
}
macro_rules! slot_fragment_trait {
($trait_name:ident, $($default_method:tt)*) => {
#[allow(non_camel_case_types)]
@ -602,26 +580,6 @@ macro_rules! generate_pyclass_pow_slot {
}
pub use generate_pyclass_pow_slot;
pub trait PyClassAllocImpl<T> {
fn alloc_impl(self) -> Option<ffi::allocfunc>;
}
impl<T> PyClassAllocImpl<T> for &'_ PyClassImplCollector<T> {
fn alloc_impl(self) -> Option<ffi::allocfunc> {
None
}
}
pub trait PyClassFreeImpl<T> {
fn free_impl(self) -> Option<ffi::freefunc>;
}
impl<T> PyClassFreeImpl<T> for &'_ PyClassImplCollector<T> {
fn free_impl(self) -> Option<ffi::freefunc> {
None
}
}
/// Implements a freelist.
///
/// Do not implement this trait manually. Instead, use `#[pyclass(freelist = N)]`
@ -757,9 +715,6 @@ mod pyproto_traits {
#[cfg(feature = "pyproto")]
pub use pyproto_traits::*;
// items that PyO3 implements by default, but can be overidden by the users.
items_trait!(PyClassDefaultItems, pyclass_default_items);
// Protocol slots from #[pymethods] if not using inventory.
#[cfg(not(feature = "multiple-pymethods"))]
items_trait!(PyMethodsProtocolItems, methods_protocol_items);
@ -846,19 +801,6 @@ impl<T: PyClass> PyClassBaseType for T {
type Initializer = crate::pyclass_init::PyClassInitializer<Self>;
}
/// Default new implementation
pub(crate) unsafe extern "C" fn fallback_new(
_subtype: *mut ffi::PyTypeObject,
_args: *mut ffi::PyObject,
_kwds: *mut ffi::PyObject,
) -> *mut ffi::PyObject {
crate::callback_body!(py, {
Err::<(), _>(crate::exceptions::PyTypeError::new_err(
"No constructor defined",
))
})
}
/// Implementation of tp_dealloc for all pyclasses
pub(crate) unsafe extern "C" fn tp_dealloc<T: PyClass>(obj: *mut ffi::PyObject) {
crate::callback_body!(py, T::Layout::tp_dealloc(obj, py))

View file

@ -3,8 +3,8 @@ use crate::{
callback::IntoPyCallbackOutput,
ffi,
impl_::pyclass::{
assign_sequence_item_from_mapping, fallback_new, get_sequence_item_from_mapping,
tp_dealloc, PyClassDict, PyClassImpl, PyClassItems, PyClassWeakRef,
assign_sequence_item_from_mapping, get_sequence_item_from_mapping, tp_dealloc, PyClassDict,
PyClassImpl, PyClassItems, PyClassWeakRef,
},
IntoPy, IntoPyPointer, PyCell, PyErr, PyMethodDefType, PyNativeType, PyObject, PyResult,
PyTypeInfo, Python,
@ -49,10 +49,7 @@ where
T::NAME,
T::BaseType::type_object_raw(py),
std::mem::size_of::<T::Layout>(),
T::get_new(),
tp_dealloc::<T>,
T::get_alloc(),
T::get_free(),
T::dict_offset(),
T::weaklist_offset(),
&T::for_all_items,
@ -73,10 +70,7 @@ unsafe fn create_type_object_impl(
name: &str,
base_type_object: *mut ffi::PyTypeObject,
basicsize: usize,
tp_new: Option<ffi::newfunc>,
tp_dealloc: ffi::destructor,
tp_alloc: Option<ffi::allocfunc>,
tp_free: Option<ffi::freefunc>,
dict_offset: Option<ffi::Py_ssize_t>,
weaklist_offset: Option<ffi::Py_ssize_t>,
for_all_items: &dyn Fn(&mut dyn FnMut(&PyClassItems)),
@ -94,20 +88,8 @@ unsafe fn create_type_object_impl(
push_slot(&mut slots, ffi::Py_tp_doc, doc as _);
}
push_slot(
&mut slots,
ffi::Py_tp_new,
tp_new.unwrap_or(fallback_new) as _,
);
push_slot(&mut slots, ffi::Py_tp_dealloc, tp_dealloc as _);
if let Some(alloc) = tp_alloc {
push_slot(&mut slots, ffi::Py_tp_alloc, alloc as _);
}
if let Some(free) = tp_free {
push_slot(&mut slots, ffi::Py_tp_free, free as _);
}
#[cfg(Py_3_9)]
{
let members = py_class_members(dict_offset, weaklist_offset);
@ -132,6 +114,7 @@ unsafe fn create_type_object_impl(
}
// protocol methods
let mut has_new = false;
let mut has_getitem = false;
let mut has_setitem = false;
let mut has_gc_methods = false;
@ -142,6 +125,7 @@ unsafe fn create_type_object_impl(
for_all_items(&mut |items| {
for slot in items.slots {
has_new |= slot.slot == ffi::Py_tp_new;
has_getitem |= slot.slot == ffi::Py_mp_subscript;
has_setitem |= slot.slot == ffi::Py_mp_ass_subscript;
has_gc_methods |= slot.slot == ffi::Py_tp_clear || slot.slot == ffi::Py_tp_traverse;
@ -182,6 +166,10 @@ unsafe fn create_type_object_impl(
);
}
if !has_new {
push_slot(&mut slots, ffi::Py_tp_new, no_constructor_defined as _);
}
// Add empty sentinel at the end
push_slot(&mut slots, 0, ptr::null_mut());
@ -561,3 +549,16 @@ where
}
}
}
/// Default new implementation
pub(crate) unsafe extern "C" fn no_constructor_defined(
_subtype: *mut ffi::PyTypeObject,
_args: *mut ffi::PyObject,
_kwds: *mut ffi::PyObject,
) -> *mut ffi::PyObject {
crate::callback_body!(py, {
Err::<(), _>(crate::exceptions::PyTypeError::new_err(
"No constructor defined",
))
})
}

View file

@ -119,4 +119,15 @@ impl MyClass {
fn default_arg_before_required(&self, has_default: isize, required: isize) {}
}
struct TwoNew { }
#[pymethods]
impl TwoNew {
#[new]
fn new_1() -> Self { Self { } }
#[new]
fn new_2() -> Self { Self { } }
}
fn main() {}

View file

@ -101,3 +101,14 @@ error: `pass_module` cannot be used on Python methods
|
112 | #[pyo3(pass_module)]
| ^^^^^^^^^^^
error[E0592]: duplicate definitions with name `__pymethod__new__`
--> tests/ui/invalid_pymethods.rs:124:1
|
124 | #[pymethods]
| ^^^^^^^^^^^^
| |
| duplicate definitions for `__pymethod__new__`
| other definition for `__pymethod__new__`
|
= note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)