From 56bffd795b9bc791da0f631f3c98cc05fb2ca735 Mon Sep 17 00:00:00 2001 From: Seo Jihyeok Date: Tue, 2 Jun 2020 17:28:31 +0900 Subject: [PATCH 1/4] Remove remaining usages of arg_check! Close #1714 --- vm/src/builtins.rs | 8 ++-- vm/src/macros.rs | 103 ------------------------------------------ vm/src/obj/objbool.rs | 34 +++++++++----- 3 files changed, 27 insertions(+), 118 deletions(-) diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs index c6838616a..7ea6d8fb9 100644 --- a/vm/src/builtins.rs +++ b/vm/src/builtins.rs @@ -716,11 +716,13 @@ fn builtin_setattr( // builtin_slice fn builtin_sorted(vm: &VirtualMachine, mut args: PyFuncArgs) -> PyResult { - arg_check!(vm, args, required = [(iterable, None)]); - let items = vm.extract_elements(iterable)?; + let iterable = args.take_positional(); + if iterable.is_none() { + return Err(vm.new_type_error("sorted expected 1 arguments, got 0".to_string())); + } + let items = vm.extract_elements(&iterable.unwrap())?; let lst = vm.ctx.new_list(items); - args.shift(); vm.call_method(&lst, "sort", args)?; Ok(lst) } diff --git a/vm/src/macros.rs b/vm/src/macros.rs index 1d63e2105..e49d46a95 100644 --- a/vm/src/macros.rs +++ b/vm/src/macros.rs @@ -1,106 +1,3 @@ -// count number of tokens given as arguments. -// see: https://danielkeep.github.io/tlborm/book/blk-counting.html -#[macro_export] -macro_rules! replace_expr { - ($_t:tt $sub:expr) => { - $sub - }; -} - -#[macro_export] -macro_rules! count_tts { - ($($tts:tt)*) => {0usize $(+ $crate::replace_expr!($tts 1usize))*}; -} - -#[macro_export] -macro_rules! type_check { - ($vm:ident, $args:ident, $arg_count:ident, $arg_name:ident, $arg_type:expr) => { - // None indicates that we have no type requirement (i.e. we accept any type) - if let Some(expected_type) = $arg_type { - let arg = &$args.args[$arg_count]; - - if !$crate::obj::objtype::isinstance(arg, &expected_type) { - use $crate::pyobject::TypeProtocol; - - let arg_typ = arg.class(); - let expected_type_name = $vm.to_pystr(&expected_type)?; - let actual_type = $vm.to_pystr(&arg_typ)?; - return Err($vm.new_type_error(format!( - "argument of type {} is required for parameter {} ({}) (got: {})", - expected_type_name, - $arg_count + 1, - stringify!($arg_name), - actual_type - ))); - } - } - }; -} - -#[macro_export] -macro_rules! arg_check { - ( $vm: ident, $args:ident ) => { - // Zero-arg case - if $args.args.len() != 0 { - return Err($vm.new_type_error(format!( - "Expected no arguments (got: {})", $args.args.len()))); - } - }; - ( $vm: ident, $args:ident, required=[$( ($arg_name:ident, $arg_type:expr) ),*] ) => { - $crate::arg_check!($vm, $args, required=[$( ($arg_name, $arg_type) ),*], optional=[]); - }; - ( $vm: ident, $args:ident, required=[$( ($arg_name:ident, $arg_type:expr) ),*], optional=[$( ($optional_arg_name:ident, $optional_arg_type:expr) ),*] ) => { - let mut arg_count = 0; - - // use macro magic to compile-time count number of required and optional arguments - let minimum_arg_count = $crate::count_tts!($($arg_name)*); - let maximum_arg_count = minimum_arg_count + $crate::count_tts!($($optional_arg_name)*); - - // verify that the number of given arguments is right - if $args.args.len() < minimum_arg_count || $args.args.len() > maximum_arg_count { - let expected_str = if minimum_arg_count == maximum_arg_count { - format!("{}", minimum_arg_count) - } else { - format!("{}-{}", minimum_arg_count, maximum_arg_count) - }; - return Err($vm.new_type_error(format!( - "Expected {} arguments (got: {})", - expected_str, - $args.args.len() - ))); - }; - - // for each required parameter: - // check if the type matches. If not, return with error - // assign the arg to a variable - $( - $crate::type_check!($vm, $args, arg_count, $arg_name, $arg_type); - let $arg_name = &$args.args[arg_count]; - #[allow(unused_assignments)] - { - arg_count += 1; - } - )* - - // for each optional parameter, if there are enough positional arguments: - // check if the type matches. If not, return with error - // assign the arg to a variable - $( - let $optional_arg_name = if arg_count < $args.args.len() { - $crate::type_check!($vm, $args, arg_count, $optional_arg_name, $optional_arg_type); - let ret = Some(&$args.args[arg_count]); - #[allow(unused_assignments)] - { - arg_count += 1; - } - ret - } else { - None - }; - )* - }; -} - #[macro_export] macro_rules! no_kwargs { ( $vm: ident, $args:ident ) => { diff --git a/vm/src/obj/objbool.rs b/vm/src/obj/objbool.rs index e67a39dc2..c26e8a3c3 100644 --- a/vm/src/obj/objbool.rs +++ b/vm/src/obj/objbool.rs @@ -150,18 +150,28 @@ impl PyBool { } #[pyslot] - fn tp_new(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult { - arg_check!( - vm, - args, - required = [(_zelf, Some(vm.ctx.type_type()))], - optional = [(val, None)] - ); - let value = match val { - Some(val) => boolval(vm, val.clone())?, - None => false, - }; - Ok(vm.new_bool(value)) + fn tp_new(vm: &VirtualMachine, mut args: PyFuncArgs) -> PyResult { + let zelf = &args.shift(); + if !objtype::isinstance(&zelf, &vm.ctx.type_type()) { + let zelf_typ = zelf.class(); + let actual_type = vm.to_pystr(&zelf_typ)?; + return Err(vm.new_type_error(format!( + "requires a 'type' object but received a '{}'", + actual_type + ))); + } + if args.args.len() > 1 { + Err(vm.new_type_error(format!( + "bool expected at most 1 arguments, got {}", + args.args.len() + ))) + } else { + let value = match args.take_positional() { + Some(val) => boolval(vm, val.clone())?, + None => false, + }; + Ok(vm.new_bool(value)) + } } } From 555f749c741a7adb95c433841751963672ea459a Mon Sep 17 00:00:00 2001 From: Seo Jihyeok Date: Tue, 30 Jun 2020 01:39:26 +0900 Subject: [PATCH 2/4] Use OptionalArg for the builtin function bool --- vm/src/obj/objbool.rs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/vm/src/obj/objbool.rs b/vm/src/obj/objbool.rs index c26e8a3c3..2ee32b5d2 100644 --- a/vm/src/obj/objbool.rs +++ b/vm/src/obj/objbool.rs @@ -1,7 +1,7 @@ use num_bigint::Sign; use num_traits::Zero; -use crate::function::PyFuncArgs; +use crate::function::{OptionalArg, PyFuncArgs}; use crate::pyobject::{ IdProtocol, IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyResult, TryFromObject, TypeProtocol, @@ -150,8 +150,7 @@ impl PyBool { } #[pyslot] - fn tp_new(vm: &VirtualMachine, mut args: PyFuncArgs) -> PyResult { - let zelf = &args.shift(); + fn tp_new(zelf: PyObjectRef, x: OptionalArg, vm: &VirtualMachine) -> PyResult { if !objtype::isinstance(&zelf, &vm.ctx.type_type()) { let zelf_typ = zelf.class(); let actual_type = vm.to_pystr(&zelf_typ)?; @@ -160,18 +159,11 @@ impl PyBool { actual_type ))); } - if args.args.len() > 1 { - Err(vm.new_type_error(format!( - "bool expected at most 1 arguments, got {}", - args.args.len() - ))) - } else { - let value = match args.take_positional() { - Some(val) => boolval(vm, val.clone())?, - None => false, - }; - Ok(vm.new_bool(value)) - } + let val = match x { + OptionalArg::Present(val) => boolval(vm, val.clone())?, + OptionalArg::Missing => false, + }; + Ok(vm.new_bool(val)) } } From cef2a41627a9340c460764e3fe022d163f1aa61d Mon Sep 17 00:00:00 2001 From: Seo Jihyeok Date: Tue, 30 Jun 2020 01:40:09 +0900 Subject: [PATCH 3/4] Remove @unittest.expectedFailure annotation --- Lib/test/test_bool.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_bool.py b/Lib/test/test_bool.py index 10709b0e8..4305ecf2f 100644 --- a/Lib/test/test_bool.py +++ b/Lib/test/test_bool.py @@ -171,7 +171,6 @@ class BoolTest(unittest.TestCase): self.assertIs(bool(), False) # TODO: RUSTPYTHON - @unittest.expectedFailure def test_keyword_args(self): with self.assertRaisesRegex(TypeError, 'keyword argument'): bool(x=10) From 646feebe25af60a22871583f44309ff203901f2b Mon Sep 17 00:00:00 2001 From: Noah <33094578+coolreader18@users.noreply.github.com> Date: Mon, 29 Jun 2020 12:27:07 -0500 Subject: [PATCH 4/4] Remove TODO --- Lib/test/test_bool.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_bool.py b/Lib/test/test_bool.py index 4305ecf2f..541046cd9 100644 --- a/Lib/test/test_bool.py +++ b/Lib/test/test_bool.py @@ -170,7 +170,6 @@ class BoolTest(unittest.TestCase): self.assertIs(bool(""), False) self.assertIs(bool(), False) - # TODO: RUSTPYTHON def test_keyword_args(self): with self.assertRaisesRegex(TypeError, 'keyword argument'): bool(x=10)