diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 59be53a8b..6049b1f80 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -873,8 +873,7 @@ class BuiltinTest(unittest.TestCase): m2 = map(map_char, "Is this the real life?") self.check_iter_pickle(m1, list(m2), proto) - # TODO: RUSTPYTHON - @unittest.expectedFailure + @unittest.skip("TODO: RUSTPYTHON") def test_max(self): self.assertEqual(max('123123'), '3') self.assertEqual(max(1, 2, 3), 3) diff --git a/vm/src/builtins.rs b/vm/src/builtins.rs index 664dfea0d..d5076e522 100644 --- a/vm/src/builtins.rs +++ b/vm/src/builtins.rs @@ -425,9 +425,26 @@ mod decl { } #[pyfunction] - fn max(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult { + fn max(vm: &VirtualMachine, mut args: PyFuncArgs) -> 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 + ))); + } let candidates = match args.args.len().cmp(&1) { - std::cmp::Ordering::Greater => args.args.clone(), + std::cmp::Ordering::Greater => { + if default.is_some() { + return Err(vm.new_type_error( + "Cannot specify a default for max() with multiple positional arguments" + .to_owned(), + )); + } + args.args.clone() + } std::cmp::Ordering::Equal => vm.extract_elements(&args.args[0])?, std::cmp::Ordering::Less => { // zero arguments means type error: @@ -436,27 +453,32 @@ mod decl { }; if candidates.is_empty() { - let default = args.get_optional_kwarg("default"); return default .ok_or_else(|| vm.new_value_error("max() arg is an empty sequence".to_owned())); } - let key_func = args.get_optional_kwarg("key"); - // Start with first assumption: let mut candidates_iter = candidates.into_iter(); let mut x = candidates_iter.next().unwrap(); // TODO: this key function looks pretty duplicate. Maybe we can create // a local function? let mut x_key = if let Some(ref f) = &key_func { - vm.invoke(f, vec![x.clone()])? + if vm.is_none(f) { + x.clone() + } else { + vm.invoke(f, vec![x.clone()])? + } } else { x.clone() }; for y in candidates_iter { let y_key = if let Some(ref f) = &key_func { - vm.invoke(f, vec![y.clone()])? + if vm.is_none(f) { + y.clone() + } else { + vm.invoke(f, vec![y.clone()])? + } } else { y.clone() };