diff --git a/Lib/test/test_extcall.py b/Lib/test/test_extcall.py index 274ecc40024..483d5ad5f2b 100644 --- a/Lib/test/test_extcall.py +++ b/Lib/test/test_extcall.py @@ -50,15 +50,15 @@ >>> f(1, 2, 3, **{'a':4, 'b':5}) (1, 2, 3) {'a': 4, 'b': 5} - >>> f(1, 2, **{'a': -1, 'b': 5}, **{'a': 4, 'c': 6}) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> f(1, 2, **{'a': -1, 'b': 5}, **{'a': 4, 'c': 6}) Traceback (most recent call last): ... TypeError: test.test_extcall.f() got multiple values for keyword argument 'a' - >>> f(1, 2, **{'a': -1, 'b': 5}, a=4, c=6) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> f(1, 2, **{'a': -1, 'b': 5}, a=4, c=6) Traceback (most recent call last): ... TypeError: test.test_extcall.f() got multiple values for keyword argument 'a' - >>> f(1, 2, a=3, **{'a': 4}, **{'a': 5}) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> f(1, 2, a=3, **{'a': 4}, **{'a': 5}) Traceback (most recent call last): ... TypeError: test.test_extcall.f() got multiple values for keyword argument 'a' @@ -134,7 +134,7 @@ >>> class Nothing: pass ... - >>> g(*Nothing()) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> g(*Nothing()) Traceback (most recent call last): ... TypeError: test.test_extcall.g() argument after * must be an iterable, not Nothing @@ -143,7 +143,7 @@ ... def __len__(self): return 5 ... - >>> g(*Nothing()) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> g(*Nothing()) Traceback (most recent call last): ... TypeError: test.test_extcall.g() argument after * must be an iterable, not Nothing @@ -263,75 +263,75 @@ ... TypeError: h() got an unexpected keyword argument 'e' - >>> h(*h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> h(*h) Traceback (most recent call last): ... TypeError: test.test_extcall.h() argument after * must be an iterable, not function - >>> h(1, *h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> h(1, *h) Traceback (most recent call last): ... TypeError: Value after * must be an iterable, not function - >>> h(*[1], *h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> h(*[1], *h) Traceback (most recent call last): ... TypeError: Value after * must be an iterable, not function - >>> dir(*h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> dir(*h) Traceback (most recent call last): ... TypeError: dir() argument after * must be an iterable, not function >>> nothing = None - >>> nothing(*h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> nothing(*h) Traceback (most recent call last): ... TypeError: None argument after * must be an iterable, \ not function - >>> h(**h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> h(**h) Traceback (most recent call last): ... TypeError: test.test_extcall.h() argument after ** must be a mapping, not function - >>> h(**[]) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> h(**[]) Traceback (most recent call last): ... TypeError: test.test_extcall.h() argument after ** must be a mapping, not list - >>> h(a=1, **h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> h(a=1, **h) Traceback (most recent call last): ... TypeError: test.test_extcall.h() argument after ** must be a mapping, not function - >>> h(a=1, **[]) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> h(a=1, **[]) Traceback (most recent call last): ... TypeError: test.test_extcall.h() argument after ** must be a mapping, not list - >>> h(**{'a': 1}, **h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> h(**{'a': 1}, **h) Traceback (most recent call last): ... TypeError: test.test_extcall.h() argument after ** must be a mapping, not function - >>> h(**{'a': 1}, **[]) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> h(**{'a': 1}, **[]) Traceback (most recent call last): ... TypeError: test.test_extcall.h() argument after ** must be a mapping, not list - >>> dir(**h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> dir(**h) Traceback (most recent call last): ... TypeError: dir() argument after ** must be a mapping, not function - >>> nothing(**h) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> nothing(**h) Traceback (most recent call last): ... TypeError: None argument after ** must be a mapping, \ not function - >>> dir(b=1, **{'b': 1}) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> dir(b=1, **{'b': 1}) Traceback (most recent call last): ... TypeError: dir() got multiple values for keyword argument 'b' @@ -367,17 +367,17 @@ >>> g(**MultiDict([('x', 1), ('y', 2)])) 1 () {'y': 2} - >>> g(**MultiDict([('x', 1), ('x', 2)])) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> g(**MultiDict([('x', 1), ('x', 2)])) Traceback (most recent call last): ... TypeError: test.test_extcall.g() got multiple values for keyword argument 'x' - >>> g(a=3, **MultiDict([('x', 1), ('x', 2)])) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> g(a=3, **MultiDict([('x', 1), ('x', 2)])) Traceback (most recent call last): ... TypeError: test.test_extcall.g() got multiple values for keyword argument 'x' - >>> g(**MultiDict([('a', 3)]), **MultiDict([('x', 1), ('x', 2)])) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> g(**MultiDict([('a', 3)]), **MultiDict([('x', 1), ('x', 2)])) Traceback (most recent call last): ... TypeError: test.test_extcall.g() got multiple values for keyword argument 'x' @@ -398,7 +398,7 @@ >>> assert s1(**md) == {'a': 1, 'b': 2} >>> assert s2(*(1, 2), **md) == ((1, 2), {'a': 1, 'b': 2}) >>> assert s3(**MyDict({'n': 1, 'b': 2})) == (1, {'b': 2}) - >>> s3(**md) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> s3(**md) Traceback (most recent call last): ... TypeError: s3() missing 1 required keyword-only argument: 'n' @@ -442,7 +442,7 @@ ... False True - >>> id(1, **{'foo': 1}) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> id(1, **{'foo': 1}) # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE Traceback (most recent call last): ... TypeError: id() takes no keyword arguments @@ -484,17 +484,17 @@ ... TypeError: f() takes from 1 to 2 positional arguments but 3 were given >>> def f(*, kw): pass - >>> f(1, kw=3) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> f(1, kw=3) Traceback (most recent call last): ... TypeError: f() takes 0 positional arguments but 1 positional argument (and 1 keyword-only argument) were given >>> def f(*, kw, b): pass - >>> f(1, 2, 3, b=3, kw=3) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> f(1, 2, 3, b=3, kw=3) Traceback (most recent call last): ... TypeError: f() takes 0 positional arguments but 3 positional arguments (and 2 keyword-only arguments) were given >>> def f(a, b=2, *, kw): pass - >>> f(2, 3, 4, kw=4) # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> f(2, 3, 4, kw=4) Traceback (most recent call last): ... TypeError: f() takes from 1 to 2 positional arguments but 3 positional arguments (and 1 keyword-only argument) were given @@ -530,12 +530,12 @@ Same with keyword only args: >>> def f(*, w): pass - >>> f() # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> f() Traceback (most recent call last): ... TypeError: f() missing 1 required keyword-only argument: 'w' >>> def f(*, a, b, c, d, e): pass - >>> f() # TODO: RUSTPYTHON # doctest: +EXPECTED_FAILURE + >>> f() Traceback (most recent call last): ... TypeError: f() missing 5 required keyword-only arguments: 'a', 'b', 'c', 'd', and 'e' @@ -545,9 +545,15 @@ import doctest import unittest +EXPECTED_FAILURE = doctest.register_optionflag('EXPECTED_FAILURE') # TODO: RUSTPYTHON +class CustomOutputChecker(doctest.OutputChecker): # TODO: RUSTPYTHON + def check_output(self, want, got, optionflags): # TODO: RUSTPYTHON + if optionflags & EXPECTED_FAILURE: # TODO: RUSTPYTHON + return not super().check_output(want, got, optionflags) # TODO: RUSTPYTHON + return super().check_output(want, got, optionflags) # TODO: RUSTPYTHON + def load_tests(loader, tests, pattern): - from test.support.rustpython import DocTestChecker # TODO: RUSTPYTHON - tests.addTest(doctest.DocTestSuite(checker=DocTestChecker())) # XXX: RUSTPYTHON + tests.addTest(doctest.DocTestSuite(checker=CustomOutputChecker())) # TODO: RUSTPYTHON return tests diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index 4de421180ca..3d19a35a1cd 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -2247,7 +2247,6 @@ def test_varkw_only(self): self.assertEqualCallArgs(f, '**collections.UserDict(a=1, b=2)') self.assertEqualCallArgs(f, 'c=3, **collections.UserDict(a=1, b=2)') - @unittest.expectedFailure # TODO: RUSTPYTHON; ? ^^^^^^^^^^^^ ++ ++ ^^^^ def test_keyword_only(self): f = self.makeCallable('a=3, *, c, d=2') self.assertEqualCallArgs(f, 'c=3') diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 897dc3e367d..7f432a15199 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -89,7 +89,6 @@ def dumps(self, arg, proto=None, **kwargs): def test_bad_newobj_args(self): return super().test_bad_newobj_args() - @unittest.expectedFailure # TODO: RUSTPYTHON def test_bad_newobj_ex_args(self): return super().test_bad_newobj_ex_args() diff --git a/Lib/test/test_positional_only_arg.py b/Lib/test/test_positional_only_arg.py index 4b27f4f4d3f..6c20e3767e0 100644 --- a/Lib/test/test_positional_only_arg.py +++ b/Lib/test/test_positional_only_arg.py @@ -163,8 +163,6 @@ def f(a, b, /, c=3): with self.assertRaisesRegex(TypeError, r"f\(\) takes from 2 to 3 positional arguments but 4 were given"): f(1, 2, 3, 4) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_positional_only_and_kwonlyargs_invalid_calls(self): def f(a, b, /, c, *, d, e): pass diff --git a/Lib/test/test_unpack_ex.py b/Lib/test/test_unpack_ex.py index 91ff1121741..d84befd9c7b 100644 --- a/Lib/test/test_unpack_ex.py +++ b/Lib/test/test_unpack_ex.py @@ -113,7 +113,7 @@ >>> sorted({**{'x': 1}, 'y': 2, **{'x': 3}}.items()) [('x', 3), ('y', 2)] - >>> sorted({**{'x': 1}, **{'x': 3}, 'x': 4}.items()) # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE + >>> sorted({**{'x': 1}, **{'x': 3}, 'x': 4}.items()) [('x', 4)] >>> {**{}} @@ -138,7 +138,7 @@ ... for i in range(1000)) + "}")) 1000 - >>> {0:1, **{0:2}, 0:3, 0:4} # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE + >>> {0:1, **{0:2}, 0:3, 0:4} {0: 4} List comprehension element unpacking @@ -249,34 +249,34 @@ Overridden parameters - >>> f(x=5, **{'x': 3}, y=2) # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE + >>> f(x=5, **{'x': 3}, y=2) Traceback (most recent call last): ... TypeError: test.test_unpack_ex.f() got multiple values for keyword argument 'x' - >>> f(**{'x': 3}, x=5, y=2) # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE + >>> f(**{'x': 3}, x=5, y=2) Traceback (most recent call last): ... TypeError: test.test_unpack_ex.f() got multiple values for keyword argument 'x' - >>> f(**{'x': 3}, **{'x': 5}, y=2) # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE + >>> f(**{'x': 3}, **{'x': 5}, y=2) Traceback (most recent call last): ... TypeError: test.test_unpack_ex.f() got multiple values for keyword argument 'x' - >>> f(x=5, **{'x': 3}, **{'x': 2}) # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE + >>> f(x=5, **{'x': 3}, **{'x': 2}) Traceback (most recent call last): ... TypeError: test.test_unpack_ex.f() got multiple values for keyword argument 'x' - >>> f(**{1: 3}, **{1: 5}) # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE + >>> f(**{1: 3}, **{1: 5}) Traceback (most recent call last): ... TypeError: test.test_unpack_ex.f() got multiple values for keyword argument '1' Unpacking non-sequence - >>> a, *b = 7 # TODO: RUSTPYTHON # doctest:+EXPECTED_FAILURE + >>> a, *b = 7 Traceback (most recent call last): ... TypeError: cannot unpack non-iterable int object @@ -321,17 +321,17 @@ Now some general starred expressions (all fail). - >>> a, *b, c, *d, e = range(10) # TODO: RUSTPYTHON # doctest:+ELLIPSIS +EXPECTED_FAILURE + >>> a, *b, c, *d, e = range(10) # doctest:+ELLIPSIS Traceback (most recent call last): ... SyntaxError: multiple starred expressions in assignment - >>> [*b, *c] = range(10) # TODO: RUSTPYTHON # doctest:+ELLIPSIS +EXPECTED_FAILURE + >>> [*b, *c] = range(10) # doctest:+ELLIPSIS Traceback (most recent call last): ... SyntaxError: multiple starred expressions in assignment - >>> a,*b,*c,*d = range(4) # TODO: RUSTPYTHON # doctest:+ELLIPSIS +EXPECTED_FAILURE + >>> a,*b,*c,*d = range(4) # doctest:+ELLIPSIS Traceback (most recent call last): ... SyntaxError: multiple starred expressions in assignment @@ -341,17 +341,17 @@ ... SyntaxError: starred assignment target must be in a list or tuple - >>> *a # TODO: RUSTPYTHON # doctest:+ELLIPSIS +EXPECTED_FAILURE + >>> *a # doctest:+ELLIPSIS Traceback (most recent call last): ... SyntaxError: can't use starred expression here - >>> *1 # TODO: RUSTPYTHON # doctest:+ELLIPSIS +EXPECTED_FAILURE + >>> *1 # doctest:+ELLIPSIS Traceback (most recent call last): ... SyntaxError: can't use starred expression here - >>> x = *a # TODO: RUSTPYTHON # doctest:+ELLIPSIS +EXPECTED_FAILURE + >>> x = *a # doctest:+ELLIPSIS Traceback (most recent call last): ... SyntaxError: can't use starred expression here diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index a2f32448ff2..28d47cb0424 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -6647,20 +6647,67 @@ impl Compiler { } fn compile_dict(&mut self, items: &[ast::DictItem]) -> CompileResult<()> { - // FIXME: correct order to build map, etc d = {**a, 'key': 2} should override - // 'key' in dict a - let mut size = 0; - let (packed, unpacked): (Vec<_>, Vec<_>) = items.iter().partition(|x| x.key.is_some()); - for item in packed { - self.compile_expression(item.key.as_ref().unwrap())?; - self.compile_expression(&item.value)?; - size += 1; + let has_unpacking = items.iter().any(|item| item.key.is_none()); + + if !has_unpacking { + // Simple case: no ** unpacking, build all pairs directly + for item in items { + self.compile_expression(item.key.as_ref().unwrap())?; + self.compile_expression(&item.value)?; + } + emit!( + self, + Instruction::BuildMap { + size: u32::try_from(items.len()).expect("too many dict items"), + } + ); + return Ok(()); + } + + // Complex case with ** unpacking: preserve insertion order. + // Collect runs of regular k:v pairs and emit BUILD_MAP + DICT_UPDATE + // for each run, and DICT_UPDATE for each ** entry. + let mut have_dict = false; + let mut elements: u32 = 0; + + // Flush pending regular pairs as a BUILD_MAP, merging into the + // accumulator dict via DICT_UPDATE when one already exists. + macro_rules! flush_pending { + () => { + #[allow(unused_assignments)] + if elements > 0 { + emit!(self, Instruction::BuildMap { size: elements }); + if have_dict { + emit!(self, Instruction::DictUpdate { index: 1 }); + } else { + have_dict = true; + } + elements = 0; + } + }; + } + + for item in items { + if let Some(key) = &item.key { + // Regular key: value pair + self.compile_expression(key)?; + self.compile_expression(&item.value)?; + elements += 1; + } else { + // ** unpacking entry + flush_pending!(); + if !have_dict { + emit!(self, Instruction::BuildMap { size: 0 }); + have_dict = true; + } + self.compile_expression(&item.value)?; + emit!(self, Instruction::DictUpdate { index: 1 }); + } } - emit!(self, Instruction::BuildMap { size }); - for item in unpacked { - self.compile_expression(&item.value)?; - emit!(self, Instruction::DictUpdate { index: 1 }); + flush_pending!(); + if !have_dict { + emit!(self, Instruction::BuildMap { size: 0 }); } Ok(()) @@ -7372,26 +7419,10 @@ impl Compiler { && nelts == 1 && matches!(arguments.args[0], ast::Expr::Starred(_)) { - // Special case: single starred arg - // Even in this case, we need to ensure it's a tuple for CallFunctionEx + // Single starred arg: pass value directly to CallFunctionEx. + // Runtime will convert to tuple and validate with function name. if let ast::Expr::Starred(ast::ExprStarred { value, .. }) = &arguments.args[0] { - // Check if the value is already a tuple expression - if matches!(value.as_ref(), ast::Expr::Tuple(_)) { - // Tuple literals can be used directly - self.compile_expression(value)?; - } else { - // For all other cases (including variables that might be lists), - // build a list and convert to tuple to ensure correct type - emit!(self, Instruction::BuildList { size: 0 }); - self.compile_expression(value)?; - emit!(self, Instruction::ListExtend { i: 0 }); - emit!( - self, - Instruction::CallIntrinsic1 { - func: IntrinsicFunction1::ListToTuple - } - ); - } + self.compile_expression(value)?; } } else { // Use starunpack_helper to build a list, then convert to tuple diff --git a/crates/codegen/src/error.rs b/crates/codegen/src/error.rs index 9f1dcc27058..086f9dfd739 100644 --- a/crates/codegen/src/error.rs +++ b/crates/codegen/src/error.rs @@ -107,9 +107,9 @@ impl fmt::Display for CodegenErrorType { Delete(target) => write!(f, "cannot delete {target}"), SyntaxError(err) => write!(f, "{}", err.as_str()), MultipleStarArgs => { - write!(f, "two starred expressions in assignment") + write!(f, "multiple starred expressions in assignment") } - InvalidStarExpr => write!(f, "cannot use starred expression here"), + InvalidStarExpr => write!(f, "can't use starred expression here"), InvalidBreak => write!(f, "'break' outside loop"), InvalidContinue => write!(f, "'continue' outside loop"), InvalidReturn => write!(f, "'return' outside function"), diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index 5afde116a02..9a6a6d49e3c 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -25,6 +25,37 @@ use itertools::Itertools; #[cfg(feature = "jit")] use rustpython_jit::CompiledCode; +fn format_missing_args( + qualname: impl core::fmt::Display, + kind: &str, + missing: &mut Vec, +) -> String { + let count = missing.len(); + let last = if missing.len() > 1 { + missing.pop() + } else { + None + }; + let (and, right): (&str, String) = if let Some(last) = last { + ( + if missing.len() == 1 { + "' and '" + } else { + "', and '" + }, + format!("{last}"), + ) + } else { + ("", String::new()) + }; + format!( + "{qualname}() missing {count} required {kind} argument{}: '{}{}{right}'", + if count == 1 { "" } else { "s" }, + missing.iter().join("', '"), + and, + ) +} + #[pyclass(module = false, name = "function", traverse = "manual")] #[derive(Debug)] pub struct PyFunction { @@ -227,13 +258,37 @@ impl PyFunction { } else { n_expected_args.to_string() }; + + // Count keyword-only arguments that were actually provided + let kw_only_given = if code.kwonlyarg_count > 0 { + let start = code.arg_count as usize; + let end = start + code.kwonlyarg_count as usize; + code.varnames[start..end] + .iter() + .filter(|name| func_args.kwargs.contains_key(name.as_str())) + .count() + } else { + 0 + }; + + let given_msg = if kw_only_given > 0 { + format!( + "{} positional argument{} (and {} keyword-only argument{}) were", + nargs, + if nargs == 1 { "" } else { "s" }, + kw_only_given, + if kw_only_given == 1 { "" } else { "s" }, + ) + } else { + format!("{} {}", nargs, if nargs == 1 { "was" } else { "were" }) + }; + return Err(vm.new_type_error(format!( - "{}() takes {} positional argument{} but {} {} given", + "{}() takes {} positional argument{} but {} given", self.__qualname__(), takes_msg, if n_expected_args == 1 { "" } else { "s" }, - nargs, - if nargs == 1 { "was" } else { "were" } + given_msg, ))); } } @@ -319,36 +374,12 @@ impl PyFunction { } }) .collect(); - let missing_args_len = missing.len(); if !missing.is_empty() { - let last = if missing.len() > 1 { - missing.pop() - } else { - None - }; - - let (and, right) = if let Some(last) = last { - ( - if missing.len() == 1 { - "' and '" - } else { - "', and '" - }, - last.as_str(), - ) - } else { - ("", "") - }; - - return Err(vm.new_type_error(format!( - "{}() missing {} required positional argument{}: '{}{}{}'", + return Err(vm.new_type_error(format_missing_args( self.__qualname__(), - missing_args_len, - if missing_args_len == 1 { "" } else { "s" }, - missing.iter().join("', '"), - and, - right, + "positional", + &mut missing, ))); } @@ -368,8 +399,7 @@ impl PyFunction { }; if code.kwonlyarg_count > 0 { - // TODO: compile a list of missing arguments - // let mut missing = vec![]; + let mut missing = Vec::new(); // Check if kw only arguments are all present: for (slot, kwarg) in fastlocals .iter_mut() @@ -386,9 +416,15 @@ impl PyFunction { } // No default value and not specified. - return Err( - vm.new_type_error(format!("Missing required kw only argument: '{kwarg}'")) - ); + missing.push(kwarg); + } + + if !missing.is_empty() { + return Err(vm.new_type_error(format_missing_args( + self.__qualname__(), + "keyword-only", + &mut missing, + ))); } } diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index f1647eb6fc1..23137a3f280 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -1070,28 +1070,32 @@ impl ExecutingFrame<'_> { let dict: &Py = unsafe { dict_ref.downcast_unchecked_ref() }; + // Get callable for error messages + // Stack: [callable, self_or_null, args_tuple, kwargs_dict] + let callable = self.nth_value(idx + 2); + let func_str = Self::object_function_str(callable, vm); + // Check if source is a mapping if vm .get_method(source.clone(), vm.ctx.intern_str("keys")) .is_none() { return Err(vm.new_type_error(format!( - "'{}' object is not a mapping", + "{} argument after ** must be a mapping, not {}", + func_str, source.class().name() ))); } - // Check for duplicate keys + // Merge keys, checking for duplicates let keys_iter = vm.call_method(&source, "keys", ())?; for key in keys_iter.try_to_value::>(vm)? { - if key.downcast_ref::().is_none() { - return Err(vm.new_type_error("keywords must be strings".to_owned())); - } if dict.contains_key(&*key, vm) { - let key_repr = key.repr(vm)?; + let key_str = key.str(vm)?; return Err(vm.new_type_error(format!( - "got multiple values for keyword argument {}", - key_repr.as_str() + "{} got multiple values for keyword argument '{}'", + func_str, + key_str.as_str() ))); } let value = vm.call_method(&source, "__getitem__", (key.clone(),))?; @@ -1323,7 +1327,23 @@ impl ExecutingFrame<'_> { // SAFETY: compiler guarantees correct type obj.downcast_unchecked_ref() }; - list.extend(iterable, vm)?; + let type_name = iterable.class().name().to_owned(); + // Only rewrite the error if the type is truly not iterable + // (no __iter__ and no __getitem__). Preserve original TypeError + // from custom iterables that raise during iteration. + let not_iterable = iterable.class().slots.iter.load().is_none() + && iterable + .get_class_attr(vm.ctx.intern_str("__getitem__")) + .is_none(); + list.extend(iterable, vm).map_err(|e| { + if not_iterable && e.class().is(vm.ctx.exceptions.type_error) { + vm.new_type_error(format!( + "Value after * must be an iterable, not {type_name}" + )) + } else { + e + } + })?; Ok(None) } Instruction::LoadAttr { idx } => self.load_attr(vm, idx.get(arg)), @@ -2587,8 +2607,11 @@ impl ExecutingFrame<'_> { let kwargs = if let Some(kw_obj) = kwargs_or_null { let mut kwargs = IndexMap::new(); - // Use keys() method for all mapping objects to preserve order - Self::iterate_mapping_keys(vm, &kw_obj, "argument after **", |key| { + // Stack: [callable, self_or_null, args_tuple] + let callable = self.nth_value(2); + let func_str = Self::object_function_str(callable, vm); + + Self::iterate_mapping_keys(vm, &kw_obj, &func_str, |key| { let key_str = key .downcast_ref::() .ok_or_else(|| vm.new_type_error("keywords must be strings"))?; @@ -2600,26 +2623,75 @@ impl ExecutingFrame<'_> { } else { IndexMap::new() }; - // SAFETY: trust compiler - let args = unsafe { self.pop_value().downcast_unchecked::() } - .as_slice() - .to_vec(); + let args_obj = self.pop_value(); + let args = if let Some(tuple) = args_obj.downcast_ref::() { + tuple.as_slice().to_vec() + } else { + // Single *arg passed directly; convert to sequence at runtime. + // Stack: [callable, self_or_null] + let callable = self.nth_value(1); + let func_str = Self::object_function_str(callable, vm); + let not_iterable = args_obj.class().slots.iter.load().is_none() + && args_obj + .get_class_attr(vm.ctx.intern_str("__getitem__")) + .is_none(); + args_obj.try_to_value::>(vm).map_err(|e| { + if not_iterable && e.class().is(vm.ctx.exceptions.type_error) { + vm.new_type_error(format!( + "{} argument after * must be an iterable, not {}", + func_str, + args_obj.class().name() + )) + } else { + e + } + })? + }; Ok(FuncArgs { args, kwargs }) } + /// Returns a display string for a callable object for use in error messages. + /// For objects with `__qualname__`, returns "module.qualname()" or "qualname()". + /// For other objects, returns repr(obj). + fn object_function_str(obj: &PyObject, vm: &VirtualMachine) -> String { + let Ok(qualname) = obj.get_attr(vm.ctx.intern_str("__qualname__"), vm) else { + return obj + .repr(vm) + .map(|s| s.as_str().to_owned()) + .unwrap_or_else(|_| "?".to_owned()); + }; + let Some(qualname_str) = qualname.downcast_ref::() else { + return obj + .repr(vm) + .map(|s| s.as_str().to_owned()) + .unwrap_or_else(|_| "?".to_owned()); + }; + if let Ok(module) = obj.get_attr(vm.ctx.intern_str("__module__"), vm) + && let Some(module_str) = module.downcast_ref::() + && module_str.as_str() != "builtins" + { + return format!("{}.{}()", module_str.as_str(), qualname_str.as_str()); + } + format!("{}()", qualname_str.as_str()) + } + /// Helper function to iterate over mapping keys using the keys() method. /// This ensures proper order preservation for OrderedDict and other custom mappings. fn iterate_mapping_keys( vm: &VirtualMachine, mapping: &PyObject, - error_prefix: &str, + func_str: &str, mut key_handler: F, ) -> PyResult<()> where F: FnMut(PyObjectRef) -> PyResult<()>, { let Some(keys_method) = vm.get_method(mapping.to_owned(), vm.ctx.intern_str("keys")) else { - return Err(vm.new_type_error(format!("{error_prefix} must be a mapping"))); + return Err(vm.new_type_error(format!( + "{} argument after ** must be a mapping, not {}", + func_str, + mapping.class().name() + ))); }; let keys = keys_method?.call((), vm)?.get_iter(vm)?; @@ -2731,7 +2803,20 @@ impl ExecutingFrame<'_> { fn execute_unpack_ex(&mut self, vm: &VirtualMachine, before: u8, after: u8) -> FrameResult { let (before, after) = (before as usize, after as usize); let value = self.pop_value(); - let elements: Vec<_> = value.try_to_value(vm)?; + let not_iterable = value.class().slots.iter.load().is_none() + && value + .get_class_attr(vm.ctx.intern_str("__getitem__")) + .is_none(); + let elements: Vec<_> = value.try_to_value(vm).map_err(|e| { + if not_iterable && e.class().is(vm.ctx.exceptions.type_error) { + vm.new_type_error(format!( + "cannot unpack non-iterable {} object", + value.class().name() + )) + } else { + e + } + })?; let min_expected = before + after; let middle = elements.len().checked_sub(min_expected).ok_or_else(|| { @@ -2932,8 +3017,12 @@ impl ExecutingFrame<'_> { fn unpack_sequence(&mut self, size: u32, vm: &VirtualMachine) -> FrameResult { let value = self.pop_value(); + let not_iterable = value.class().slots.iter.load().is_none() + && value + .get_class_attr(vm.ctx.intern_str("__getitem__")) + .is_none(); let elements: Vec<_> = value.try_to_value(vm).map_err(|e| { - if e.class().is(vm.ctx.exceptions.type_error) { + if not_iterable && e.class().is(vm.ctx.exceptions.type_error) { vm.new_type_error(format!( "cannot unpack non-iterable {} object", value.class().name()