From 87cf891e5062fe518944ceca70e3c0b168cfb16e Mon Sep 17 00:00:00 2001 From: Noa Date: Tue, 14 Nov 2023 21:52:47 -0600 Subject: [PATCH] Make PyMethodDef construction const (#5117) * Make PyMethodDef construction const * Remove iter_chain![] Obsolete since arrays now impl IntoIterator --- derive-impl/src/pyclass.rs | 72 +++++++++++++++++-------------------- derive-impl/src/pymodule.rs | 61 ++++++++++++++++--------------- derive-impl/src/util.rs | 7 ---- vm/src/class.rs | 7 ++-- vm/src/function/builtin.rs | 32 +++++++++++++++++ vm/src/function/method.rs | 51 ++++++++++++++++++++++++++ vm/src/function/mod.rs | 2 +- 7 files changed, 148 insertions(+), 84 deletions(-) diff --git a/derive-impl/src/pyclass.rs b/derive-impl/src/pyclass.rs index 55705c742..8f688b366 100644 --- a/derive-impl/src/pyclass.rs +++ b/derive-impl/src/pyclass.rs @@ -4,7 +4,7 @@ use crate::util::{ ClassItemMeta, ContentItem, ContentItemInner, ErrorVec, ExceptionItemMeta, ItemMeta, ItemMetaInner, ItemNursery, SimpleItemMeta, ALL_ALLOWED_NAMES, }; -use proc_macro2::{Span, TokenStream}; +use proc_macro2::{Delimiter, Group, Span, TokenStream, TokenTree}; use quote::{quote, quote_spanned, ToTokens}; use std::collections::{HashMap, HashSet}; use std::str::FromStr; @@ -172,14 +172,9 @@ pub(crate) fn impl_pyclass_impl(attr: AttributeArgs, item: Item) -> Result, - ) { - #method_def - } + const __OWN_METHOD_DEFS: &'static [::rustpython_vm::function::PyMethodDef] = &#method_def; }, parse_quote! { fn __extend_py_class( @@ -201,6 +196,15 @@ pub(crate) fn impl_pyclass_impl(attr: AttributeArgs, item: Item) -> Result(&[#impl_ty::__OWN_METHOD_DEFS, #(#with_method_defs,)*]) + ) + }; quote! { #imp impl ::rustpython_vm::class::PyClassImpl for #payload_ty { @@ -214,13 +218,7 @@ pub(crate) fn impl_pyclass_impl(attr: AttributeArgs, item: Item) -> Result, - ) { - #impl_ty::__extend_method_def(method_defs); - #with_method_defs - } + const METHOD_DEFS: &'static [::rustpython_vm::function::PyMethodDef] = &#method_defs; fn extend_slots(slots: &mut ::rustpython_vm::types::PyTypeSlots) { #impl_ty::__extend_slots(slots); @@ -268,14 +266,9 @@ pub(crate) fn impl_pyclass_impl(attr: AttributeArgs, item: Item) -> Result, - ) { - #method_def - } + const __OWN_METHOD_DEFS: &'static [::rustpython_vm::function::PyMethodDef] = &#method_def; }, parse_quote! { fn __extend_py_class( @@ -983,6 +976,7 @@ impl MethodNursery { impl ToTokens for MethodNursery { fn to_tokens(&self, tokens: &mut TokenStream) { + let mut inner_tokens = TokenStream::new(); for item in &self.items { let py_name = &item.py_name; let ident = &item.ident; @@ -1011,16 +1005,18 @@ impl ToTokens for MethodNursery { // } else { // quote_spanned! { ident.span() => #py_name } // }; - tokens.extend(quote! { + inner_tokens.extend(quote! [ #(#cfgs)* - method_defs.push(rustpython_vm::function::PyMethodDef { - name: #py_name, - func: rustpython_vm::function::IntoPyNativeFn::into_func(Self::#ident), - flags: #flags, - doc: #doc, - }); - }); + rustpython_vm::function::PyMethodDef::new_const( + #py_name, + Self::#ident, + #flags, + #doc, + ), + ]); } + let array: TokenTree = Group::new(Delimiter::Bracket, inner_tokens).into(); + tokens.extend([array]); } } @@ -1436,7 +1432,7 @@ struct ExtractedImplAttrs { payload: Option, flags: TokenStream, with_impl: TokenStream, - with_method_defs: TokenStream, + with_method_defs: Vec, with_slots: TokenStream, } @@ -1465,18 +1461,18 @@ fn extract_impl_attrs(attr: AttributeArgs, item: &Ident) -> Result::__extend_py_class), - quote!(#path::::__extend_method_def), + quote!(#path::::__OWN_METHOD_DEFS), quote!(#path::::__extend_slots), ) } else { ( quote!(::__extend_py_class), - quote!(::__extend_method_def), + quote!(::__OWN_METHOD_DEFS), quote!(::__extend_slots), ) }; @@ -1484,9 +1480,7 @@ fn extract_impl_attrs(attr: AttributeArgs, item: &Ident) -> Result #extend_class(ctx, class); }); - with_method_defs.push(quote_spanned! { path.span() => - #extend_method_def(method_defs); - }); + with_method_defs.push(method_defs); with_slots.push(quote_spanned! { item_span => #extend_slots(slots); }); @@ -1530,9 +1524,7 @@ fn extract_impl_attrs(attr: AttributeArgs, item: &Ident) -> Result Result Result &'static ::rustpython_vm::builtins::PyModuleDef { DEF.get_or_init(|| { - #[allow(clippy::ptr_arg)] - let method_defs = { - let mut method_defs = Vec::new(); - extend_method_def(ctx, &mut method_defs); - method_defs - }; let mut def = ::rustpython_vm::builtins::PyModuleDef { name: ctx.intern_str(MODULE_NAME), doc: DOC.map(|doc| ctx.intern_str(doc)), - methods: Box::leak(method_defs.into_boxed_slice()), + methods: METHOD_DEFS, slots: Default::default(), }; def.slots.exec = Some(extend_module); @@ -161,23 +155,24 @@ pub fn impl_pymodule(attr: AttributeArgs, module_item: Item) -> Result(&[#(super::#withs::METHOD_DEFS,)* OWN_METHODS]) + }) + }; + items.extend([ parse_quote! { ::rustpython_vm::common::static_cell! { pub(crate) static DEF: ::rustpython_vm::builtins::PyModuleDef; } }, parse_quote! { - #[allow(clippy::ptr_arg)] - pub(crate) fn extend_method_def( - ctx: &::rustpython_vm::Context, - method_defs: &mut Vec<::rustpython_vm::function::PyMethodDef>, - ) { - #( - super::#withs::extend_method_def(ctx, method_defs); - )* - #function_items - } + pub(crate) const METHOD_DEFS: &'static [::rustpython_vm::function::PyMethodDef] = &#method_defs; }, parse_quote! { pub(crate) fn __init_attributes( @@ -361,26 +356,30 @@ struct ValidatedFunctionNursery(FunctionNursery); impl ToTokens for ValidatedFunctionNursery { fn to_tokens(&self, tokens: &mut TokenStream) { + let mut inner_tokens = TokenStream::new(); + let flags = quote! { rustpython_vm::function::PyMethodFlags::empty() }; for item in &self.0.items { let ident = &item.ident; let cfgs = &item.cfgs; + let cfgs = quote!(#(#cfgs)*); let py_names = &item.py_names; let doc = &item.doc; - let flags = quote! { rustpython_vm::function::PyMethodFlags::empty() }; + let doc = quote!(Some(#doc)); - tokens.extend(quote! { - #(#cfgs)* - { - let doc = Some(#doc); - #(method_defs.push(rustpython_vm::function::PyMethodDef::new( - (#py_names), + inner_tokens.extend(quote![ + #( + #cfgs + rustpython_vm::function::PyMethodDef::new_const( + #py_names, #ident, #flags, - doc, - ));)* - } - }); + #doc, + ), + )* + ]); } + let array: TokenTree = Group::new(Delimiter::Bracket, inner_tokens).into(); + tokens.extend([array]); } } diff --git a/derive-impl/src/util.rs b/derive-impl/src/util.rs index 9f827f9e9..916b19db0 100644 --- a/derive-impl/src/util.rs +++ b/derive-impl/src/util.rs @@ -639,13 +639,6 @@ impl ErrorVec for Vec { } } -macro_rules! iter_chain { - ($($it:expr),*$(,)?) => { - ::std::iter::empty() - $(.chain(::std::iter::once($it)))* - }; -} - pub(crate) fn iter_use_idents<'a, F, R: 'a>(item_use: &'a syn::ItemUse, mut f: F) -> Result> where F: FnMut(&'a syn::Ident, bool) -> Result, diff --git a/vm/src/class.rs b/vm/src/class.rs index 8b10c5e62..a2eea2121 100644 --- a/vm/src/class.rs +++ b/vm/src/class.rs @@ -135,19 +135,16 @@ pub trait PyClassImpl: PyClassDef { } fn impl_extend_class(ctx: &Context, class: &'static Py); - fn impl_extend_method_def(method_defs: &mut Vec); + const METHOD_DEFS: &'static [PyMethodDef]; fn extend_slots(slots: &mut PyTypeSlots); fn make_slots() -> PyTypeSlots { - let mut method_defs = Vec::new(); - Self::impl_extend_method_def(&mut method_defs); - let mut slots = PyTypeSlots { flags: Self::TP_FLAGS, name: Self::TP_NAME, basicsize: Self::BASICSIZE, doc: Self::DOC, - methods: Box::leak(method_defs.into_boxed_slice()), + methods: Self::METHOD_DEFS, ..Default::default() }; diff --git a/vm/src/function/builtin.rs b/vm/src/function/builtin.rs index e37270bcc..3faaf594f 100644 --- a/vm/src/function/builtin.rs +++ b/vm/src/function/builtin.rs @@ -30,6 +30,7 @@ pub type PyNativeFn = py_dyn_fn!(dyn Fn(&VirtualMachine, FuncArgs) -> PyResult); /// `fn foo(f: F) where F: IntoPyNativeFn` pub trait IntoPyNativeFn: Sized + PyThreadingConstraint + 'static { fn call(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult; + /// `IntoPyNativeFn::into_func()` generates a PyNativeFn that performs the /// appropriate type and arity checking, any requested conversions, and then if /// successful calls the function with the extracted parameters. @@ -37,6 +38,36 @@ pub trait IntoPyNativeFn: Sized + PyThreadingConstraint + 'static { let boxed = Box::new(move |vm: &VirtualMachine, args| self.call(vm, args)); Box::leak(boxed) } + + /// Equivalent to `into_func()`, but accessible as a constant. This is only + /// valid if this function is zero-sized, i.e. that + /// `std::mem::size_of::() == 0`. If it isn't, use of this constant will + /// raise a compile error. + const STATIC_FUNC: &'static PyNativeFn = { + if std::mem::size_of::() == 0 { + &|vm, args| { + // SAFETY: we just confirmed that Self is zero-sized, so there + // aren't any bytes in it that could be uninit. + #[allow(clippy::uninit_assumed_init)] + let f = unsafe { std::mem::MaybeUninit::::uninit().assume_init() }; + f.call(vm, args) + } + } else { + panic!("function must be zero-sized to access STATIC_FUNC") + } + }; +} + +/// Get the [`STATIC_FUNC`](IntoPyNativeFn::STATIC_FUNC) of the passed function. The same +/// requirements of zero-sizedness apply, see that documentation for details. +#[inline(always)] +pub const fn static_func>(f: F) -> &'static PyNativeFn { + // if f is zero-sized, there's no issue forgetting it - even if a capture of f does have a Drop + // impl, it would never get called anyway. If you passed it to into_func, it would just get + // Box::leak'd, and as a 'static reference it'll never be dropped. and if f isn't zero-sized, + // we'll never reach this point anyway because we'll fail to compile. + std::mem::forget(f); + F::STATIC_FUNC } // TODO: once higher-rank trait bounds are stabilized, remove the `Kind` type @@ -186,5 +217,6 @@ mod tests { check_zst(py_func.into_func()); let empty_closure = || "foo".to_owned(); check_zst(empty_closure.into_func()); + check_zst(static_func(empty_closure)); } } diff --git a/vm/src/function/method.rs b/vm/src/function/method.rs index 5922d1b19..fa47c16f4 100644 --- a/vm/src/function/method.rs +++ b/vm/src/function/method.rs @@ -86,6 +86,22 @@ impl PyMethodDef { doc, } } + + #[inline] + pub const fn new_const( + name: &'static str, + func: impl IntoPyNativeFn, + flags: PyMethodFlags, + doc: Option<&'static str>, + ) -> Self { + Self { + name, + func: super::static_func(func), + flags, + doc, + } + } + pub fn to_proper_method( &'static self, class: &'static Py, @@ -192,6 +208,41 @@ impl PyMethodDef { let func = self.to_function(); PyNativeMethod { func, class }.into_ref(ctx) } + + #[doc(hidden)] + pub const fn __const_concat_arrays( + method_groups: &[&[Self]], + ) -> [Self; SUM_LEN] { + const NULL_METHOD: PyMethodDef = PyMethodDef { + name: "", + func: &|_, _| unreachable!(), + flags: PyMethodFlags::empty(), + doc: None, + }; + let mut all_methods = [NULL_METHOD; SUM_LEN]; + let mut all_idx = 0; + let mut group_idx = 0; + while group_idx < method_groups.len() { + let group = method_groups[group_idx]; + let mut method_idx = 0; + while method_idx < group.len() { + all_methods[all_idx] = group[method_idx].const_copy(); + method_idx += 1; + all_idx += 1; + } + group_idx += 1; + } + all_methods + } + + const fn const_copy(&self) -> Self { + Self { + name: self.name, + func: self.func, + flags: self.flags, + doc: self.doc, + } + } } impl std::fmt::Debug for PyMethodDef { diff --git a/vm/src/function/mod.rs b/vm/src/function/mod.rs index 1c3babd80..a0a20c996 100644 --- a/vm/src/function/mod.rs +++ b/vm/src/function/mod.rs @@ -15,7 +15,7 @@ pub use argument::{ }; pub use arithmetic::{PyArithmeticValue, PyComparisonValue}; pub use buffer::{ArgAsciiBuffer, ArgBytesLike, ArgMemoryBuffer, ArgStrOrBytesLike}; -pub use builtin::{IntoPyNativeFn, PyNativeFn}; +pub use builtin::{static_func, IntoPyNativeFn, PyNativeFn}; pub use either::Either; pub use fspath::FsPath; pub use getset::PySetterValue;