From 926e88980ebf772f20d2ace1952d1c79b62e7efa Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Sat, 25 Jun 2022 07:45:41 +0100 Subject: [PATCH] llvm-lines: use iterator to collect class items --- guide/src/class.md | 5 +- pyo3-macros-backend/src/pyclass.rs | 41 ++-- src/impl_/pyclass.rs | 208 +++++++++++++++++++- src/pyclass.rs | 21 +- src/type_object.rs | 14 +- tests/ui/abi3_nativetype_inheritance.stderr | 22 +-- tests/ui/pyclass_send.stderr | 30 +-- 7 files changed, 270 insertions(+), 71 deletions(-) diff --git a/guide/src/class.md b/guide/src/class.md index b87d8b0b4d8..c45666da672 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -1001,12 +1001,11 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass { type WeakRef = ::pyo3::impl_::pyclass::PyClassDummySlot; type BaseNativeType = ::pyo3::PyAny; - fn for_all_items(visitor: &mut dyn FnMut(&pyo3::impl_::pyclass::PyClassItems)) { + fn items_iter() -> pyo3::impl_::pyclass::PyClassItemsIter { use pyo3::impl_::pyclass::*; let collector = PyClassImplCollector::::new(); static INTRINSIC_ITEMS: PyClassItems = PyClassItems { slots: &[], methods: &[] }; - visitor(&INTRINSIC_ITEMS); - visitor(collector.py_methods()); + PyClassItemsIter::new(&INTRINSIC_ITEMS, collector.py_methods()) } } diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index bc1f5fa247b..8041e46977b 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -859,9 +859,7 @@ impl<'a> PyClassImplsBuilder<'a> { }; let (pymethods_items, inventory, inventory_class) = match self.methods_type { - PyClassMethodsType::Specialization => { - (quote! { visitor(collector.py_methods()); }, None, None) - } + PyClassMethodsType::Specialization => (quote! { collector.py_methods() }, None, None), PyClassMethodsType::Inventory => { // To allow multiple #[pymethods] block, we define inventory types. let inventory_class_name = syn::Ident::new( @@ -870,9 +868,12 @@ impl<'a> PyClassImplsBuilder<'a> { ); ( quote! { - for inventory in _pyo3::inventory::iter::<::Inventory>() { - visitor(_pyo3::impl_::pyclass::PyClassInventory::items(inventory)); - } + ::std::boxed::Box::new( + ::std::iter::Iterator::map( + _pyo3::inventory::iter::<::Inventory>(), + _pyo3::impl_::pyclass::PyClassInventory::items + ) + ) }, Some(quote! { type Inventory = #inventory_class_name; }), Some(define_inventory_class(&inventory_class_name)), @@ -882,15 +883,15 @@ impl<'a> PyClassImplsBuilder<'a> { let pyproto_items = if cfg!(feature = "pyproto") { Some(quote! { - visitor(collector.object_protocol_items()); - visitor(collector.number_protocol_items()); - visitor(collector.iter_protocol_items()); - visitor(collector.gc_protocol_items()); - visitor(collector.descr_protocol_items()); - visitor(collector.mapping_protocol_items()); - visitor(collector.sequence_protocol_items()); - visitor(collector.async_protocol_items()); - visitor(collector.buffer_protocol_items()); + collector.object_protocol_items(), + collector.number_protocol_items(), + collector.iter_protocol_items(), + collector.gc_protocol_items(), + collector.descr_protocol_items(), + collector.mapping_protocol_items(), + collector.sequence_protocol_items(), + collector.async_protocol_items(), + collector.buffer_protocol_items(), }) } else { None @@ -959,7 +960,7 @@ impl<'a> PyClassImplsBuilder<'a> { type WeakRef = #weakref; type BaseNativeType = #base_nativetype; - fn for_all_items(visitor: &mut dyn ::std::ops::FnMut(& _pyo3::impl_::pyclass::PyClassItems)) { + fn items_iter() -> _pyo3::impl_::pyclass::PyClassItemsIter { use _pyo3::impl_::pyclass::*; let collector = PyClassImplCollector::::new(); #deprecations; @@ -967,9 +968,11 @@ impl<'a> PyClassImplsBuilder<'a> { methods: &[#(#default_method_defs),*], slots: &[#(#default_slot_defs),* #(#freelist_slots),*], }; - visitor(&INTRINSIC_ITEMS); - #pymethods_items - #pyproto_items + PyClassItemsIter::new( + &INTRINSIC_ITEMS, + #pymethods_items, + #pyproto_items + ) } #dict_offset diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index 5925f17c5bc..01b1e722d74 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -177,7 +177,7 @@ pub trait PyClassImpl: Sized { #[cfg(feature = "multiple-pymethods")] type Inventory: PyClassInventory; - fn for_all_items(visitor: &mut dyn FnMut(&PyClassItems)); + fn items_iter() -> PyClassItemsIter; #[inline] fn dict_offset() -> Option { @@ -189,6 +189,208 @@ pub trait PyClassImpl: Sized { } } +/// Iterator used to process all class items during type instantiation. +pub struct PyClassItemsIter { + /// Iteration state + idx: usize, + /// Items from the `#[pyclass]` macro + pyclass_items: &'static PyClassItems, + /// Items from the `#[pymethods]` macro + #[cfg(not(feature = "multiple-pymethods"))] + pymethods_items: &'static PyClassItems, + /// Items from the `#[pymethods]` macro with inventory + #[cfg(feature = "multiple-pymethods")] + pymethods_items: Box>, + + // pyproto items, to be removed soon. + #[cfg(feature = "pyproto")] + object_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] + descr_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] + gc_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] + iter_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] + mapping_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] + number_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] + async_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] + sequence_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] + buffer_protocol_items: &'static PyClassItems, +} + +impl PyClassItemsIter { + #[cfg_attr(feature = "pyproto", allow(clippy::too_many_arguments))] + pub fn new( + pyclass_items: &'static PyClassItems, + #[cfg(not(feature = "multiple-pymethods"))] pymethods_items: &'static PyClassItems, + #[cfg(feature = "multiple-pymethods")] pymethods_items: Box< + dyn Iterator, + >, + #[cfg(feature = "pyproto")] object_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] descr_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] gc_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] iter_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] mapping_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] number_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] async_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] sequence_protocol_items: &'static PyClassItems, + #[cfg(feature = "pyproto")] buffer_protocol_items: &'static PyClassItems, + ) -> Self { + Self { + idx: 0, + pyclass_items, + pymethods_items, + #[cfg(feature = "pyproto")] + object_protocol_items, + #[cfg(feature = "pyproto")] + descr_protocol_items, + #[cfg(feature = "pyproto")] + gc_protocol_items, + #[cfg(feature = "pyproto")] + iter_protocol_items, + #[cfg(feature = "pyproto")] + mapping_protocol_items, + #[cfg(feature = "pyproto")] + number_protocol_items, + #[cfg(feature = "pyproto")] + async_protocol_items, + #[cfg(feature = "pyproto")] + sequence_protocol_items, + #[cfg(feature = "pyproto")] + buffer_protocol_items, + } + } +} + +impl Iterator for PyClassItemsIter { + type Item = &'static PyClassItems; + + #[cfg(not(feature = "multiple-pymethods"))] + fn next(&mut self) -> Option { + match self.idx { + 0 => { + self.idx += 1; + Some(self.pyclass_items) + } + 1 => { + self.idx += 1; + Some(self.pymethods_items) + } + #[cfg(feature = "pyproto")] + 2 => { + self.idx += 1; + Some(self.object_protocol_items) + } + #[cfg(feature = "pyproto")] + 3 => { + self.idx += 1; + Some(self.descr_protocol_items) + } + #[cfg(feature = "pyproto")] + 4 => { + self.idx += 1; + Some(self.gc_protocol_items) + } + #[cfg(feature = "pyproto")] + 5 => { + self.idx += 1; + Some(self.iter_protocol_items) + } + #[cfg(feature = "pyproto")] + 6 => { + self.idx += 1; + Some(self.mapping_protocol_items) + } + #[cfg(feature = "pyproto")] + 7 => { + self.idx += 1; + Some(self.number_protocol_items) + } + #[cfg(feature = "pyproto")] + 8 => { + self.idx += 1; + Some(self.async_protocol_items) + } + #[cfg(feature = "pyproto")] + 9 => { + self.idx += 1; + Some(self.sequence_protocol_items) + } + #[cfg(feature = "pyproto")] + 10 => { + self.idx += 1; + Some(self.buffer_protocol_items) + } + // Termination clause + // NB self.idx reaches different final value (2 vs 11) depending on pyproto feature + _ => None, + } + } + + #[cfg(feature = "multiple-pymethods")] + fn next(&mut self) -> Option { + match self.idx { + 0 => { + self.idx += 1; + Some(self.pyclass_items) + } + #[cfg(feature = "pyproto")] + 1 => { + self.idx += 1; + Some(self.object_protocol_items) + } + #[cfg(feature = "pyproto")] + 2 => { + self.idx += 1; + Some(self.descr_protocol_items) + } + #[cfg(feature = "pyproto")] + 3 => { + self.idx += 1; + Some(self.gc_protocol_items) + } + #[cfg(feature = "pyproto")] + 4 => { + self.idx += 1; + Some(self.iter_protocol_items) + } + #[cfg(feature = "pyproto")] + 5 => { + self.idx += 1; + Some(self.mapping_protocol_items) + } + #[cfg(feature = "pyproto")] + 6 => { + self.idx += 1; + Some(self.number_protocol_items) + } + #[cfg(feature = "pyproto")] + 7 => { + self.idx += 1; + Some(self.async_protocol_items) + } + #[cfg(feature = "pyproto")] + 8 => { + self.idx += 1; + Some(self.sequence_protocol_items) + } + #[cfg(feature = "pyproto")] + 9 => { + self.idx += 1; + Some(self.buffer_protocol_items) + } + // Termination clause + // NB self.idx reaches different final value (1 vs 10) depending on pyproto feature + _ => self.pymethods_items.next(), + } + } +} + // Traits describing known special methods. macro_rules! slot_fragment_trait { @@ -809,10 +1011,6 @@ mod pyproto_traits { #[cfg(feature = "pyproto")] pub use pyproto_traits::*; -// Protocol slots from #[pymethods] if not using inventory. -#[cfg(not(feature = "multiple-pymethods"))] -items_trait!(PyMethodsProtocolItems, methods_protocol_items); - // Thread checkers #[doc(hidden)] diff --git a/src/pyclass.rs b/src/pyclass.rs index 3a537157a48..52201e90eb6 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -5,7 +5,7 @@ use crate::{ ffi, impl_::pyclass::{ assign_sequence_item_from_mapping, get_sequence_item_from_mapping, tp_dealloc, PyClassImpl, - PyClassItems, + PyClassItemsIter, }, IntoPy, IntoPyPointer, PyCell, PyErr, PyMethodDefType, PyObject, PyResult, PyTypeInfo, Python, }; @@ -48,7 +48,7 @@ where tp_dealloc::, T::dict_offset(), T::weaklist_offset(), - &T::for_all_items, + T::items_iter, T::IS_BASETYPE, T::IS_MAPPING, ) @@ -69,7 +69,7 @@ unsafe fn create_type_object_impl( tp_dealloc: ffi::destructor, dict_offset: Option, weaklist_offset: Option, - for_all_items: &dyn Fn(&mut dyn FnMut(&PyClassItems)), + get_items_iter: fn() -> PyClassItemsIter, is_basetype: bool, is_mapping: bool, ) -> PyResult<*mut ffi::PyTypeObject> { @@ -97,7 +97,7 @@ unsafe fn create_type_object_impl( let PyClassInfo { method_defs, property_defs, - } = method_defs_to_pyclass_info(for_all_items, dict_offset.is_none()); + } = method_defs_to_pyclass_info(get_items_iter(), dict_offset.is_none()); // normal methods if !method_defs.is_empty() { @@ -120,7 +120,7 @@ unsafe fn create_type_object_impl( #[cfg(all(not(Py_3_9), not(Py_LIMITED_API)))] let mut buffer_procs: ffi::PyBufferProcs = Default::default(); - for_all_items(&mut |items| { + for items in get_items_iter() { for slot in items.slots { match slot.slot { ffi::Py_tp_new => has_new = true, @@ -142,7 +142,7 @@ unsafe fn create_type_object_impl( } } slots.extend_from_slice(items.slots); - }); + } if !is_mapping { // If mapping methods implemented, define sequence methods get implemented too. @@ -320,14 +320,11 @@ struct PyClassInfo { property_defs: Vec, } -fn method_defs_to_pyclass_info( - for_all_items: &dyn Fn(&mut dyn FnMut(&PyClassItems)), - has_dict: bool, -) -> PyClassInfo { +fn method_defs_to_pyclass_info(items_iter: PyClassItemsIter, has_dict: bool) -> PyClassInfo { let mut method_defs = Vec::new(); let mut property_defs_map = std::collections::HashMap::new(); - for_all_items(&mut |items| { + for items in items_iter { for def in items.methods { match def { PyMethodDefType::Getter(getter) => { @@ -350,7 +347,7 @@ fn method_defs_to_pyclass_info( PyMethodDefType::ClassAttribute(_) => {} } } - }); + } // TODO: use into_values when on MSRV Rust >= 1.54 let mut property_defs: Vec<_> = property_defs_map diff --git a/src/type_object.rs b/src/type_object.rs index 35f6d05c45e..570f579c93d 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -1,7 +1,7 @@ // Copyright (c) 2017-present PyO3 Project and Contributors //! Python type object information -use crate::impl_::pyclass::PyClassItems; +use crate::impl_::pyclass::PyClassItemsIter; use crate::internal_tricks::extract_cstr_or_leak_cstring; use crate::once_cell::GILOnceCell; use crate::pyclass::create_type_object; @@ -113,8 +113,10 @@ impl LazyStaticType { // Uses explicit GILOnceCell::get_or_init:: *mut ffi::PyTypeObject> monomorphization // so that only this one monomorphization is instantiated (instead of one closure monormization for each T). - let type_object = *self.value.get_or_init:: *mut ffi::PyTypeObject>(py, inner::); - self.ensure_init(py, type_object, T::NAME, &T::for_all_items); + let type_object = *self + .value + .get_or_init:: *mut ffi::PyTypeObject>(py, inner::); + self.ensure_init(py, type_object, T::NAME, T::items_iter()); type_object } @@ -123,7 +125,7 @@ impl LazyStaticType { py: Python<'_>, type_object: *mut ffi::PyTypeObject, name: &str, - for_all_items: &dyn Fn(&mut dyn FnMut(&PyClassItems)), + items_iter: PyClassItemsIter, ) { // We might want to fill the `tp_dict` with python instances of `T` // itself. In order to do so, we must first initialize the type object @@ -173,7 +175,7 @@ impl LazyStaticType { // means that another thread can continue the initialization in the // meantime: at worst, we'll just make a useless computation. let mut items = vec![]; - for_all_items(&mut |class_items| { + for class_items in items_iter { for def in class_items.methods { if let PyMethodDefType::ClassAttribute(attr) = def { let key = extract_cstr_or_leak_cstring( @@ -193,7 +195,7 @@ impl LazyStaticType { } } } - }); + } // Now we hold the GIL and we can assume it won't be released until we // return from the function. diff --git a/tests/ui/abi3_nativetype_inheritance.stderr b/tests/ui/abi3_nativetype_inheritance.stderr index 7fd23bd1236..25084a3c969 100644 --- a/tests/ui/abi3_nativetype_inheritance.stderr +++ b/tests/ui/abi3_nativetype_inheritance.stderr @@ -22,15 +22,15 @@ note: required by a bound in `PyRefMut` = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `PyDict: PyClass` is not satisfied - --> tests/ui/abi3_nativetype_inheritance.rs:5:1 - | -5 | #[pyclass(extends=PyDict)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `PyClass` is not implemented for `PyDict` - | - = note: required because of the requirements on the impl of `PyClassBaseType` for `PyDict` + --> tests/ui/abi3_nativetype_inheritance.rs:5:1 + | +5 | #[pyclass(extends=PyDict)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `PyClass` is not implemented for `PyDict` + | + = note: required because of the requirements on the impl of `PyClassBaseType` for `PyDict` note: required by a bound in `ThreadCheckerInherited` - --> src/impl_/pyclass.rs - | - | pub struct ThreadCheckerInherited( - | ^^^^^^^^^^^^^^^ required by this bound in `ThreadCheckerInherited` - = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) + --> src/impl_/pyclass.rs + | + | pub struct ThreadCheckerInherited( + | ^^^^^^^^^^^^^^^ required by this bound in `ThreadCheckerInherited` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/ui/pyclass_send.stderr b/tests/ui/pyclass_send.stderr index 287430ac078..9054de3a978 100644 --- a/tests/ui/pyclass_send.stderr +++ b/tests/ui/pyclass_send.stderr @@ -1,18 +1,18 @@ error[E0277]: `Rc` cannot be sent between threads safely - --> tests/ui/pyclass_send.rs:4:1 - | -4 | #[pyclass] - | ^^^^^^^^^^ `Rc` cannot be sent between threads safely - | - = help: within `NotThreadSafe`, the trait `Send` is not implemented for `Rc` + --> tests/ui/pyclass_send.rs:4:1 + | +4 | #[pyclass] + | ^^^^^^^^^^ `Rc` cannot be sent between threads safely + | + = help: within `NotThreadSafe`, the trait `Send` is not implemented for `Rc` note: required because it appears within the type `NotThreadSafe` - --> tests/ui/pyclass_send.rs:5:8 - | -5 | struct NotThreadSafe { - | ^^^^^^^^^^^^^ + --> tests/ui/pyclass_send.rs:5:8 + | +5 | struct NotThreadSafe { + | ^^^^^^^^^^^^^ note: required by a bound in `ThreadCheckerStub` - --> src/impl_/pyclass.rs - | - | pub struct ThreadCheckerStub(PhantomData); - | ^^^^ required by this bound in `ThreadCheckerStub` - = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) + --> src/impl_/pyclass.rs + | + | pub struct ThreadCheckerStub(PhantomData); + | ^^^^ required by this bound in `ThreadCheckerStub` + = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info)