diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 92cae41a06..fe00787181 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -26,7 +26,7 @@ use crate::{ convert::{ToPyObject, ToPyResult}, coroutine::Coro, exceptions::ExceptionCtor, - function::{ArgMapping, Either, FuncArgs, PyMethodFlags}, + function::{ArgMapping, Either, FuncArgs, KwArgs, PyMethodFlags}, object::PyAtomicBorrow, object::{Traverse, TraverseFn}, protocol::{PyIter, PyIterReturn, PyMapping}, @@ -42,7 +42,6 @@ use core::cell::UnsafeCell; use core::sync::atomic; use core::sync::atomic::AtomicPtr; use core::sync::atomic::Ordering::{Acquire, Relaxed}; -use indexmap::IndexMap; use itertools::Itertools; use malachite_bigint::BigInt; use num_traits::Zero; @@ -6460,7 +6459,7 @@ impl ExecutingFrame<'_> { fn collect_positional_args(&mut self, nargs: u32) -> FuncArgs { FuncArgs { args: self.pop_multiple(nargs as usize).collect(), - kwargs: IndexMap::new(), + ..Default::default() } } @@ -6483,9 +6482,8 @@ impl ExecutingFrame<'_> { fn collect_ex_args(&mut self, vm: &VirtualMachine) -> PyResult { let kwargs_or_null = self.pop_value_opt(); - let kwargs = if let Some(kw_obj) = kwargs_or_null { - let mut kwargs = IndexMap::new(); - + let mut kwargs = KwArgs::default(); + if let Some(kw_obj) = kwargs_or_null { // Stack: [callable, self_or_null, args_tuple] let callable = self.nth_value(2); let func_str = Self::object_function_str(callable, vm); @@ -6497,11 +6495,9 @@ impl ExecutingFrame<'_> { let value = kw_obj.get_item(&*key, vm)?; kwargs.insert(key_str.as_str().to_owned(), value); Ok(()) - })?; - kwargs - } else { - IndexMap::new() + })? }; + let args_obj = self.pop_value(); let args = if let Some(tuple) = args_obj.downcast_ref::() { tuple.as_slice().to_vec() diff --git a/crates/vm/src/function/argument.rs b/crates/vm/src/function/argument.rs index 40408531cb..c37475ba48 100644 --- a/crates/vm/src/function/argument.rs +++ b/crates/vm/src/function/argument.rs @@ -4,7 +4,7 @@ use crate::{ convert::ToPyObject, object::{Traverse, TraverseFn}, }; -use core::ops::RangeInclusive; +use core::ops::{Deref, DerefMut, RangeInclusive}; use indexmap::IndexMap; use itertools::Itertools; @@ -60,20 +60,14 @@ into_func_args_from_tuple!((v1, T1), (v2, T2), (v3, T3), (v4, T4), (v5, T5), (v6 // The number of limitation came from: // https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments -/// The `FuncArgs` struct is one of the most used structs then creating +/// The `FuncArgs` struct is one of the most used structs when creating /// a rust function that can be called from python. It holds both positional /// arguments, as well as keyword arguments passed to the function. #[derive(Debug, Default, Clone, Traverse)] pub struct FuncArgs { pub args: Vec, // sorted map, according to https://www.python.org/dev/peps/pep-0468/ - pub kwargs: IndexMap, -} - -unsafe impl Traverse for IndexMap { - fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { - self.values().for_each(|v| v.traverse(tracer_fn)); - } + pub kwargs: KwArgs, } /// Conversion from vector of python objects to function arguments. @@ -84,7 +78,7 @@ where fn from(args: A) -> Self { Self { args: args.into().into_vec(), - kwargs: IndexMap::new(), + ..Default::default() } } } @@ -92,8 +86,8 @@ where impl From for FuncArgs { fn from(kwargs: KwArgs) -> Self { Self { - args: Vec::new(), - kwargs: kwargs.0, + kwargs, + ..Default::default() } } } @@ -111,8 +105,10 @@ impl FuncArgs { K: Into, { let PosArgs(args) = args.into(); - let KwArgs(kwargs) = kwargs.into(); - Self { args, kwargs } + Self { + args, + kwargs: kwargs.into(), + } } pub fn with_kwargs_names(mut args: A, kwarg_names: KW) -> Self @@ -127,7 +123,7 @@ impl FuncArgs { let pos_args = args.by_ref().take(pos_arg_count).collect(); - let kwargs = kwarg_names.zip_eq(args).collect::>(); + let kwargs = kwarg_names.zip_eq(args).collect(); Self { args: pos_args, @@ -147,8 +143,10 @@ impl FuncArgs { ) -> Self { debug_assert!(nargs <= args.len()); debug_assert!(kwnames.is_none_or(|kw| nargs + kw.len() <= args.len())); + let pos_args = args[..nargs].to_vec(); - let kwargs = if let Some(names) = kwnames { + + let kwargs = kwnames.map_or_else(KwArgs::default, |names| { names .iter() .zip(&args[nargs..nargs + names.len()]) @@ -161,9 +159,8 @@ impl FuncArgs { (key, val.clone()) }) .collect() - } else { - IndexMap::new() - }; + }); + Self { args: pos_args, kwargs, @@ -179,7 +176,7 @@ impl FuncArgs { ) -> Self { debug_assert!(nargs <= args.len()); debug_assert!(kwnames.is_none_or(|kw| nargs + kw.len() <= args.len())); - let kwargs = if let Some(names) = kwnames { + let kwargs = kwnames.map_or_else(KwArgs::default, |names| { let kw_count = names.len(); names .iter() @@ -193,9 +190,8 @@ impl FuncArgs { (key, val) }) .collect() - } else { - IndexMap::new() - }; + }); + args.truncate(nargs); Self { args, kwargs } } @@ -404,15 +400,35 @@ impl FromArgOptional for T { /// KwArgs is only for functions that accept arbitrary keyword arguments. For /// functions that accept only *specific* named arguments, a rust struct with /// an appropriate FromArgs implementation must be created. -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct KwArgs(IndexMap); +impl Default for KwArgs { + fn default() -> Self { + Self(IndexMap::new()) + } +} + +impl Deref for KwArgs { + type Target = IndexMap; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for KwArgs { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + unsafe impl Traverse for KwArgs where T: Traverse, { fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) { - self.0.iter().map(|(_, v)| v.traverse(tracer_fn)).count(); + self.values().for_each(|v| v.traverse(tracer_fn)); } } @@ -423,12 +439,7 @@ impl KwArgs { } pub fn pop_kwarg(&mut self, name: &str) -> Option { - self.0.swap_remove(name) - } - - #[must_use] - pub fn is_empty(self) -> bool { - self.0.is_empty() + self.swap_remove(name) } } @@ -438,9 +449,21 @@ impl FromIterator<(String, T)> for KwArgs { } } -impl Default for KwArgs { - fn default() -> Self { - Self(IndexMap::new()) +impl<'a, T> IntoIterator for &'a KwArgs { + type Item = (&'a String, &'a T); + type IntoIter = indexmap::map::Iter<'a, String, T>; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl IntoIterator for KwArgs { + type Item = (String, T); + type IntoIter = indexmap::map::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() } } @@ -457,15 +480,6 @@ where } } -impl IntoIterator for KwArgs { - type Item = (String, T); - type IntoIter = indexmap::map::IntoIter; - - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() - } -} - /// A list of positional argument values. /// /// A built-in function with a `PosArgs` parameter is analogous to a Python diff --git a/crates/vm/src/stdlib/_operator.rs b/crates/vm/src/stdlib/_operator.rs index 6d7de468b4..e4db046053 100644 --- a/crates/vm/src/stdlib/_operator.rs +++ b/crates/vm/src/stdlib/_operator.rs @@ -6,7 +6,7 @@ mod _operator { AsObject, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, builtins::{PyInt, PyIntRef, PyStr, PyStrRef, PyTupleRef, PyType, PyTypeRef, PyUtf8StrRef}, common::wtf8::{Wtf8, Wtf8Buf}, - function::{ArgBytesLike, Either, FuncArgs, KwArgs, OptionalArg}, + function::{ArgBytesLike, Either, FuncArgs, OptionalArg}, protocol::PyIter, recursion::ReprGuard, types::{Callable, Constructor, PyComparisonOp, Representable}, @@ -564,7 +564,7 @@ mod _operator { let partial = vm.import("functools", 0)?.get_attr("partial", vm)?; let args = FuncArgs::new( vec![zelf.class().to_owned().into(), zelf.name.clone().into()], - KwArgs::new(zelf.args.kwargs.clone()), + zelf.args.kwargs.clone(), ); let callable = partial.call(args, vm)?; Ok(vm.new_tuple((callable, vm.ctx.new_tuple(zelf.args.args.clone()))))