FuncArgs kwargs as optional

RustPython · Oct 16, 2020 · 9bf3d80 · 9bf3d80
1 parent 5a2e9a0
commit 9bf3d80
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 67 deletions.
diff --git a/vm/src/builtins/function.rs b/vm/src/builtins/function.rs
@@ -127,26 127,29 @@ impl PyFunction {
 
  let mut posonly_passed_as_kwarg = Vec::new();
  // Handle keyword arguments
- for (name, value) in func_args.kwargs {
- // Check if we have a parameter with this name:
- let dict = if code_object.arg_names.contains(&name)
- || code_object.kwonlyarg_names.contains(&name)
- {
- if posonly_args.contains(&name) {
- posonly_passed_as_kwarg.push(name);
- continue;
- } else if locals.contains_key(&name, vm) {
- return Err(
- vm.new_type_error(format!("Got multiple values for argument '{}'", name))
- );
- }
- locals
- } else {
- kwargs.as_ref().ok_or_else(|| {
- vm.new_type_error(format!("Got an unexpected keyword argument '{}'", name))
- })?
- };
- dict.set_item(name.as_str(), value, vm)?;
  if let Some(func_kwargs) = func_args.kwargs {
  for (name, value) in func_kwargs {
  // Check if we have a parameter with this name:
  let dict = if code_object.arg_names.contains(&name)
  || code_object.kwonlyarg_names.contains(&name)
  {
  if posonly_args.contains(&name) {
  posonly_passed_as_kwarg.push(name);
  continue;
  } else if locals.contains_key(&name, vm) {
  return Err(vm.new_type_error(format!(
  "Got multiple values for argument '{}'",
  name
  )));
  }
  locals
  } else {
  kwargs.as_ref().ok_or_else(|| {
  vm.new_type_error(format!("Got an unexpected keyword argument '{}'", name))
  })?
  };
  dict.set_item(name.as_str(), value, vm)?;
  }
  }
  if !posonly_passed_as_kwarg.is_empty() {
  return Err(vm.new_type_error(format!(

diff --git a/vm/src/builtins/make_module.rs b/vm/src/builtins/make_module.rs
@@ -458,12 458,14 @@ mod decl {
  fn max(mut args: FuncArgs, vm: &VirtualMachine) -> PyResult {
  let default = args.take_keyword("default");
  let key_func = args.take_keyword("key");
- if !args.kwargs.is_empty() {
- let invalid_keyword = args.kwargs.get_index(0).unwrap();
- return Err(vm.new_type_error(format!(
- "'{}' is an invalid keyword argument for max()",
- invalid_keyword.0
- )));
  if let Some(kwargs) = args.kwargs {
  if !kwargs.is_empty() {
  let invalid_keyword = kwargs.get_index(0).unwrap();
  return Err(vm.new_type_error(format!(
  "'{}' is an invalid keyword argument for max()",
  invalid_keyword.0
  )));
  }
  }
  let candidates = match args.args.len().cmp(&1) {
  std::cmp::Ordering::Greater => {
@@ -524,12 526,12 @@ mod decl {
  };
 
  if candidates.is_empty() {
- let default = args.get_optional_kwarg("default");
  let default = args.get_kwarg("default").cloned();
  return default
  .ok_or_else(|| vm.new_value_error("min() arg is an empty sequence".to_owned()));
  }
 
- let key_func = args.get_optional_kwarg("key");
  let key_func = args.get_kwarg("key");
 
  let mut candidates_iter = candidates.into_iter();
  let mut x = candidates_iter.next().unwrap();

diff --git a/vm/src/builtins/pytype.rs b/vm/src/builtins/pytype.rs
@@ -368,7 368,10 @@ impl PyType {
  vm_trace!("type.__new__ {:?}", args);
 
  let is_type_type = metatype.is(&vm.ctx.types.type_type);
- if is_type_type && args.args.len() == 1 && args.kwargs.is_empty() {
  if is_type_type
  && args.args.len() == 1
  && args.kwargs.as_ref().map_or(true, |kw| kw.is_empty())
  {
  return Ok(args.args[0].clone_class().into_object());
  }
 
@@ -550,7 553,10 @@ impl Callable for PyType {
  vm_trace!("type_call: {:?}", zelf);
  let obj = call_tp_new(zelf.clone(), zelf.clone(), args.clone(), vm)?;
 
- if (zelf.is(&vm.ctx.types.type_type) && args.kwargs.is_empty()) || !obj.isinstance(&zelf) {
  if (zelf.is(&vm.ctx.types.type_type)
  && args.kwargs.as_ref().map_or(false, |kw| kw.is_empty()))
  || !obj.isinstance(&zelf)
  {
  return Ok(obj);
  }
 

diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs
@@ -626,12 626,12 @@ fn import_error_init(exc_self: PyObjectRef, args: FuncArgs, vm: &VirtualMachine)
  vm.set_attr(
  &exc_self,
  "name",
- vm.unwrap_or_none(args.kwargs.get("name").cloned()),
  vm.unwrap_or_none(args.get_kwarg("name").cloned()),
  )?;
  vm.set_attr(
  &exc_self,
  "path",
- vm.unwrap_or_none(args.kwargs.get("path").cloned()),
  vm.unwrap_or_none(args.get_kwarg("path").cloned()),
  )?;
  Ok(())
 }

diff --git a/vm/src/format.rs b/vm/src/format.rs
@@ -872,7 872,8 @@ impl FormatString {
  .ok_or_else(|| vm.new_index_error("tuple index out of range".to_owned()))
  }
  FieldType::Keyword(keyword) => arguments
- .get_optional_kwarg(&keyword)
  .get_kwarg(&keyword)
  .cloned()
  .ok_or_else(|| vm.new_key_error(vm.ctx.new_str(keyword))),
  })
  }

diff --git a/vm/src/frame.rs b/vm/src/frame.rs
@@ -1052,10 1052,7 @@ impl ExecutingFrame<'_> {
  let args = match typ {
  bytecode::CallType::Positional(count) => {
  let args: Vec<PyObjectRef> = self.pop_multiple(*count);
- FuncArgs {
- args,
- kwargs: IndexMap::new(),
- }
  FuncArgs { args, kwargs: None }
  }
  bytecode::CallType::Keyword(count) => {
  let kwarg_names = self.pop_value();
@@ -1081,9 1078,9 @@ impl ExecutingFrame<'_> {
  })?;
  kwargs.insert(key.borrow_value().to_owned(), value);
  }
- kwargs
  Some(kwargs)
  } else {
- IndexMap::new()
  None
  };
  let args = self.pop_value();
  let args = vm.extract_elements(&args)?;

diff --git a/vm/src/function.rs b/vm/src/function.rs
@@ -55,7 55,7 @@ into_func_args_from_tuple!((v1, T1), (v2, T2), (v3, T3), (v4, T4), (v5, T5));
 pub struct FuncArgs {
  pub args: Vec<PyObjectRef>,
  // sorted map, according to https://www.python.org/dev/peps/pep-0468/
- pub kwargs: IndexMap<String, PyObjectRef>,
  pub kwargs: Option<IndexMap<String, PyObjectRef>>,
 }
 
 /// Conversion from vector of python objects to function arguments.
@@ -66,7 66,7 @@ where
  fn from(args: A) -> Self {
  FuncArgs {
  args: args.into().into_vec(),
- kwargs: IndexMap::new(),
  kwargs: None,
  }
  }
 }
@@ -75,7 75,7 @@ impl From<KwArgs> for FuncArgs {
  fn from(kwargs: KwArgs) -> Self {
  FuncArgs {
  args: Vec::new(),
- kwargs: kwargs.0,
  kwargs: Some(kwargs.0),
  }
  }
 }
@@ -94,18 94,26 @@ impl FuncArgs {
  {
  let Args(args) = args.into();
  let KwArgs(kwargs) = kwargs.into();
- Self { args, kwargs }
  debug_assert!(!kwargs.is_empty()); // if empty, use From<Args> to avoid index map creation
  Self {
  args,
  kwargs: Some(kwargs),
  }
  }
 
  pub fn with_kwargs_names(mut args: Vec<PyObjectRef>, kwarg_names: Vec<String>) -> Self {
- // last `kwarg_names.len()` elements of args in order of appearance in the call signature
  debug_assert!(!kwarg_names.is_empty()); // if empty, use From<Args> to avoid index map creation
  // last `kwarg_names.len()` elements of args in order of appearance in the call signature
  let kwarg_values = args.drain((args.len() - kwarg_names.len())..);
 
  let mut kwargs = IndexMap::new();
  for (name, value) in kwarg_names.iter().zip(kwarg_values) {
  kwargs.insert(name.clone(), value);
  }
- FuncArgs { args, kwargs }
  FuncArgs {
  args,
  kwargs: Some(kwargs),
  }
  }
 
  pub fn insert(&self, item: PyObjectRef) -> FuncArgs {
@@ -121,24 129,17 @@ impl FuncArgs {
  self.args.remove(0)
  }
 
- pub fn get_kwarg(&self, key: &str, default: PyObjectRef) -> PyObjectRef {
- self.kwargs
- .get(key)
- .cloned()
- .unwrap_or_else(|| default.clone())
- }
-
- pub fn get_optional_kwarg(&self, key: &str) -> Option<PyObjectRef> {
- self.kwargs.get(key).cloned()
  pub fn get_kwarg(&self, key: &str) -> Option<&PyObjectRef> {
  self.kwargs.as_ref().map_or(None, |kw| kw.get(key))
  }
 
- pub fn get_optional_kwarg_with_type(
  pub fn get_kwarg_with_type(
  &self,
  key: &str,
  ty: PyTypeRef,
  vm: &VirtualMachine,
- ) -> PyResult<Option<PyObjectRef>> {
- match self.get_optional_kwarg(key) {
  ) -> PyResult<Option<&PyObjectRef>> {
  match self.get_kwarg(key) {
  Some(kwarg) => {
  if kwarg.isinstance(&ty) {
  Ok(Some(kwarg))
@@ -168,13 169,16 @@ impl FuncArgs {
  }
 
  pub fn take_keyword(&mut self, name: &str) -> Option<PyObjectRef> {
- self.kwargs.swap_remove(name)
  self.kwargs.as_mut().map_or(None, |kw| kw.swap_remove(name))
  }
 
  pub fn remaining_keywords<'a>(
  &'a mut self,
- ) -> impl Iterator<Item = (String, PyObjectRef)> 'a {
- self.kwargs.drain(..)
  ) -> Option<impl Iterator<Item = (String, PyObjectRef)> 'a> {
  match &mut self.kwargs {
  Some(kw) => Some(kw.drain(..)),
  None => None,
  }
  }
 
  /// Binds these arguments to their respective values.
@@ -213,10 217,16 @@ impl FuncArgs {
  T::arity().end(),
  given_args,
  )))
- } else if let Some(k) = self.kwargs.keys().next() {
- Err(vm.new_type_error(format!("Unexpected keyword argument {}", k)))
  } else {
- Ok(bound)
  if let Some(kw) = self.kwargs {
  if let Some(k) = kw.keys().next() {
  Err(vm.new_type_error(format!("Unexpected keyword argument {}", k)))
  } else {
  Ok(bound)
  }
  } else {
  Ok(bound)
  }
  }
  }
 }
@@ -317,8 327,10 @@ where
 {
  fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result<Self, ArgumentError> {
  let mut kwargs = IndexMap::new();
- for (name, value) in args.remaining_keywords() {
- kwargs.insert(name, T::try_from_object(vm, value)?);
  if let Some(func_kwargs) = args.remaining_keywords() {
  for (name, value) in func_kwargs {
  kwargs.insert(name, T::try_from_object(vm, value)?);
  }
  }
  Ok(KwArgs(kwargs))
  }

diff --git a/vm/src/stdlib/csv.rs b/vm/src/stdlib/csv.rs
@@ -28,7 28,7 @@ struct ReaderOption {
 
 impl ReaderOption {
  fn new(args: FuncArgs, vm: &VirtualMachine) -> PyResult<Self> {
- let delimiter = if let Some(delimiter) = args.get_optional_kwarg("delimiter") {
  let delimiter = if let Some(delimiter) = args.get_kwarg("delimiter") {
  *pystr::borrow_value(&delimiter)
  .as_bytes()
  .iter()
@@ -41,7 41,7 @@ impl ReaderOption {
  b','
  };
 
- let quotechar = if let Some(quotechar) = args.get_optional_kwarg("quotechar") {
  let quotechar = if let Some(quotechar) = args.get_kwarg("quotechar") {
  *pystr::borrow_value(&quotechar)
  .as_bytes()
  .iter()