From 5ce3f6cad94570aae2202161ecab550a94373bd0 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 1 Sep 2026 00:05:43 +0900 Subject: [PATCH 1/2] Name the function a failed argument binding was binding for A built-in call that binds badly reported neither the function nor, for a method, the right numbers: `self` was prepended before binding, so it was counted as both an expected parameter and a given argument. `[].append()` read `expected at least 2 arguments, got 1`. `Callee` carries what a message says about the function: the name, and whether the leading argument fills the instance parameter. `PyNativeFn` takes one, `PyNativeFunction` and `PyMethodDescriptor` supply it, and the four method-descriptor call specializations in `frame.rs` do the same. `bind_for` subtracts the instance from both numbers. The messages now read as their counterparts do: - `_PyArg_CheckPositional`: `find expected at least 1 argument, got 0`, with the singular when the count is one. - `_PyArg_UnpackKeywords`: `split() got an unexpected keyword argument 'bogus'`, and `cast() missing required argument 'format' (pos 1)` for a parameter a call may pass by name. A positional-only parameter is only ever counted, so it keeps the first form. A binding that happens where the name isn't known leaves it off, the way `_PyArg_Parser.fname` is NULL. `test_find_etc_raise_correct_error_messages` now passes in both `test_bytes` and `string_tests`. Assisted-by: Claude --- Lib/test/string_tests.py | 1 - Lib/test/test_bytes.py | 1 - crates/derive-impl/src/from_args.rs | 16 ++- crates/vm/src/builtins/builtin_func.rs | 9 +- crates/vm/src/builtins/descriptor.rs | 14 +- crates/vm/src/builtins/object.rs | 6 +- crates/vm/src/frame.rs | 14 +- crates/vm/src/function/argument.rs | 192 +++++++++++++++++++++---- crates/vm/src/function/builtin.rs | 40 +++--- crates/vm/src/function/method.rs | 2 +- crates/vm/src/function/mod.rs | 2 +- 11 files changed, 226 insertions(+), 71 deletions(-) diff --git a/Lib/test/string_tests.py b/Lib/test/string_tests.py index 66e6d78940b..20e30c9b6e0 100644 --- a/Lib/test/string_tests.py +++ b/Lib/test/string_tests.py @@ -1555,7 +1555,6 @@ def test_none_arguments(self): self.checkequal(True, s, 'startswith', 'h', None, -2) self.checkequal(False, s, 'startswith', 'x', None, None) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_find_etc_raise_correct_error_messages(self): # issue 11828 s = 'hello' diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index 16099ceb665..57dec4e04a8 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -1021,7 +1021,6 @@ def test_integer_arguments_out_of_byte_range(self): self.assertRaises(ValueError, method, 256) self.assertRaises(ValueError, method, 9999) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_find_etc_raise_correct_error_messages(self): # issue 11828 b = self.type2test(b'hello') diff --git a/crates/derive-impl/src/from_args.rs b/crates/derive-impl/src/from_args.rs index adcbbd418d2..535c5c4e35d 100644 --- a/crates/derive-impl/src/from_args.rs +++ b/crates/derive-impl/src/from_args.rs @@ -174,13 +174,21 @@ fn generate_field((i, field): (usize, &Field)) -> Result { .unwrap_or_else(|| #default) } } else { + // A parameter a call may name is named back when it is missing; one + // that can only be passed by position is only ever counted. let err = match attr.kind { - ParameterKind::PositionalOnly | ParameterKind::PositionalOrKeyword => quote! { + ParameterKind::PositionalOnly => quote! { ::rustpython_vm::function::ArgumentError::TooFewArgs }, - ParameterKind::KeywordOnly => quote! { - ::rustpython_vm::function::ArgumentError::RequiredKeywordArgument(#pyname.to_owned()) - }, + ParameterKind::PositionalOrKeyword | ParameterKind::KeywordOnly => { + let pos = i + 1; + quote! { + ::rustpython_vm::function::ArgumentError::MissingRequiredArgument { + name: #pyname.to_owned(), + pos: #pos, + } + } + } ParameterKind::Flatten => unreachable!(), }; quote! { diff --git a/crates/vm/src/builtins/builtin_func.rs b/crates/vm/src/builtins/builtin_func.rs index 873bf065f01..3d9d83e0c27 100644 --- a/crates/vm/src/builtins/builtin_func.rs +++ b/crates/vm/src/builtins/builtin_func.rs @@ -4,7 +4,7 @@ use crate::{ class::PyClassImpl, common::wtf8::Wtf8, convert::TryFromObject, - function::{FuncArgs, PyComparisonValue, PyMethodDef, PyMethodFlags, PyNativeFn}, + function::{Callee, FuncArgs, PyComparisonValue, PyMethodDef, PyMethodFlags, PyNativeFn}, types::{Callable, Comparable, PyComparisonOp, Representable}, }; use alloc::fmt; @@ -71,14 +71,16 @@ impl Callable for PyNativeFunction { type Args = FuncArgs; #[inline] fn call(zelf: &Py, mut args: FuncArgs, vm: &VirtualMachine) -> PyResult { + let mut callee = Callee::named(zelf.value.name); if let Some(z) = &zelf.zelf { // STATIC methods store the class in zelf for qualname/repr purposes, // but should not prepend it to args (the Rust function doesn't expect it). if !zelf.value.flags.contains(PyMethodFlags::STATIC) { args.prepend_arg(z.clone()); + callee = callee.with_instance_arg(true); } } - (zelf.value.func)(vm, args) + (zelf.value.func)(vm, args, callee) } } @@ -254,7 +256,8 @@ fn vectorcall_native_function( FuncArgs::from_vectorcall_owned(args, nargs, kwnames) }; - (zelf.value.func)(vm, func_args) + let callee = Callee::named(zelf.value.name).with_instance_arg(needs_self); + (zelf.value.func)(vm, func_args, callee) } pub(crate) fn init(context: &'static Context) { diff --git a/crates/vm/src/builtins/descriptor.rs b/crates/vm/src/builtins/descriptor.rs index 6fddb212872..96bd67cce0b 100644 --- a/crates/vm/src/builtins/descriptor.rs +++ b/crates/vm/src/builtins/descriptor.rs @@ -5,7 +5,7 @@ use crate::{ class::PyClassImpl, common::hash::PyHash, convert::{ToPyObject, ToPyResult}, - function::{ArgSize, FuncArgs, PyMethodDef, PyMethodFlags, PySetterValue}, + function::{ArgSize, Callee, FuncArgs, PyMethodDef, PyMethodFlags, PySetterValue}, protocol::{PyNumberBinaryFunc, PyNumberTernaryFunc, PyNumberUnaryFunc}, types::{ Callable, Comparable, DelFunc, DescrGetFunc, DescrSetFunc, GenericMethod, GetDescriptor, @@ -108,7 +108,11 @@ impl Callable for PyMethodDescriptor { type Args = FuncArgs; #[inline] fn call(zelf: &Py, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - (zelf.method.func)(vm, args) + (zelf.method.func)( + vm, + args, + Callee::named(zelf.method.name).with_instance_arg(true), + ) } } @@ -443,7 +447,11 @@ fn vectorcall_method_descriptor( ) -> PyResult { let zelf: &Py = zelf_obj.downcast_ref().unwrap(); let func_args = FuncArgs::from_vectorcall_owned(args, nargs, kwnames); - (zelf.method.func)(vm, func_args) + (zelf.method.func)( + vm, + func_args, + Callee::named(zelf.method.name).with_instance_arg(true), + ) } /// Vectorcall for wrapper_descriptor: calls wrapped slot function diff --git a/crates/vm/src/builtins/object.rs b/crates/vm/src/builtins/object.rs index 303e36339c9..5b52de70156 100644 --- a/crates/vm/src/builtins/object.rs +++ b/crates/vm/src/builtins/object.rs @@ -5,7 +5,7 @@ use crate::{ AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, class::PyClassImpl, convert::ToPyResult, - function::{Either, FuncArgs, PyArithmeticValue, PyComparisonValue, PySetterValue}, + function::{Callee, Either, FuncArgs, PyArithmeticValue, PyComparisonValue, PySetterValue}, types::{Constructor, Initializer, PyComparisonOp}, }; use itertools::Itertools; @@ -295,8 +295,8 @@ fn object_getstate_default(obj: &PyObject, required: bool, vm: &VirtualMachine) #[pyclass(with(Constructor, Initializer), flags(BASETYPE))] impl PyBaseObject { #[pymethod(raw)] - fn __getstate__(vm: &VirtualMachine, args: FuncArgs) -> PyResult { - let (zelf,): (PyObjectRef,) = args.bind(vm)?; + fn __getstate__(vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult { + let (zelf,): (PyObjectRef,) = args.bind_for(vm, callee)?; object_getstate_default(&zelf, false, vm) } diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 9715ded949c..eef8fd6da4c 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -23,7 +23,7 @@ use crate::{ convert::{ToPyObject, ToPyResult}, coroutine::Coro, exceptions::ExceptionCtor, - function::{ArgMapping, Either, FuncArgs, KwArgs, PyMethodFlags}, + function::{ArgMapping, Callee, Either, FuncArgs, KwArgs, PyMethodFlags}, object::PyAtomicBorrow, object::{Traverse, TraverseFn}, protocol::{PyIter, PyIterReturn}, @@ -6369,13 +6369,14 @@ impl ExecutingFrame<'_> { .is_some_and(|self_obj| self_obj.class().is(descr.objclass)) { let func = descr.method.func; + let callee = Callee::named(descr.method.name).with_instance_arg(true); let (_callable, all_args) = self.take_call_args(nargs as usize); debug_assert_eq!(all_args.len(), total_nargs as usize); let args = FuncArgs { args: all_args, kwargs: Default::default(), }; - let result = func(vm, args)?; + let result = func(vm, args, callee)?; self.push_value(result); return Ok(None); } @@ -6409,13 +6410,14 @@ impl ExecutingFrame<'_> { .is_some_and(|self_obj| self_obj.class().is(descr.objclass)) { let func = descr.method.func; + let callee = Callee::named(descr.method.name).with_instance_arg(true); let (_callable, all_args) = self.take_call_args(nargs as usize); debug_assert_eq!(all_args.len(), total_nargs as usize); let args = FuncArgs { args: all_args, kwargs: Default::default(), }; - let result = func(vm, args)?; + let result = func(vm, args, callee)?; self.push_value(result); return Ok(None); } @@ -6449,13 +6451,14 @@ impl ExecutingFrame<'_> { .is_some_and(|self_obj| self_obj.class().is(descr.objclass)) { let func = descr.method.func; + let callee = Callee::named(descr.method.name).with_instance_arg(true); let (_callable, all_args) = self.take_call_args(nargs as usize); debug_assert_eq!(all_args.len(), total_nargs as usize); let args = FuncArgs { args: all_args, kwargs: Default::default(), }; - let result = func(vm, args)?; + let result = func(vm, args, callee)?; self.push_value(result); return Ok(None); } @@ -6554,13 +6557,14 @@ impl ExecutingFrame<'_> { .is_some_and(|self_obj| self_obj.class().is(descr.objclass)) { let func = descr.method.func; + let callee = Callee::named(descr.method.name).with_instance_arg(true); let (_callable, all_args) = self.take_call_args(nargs as usize); debug_assert_eq!(all_args.len(), total_nargs as usize); let args = FuncArgs { args: all_args, kwargs: Default::default(), }; - let result = func(vm, args)?; + let result = func(vm, args, callee)?; self.push_value(result); return Ok(None); } diff --git a/crates/vm/src/function/argument.rs b/crates/vm/src/function/argument.rs index 6bf4ae2107b..24344beb3f5 100644 --- a/crates/vm/src/function/argument.rs +++ b/crates/vm/src/function/argument.rs @@ -284,18 +284,26 @@ impl FuncArgs { /// /// If the given `FromArgs` includes any conversions, exceptions raised /// during the conversion will halt the binding and return the error. - pub fn bind(mut self, vm: &VirtualMachine) -> PyResult { - let given_args = self.args.len(); + pub fn bind(self, vm: &VirtualMachine) -> PyResult { + self.bind_for(vm, Callee::default()) + } + + /// Binds these arguments the way [`bind`](Self::bind) does, for a call whose + /// function a failure can describe. + pub fn bind_for(mut self, vm: &VirtualMachine, callee: Callee) -> PyResult { + // A message describes the parameters the function declares, and the + // instance a method is called on is not one of them. + let instance = callee.instance_args(); + let arity = T::arity(); + let arity = arity.start().saturating_sub(instance)..=arity.end().saturating_sub(instance); + let num_given = self.args.len().saturating_sub(instance); + let bound = T::from_args(vm, &mut self) - .map_err(|e| e.into_exception(T::arity(), given_args, vm))?; + .map_err(|e| e.into_exception(&arity, num_given, callee, vm))?; if !self.args.is_empty() { - Err(vm.new_type_error(format!( - "expected at most {} arguments, got {}", - T::arity().end(), - given_args, - ))) - } else if let Some(err) = self.check_kwargs_empty(vm) { + Err(ArgumentError::TooManyArgs.into_exception(&arity, num_given, callee, vm)) + } else if let Some(err) = self.check_kwargs_empty_for(vm, callee) { Err(err) } else { Ok(bound) @@ -303,10 +311,146 @@ impl FuncArgs { } pub fn check_kwargs_empty(&self, vm: &VirtualMachine) -> Option { + self.check_kwargs_empty_for(vm, Callee::default()) + } + + /// The same as [`check_kwargs_empty`](Self::check_kwargs_empty), for a call + /// whose function the message can name. + pub fn check_kwargs_empty_for( + &self, + vm: &VirtualMachine, + callee: Callee, + ) -> Option { self.kwargs .keys() .next() - .map(|k| vm.new_type_error(format!("Unexpected keyword argument {k}"))) + .map(|k| callee.unexpected_keyword(&k.to_string(), vm)) + } +} + +/// What a message says about the function whose arguments are being bound. +/// +/// A binding that happens somewhere the name isn't known leaves it off, the way +/// `_PyArg_Parser.fname` is NULL. A message describes the parameters the +/// function declares, so a method's instance argument counts as neither an +/// expected parameter nor a given argument, the way `descrobject.c` reports +/// `nargs - 1`. +#[derive(Clone, Copy, Debug, Default)] +pub struct Callee { + name: Option<&'static str>, + instance_arg: bool, +} + +impl Callee { + /// A function a message can name. + #[must_use] + pub const fn named(name: &'static str) -> Self { + Self { + name: Some(name), + instance_arg: false, + } + } + + /// The type a slot builds or initializes, named the way a message names it. + #[must_use] + pub fn for_type(class: &crate::Py) -> Self { + Self::named(class.slots.name) + } + + /// The same, for the type a slot was written for. + #[must_use] + pub fn of(vm: &VirtualMachine) -> Self { + Self::for_type(T::class(&vm.ctx)) + } + + /// Marks a call whose leading argument fills the method's instance parameter. + #[must_use] + pub const fn with_instance_arg(mut self, instance_arg: bool) -> Self { + self.instance_arg = instance_arg; + self + } + + /// How many leading arguments answer for the instance rather than for a + /// parameter the method declares. + const fn instance_args(self) -> usize { + self.instance_arg as usize + } + + /// The name a message uses. `tp_name` carries the module along with the + /// type, and a message names only the type itself. + fn short_name(self) -> Option<&'static str> { + self.name + .map(|name| name.rsplit_once('.').map_or(name, |(_, name)| name)) + } + + /// The name a message opens with and the parentheses that follow it, given + /// what to call a function whose name isn't known. + fn call_form(self, unnamed: &'static str) -> (&'static str, &'static str) { + match self.short_name() { + Some(name) => (name, "()"), + None => (unnamed, ""), + } + } + + /// The error a call that passed the wrong number of arguments gets, for a + /// slot that counts them itself. + #[must_use] + pub fn arity_error( + self, + arity: RangeInclusive, + num_given: usize, + vm: &VirtualMachine, + ) -> PyBaseExceptionRef { + let too_few = num_given < *arity.start(); + self.wrong_arity(&arity, too_few, num_given, vm) + } + + /// _PyArg_CheckPositional + fn wrong_arity( + self, + arity: &RangeInclusive, + too_few: bool, + num_given: usize, + vm: &VirtualMachine, + ) -> PyBaseExceptionRef { + let (limit, bound) = if too_few { + (*arity.start(), "at least ") + } else { + (*arity.end(), "at most ") + }; + let bound = if arity.start() == arity.end() { + "" + } else { + bound + }; + let plural = if limit == 1 { "" } else { "s" }; + let name = self + .short_name() + .map_or_else(String::new, |name| format!("{name} ")); + vm.new_type_error(format!( + "{name}expected {bound}{limit} argument{plural}, got {num_given}" + )) + } + + /// The branch of _PyArg_UnpackKeywords that names a keyword it didn't expect. + fn unexpected_keyword(self, keyword: &str, vm: &VirtualMachine) -> PyBaseExceptionRef { + let (name, parens) = self.call_form("this function"); + vm.new_type_error(format!( + "{name}{parens} got an unexpected keyword argument '{keyword}'" + )) + } + + /// The branch of _PyArg_UnpackKeywords that names a parameter it didn't get. + fn missing_argument( + self, + keyword: &str, + pos: usize, + vm: &VirtualMachine, + ) -> PyBaseExceptionRef { + let (name, parens) = self.call_form("function"); + vm.new_type_error(format!( + "{name}{parens} missing required argument '{keyword}' (pos {pos})" + )) } } @@ -319,8 +463,9 @@ pub enum ArgumentError { TooManyArgs, /// The function doesn't accept a keyword argument with the given name. InvalidKeywordArgument(String), - /// The function require a keyword argument with the given name, but one wasn't provided - RequiredKeywordArgument(String), + /// The function requires an argument for the named parameter, at the given + /// 1-based position, but the call didn't pass one. + MissingRequiredArgument { name: String, pos: usize }, /// An exception was raised while binding arguments to the function /// parameters. Exception(PyBaseExceptionRef), @@ -335,27 +480,16 @@ impl From for ArgumentError { impl ArgumentError { fn into_exception( self, - arity: RangeInclusive, + arity: &RangeInclusive, num_given: usize, + callee: Callee, vm: &VirtualMachine, ) -> PyBaseExceptionRef { match self { - Self::TooFewArgs => vm.new_type_error(format!( - "expected at least {} arguments, got {}", - arity.start(), - num_given - )), - Self::TooManyArgs => vm.new_type_error(format!( - "expected at most {} arguments, got {}", - arity.end(), - num_given - )), - Self::InvalidKeywordArgument(name) => { - vm.new_type_error(format!("{name} is an invalid keyword argument")) - } - Self::RequiredKeywordArgument(name) => { - vm.new_type_error(format!("Required keyword only argument {name}")) - } + Self::TooFewArgs => callee.wrong_arity(arity, true, num_given, vm), + Self::TooManyArgs => callee.wrong_arity(arity, false, num_given, vm), + Self::InvalidKeywordArgument(name) => callee.unexpected_keyword(&name, vm), + Self::MissingRequiredArgument { name, pos } => callee.missing_argument(&name, pos, vm), Self::Exception(ex) => ex, } } diff --git a/crates/vm/src/function/builtin.rs b/crates/vm/src/function/builtin.rs index a2753bdf145..b4bd610776d 100644 --- a/crates/vm/src/function/builtin.rs +++ b/crates/vm/src/function/builtin.rs @@ -1,4 +1,4 @@ -use super::{FromArgs, FuncArgs}; +use super::{Callee, FromArgs, FuncArgs}; use crate::{ Py, PyPayload, PyRef, PyResult, VirtualMachine, convert::ToPyResult, object::PyThreadingConstraint, @@ -8,12 +8,12 @@ use core::marker::PhantomData; /// A built-in Python function. // PyCFunction in CPython pub trait PyNativeFn: - Fn(&VirtualMachine, FuncArgs) -> PyResult + PyThreadingConstraint + 'static + Fn(&VirtualMachine, FuncArgs, Callee) -> PyResult + PyThreadingConstraint + 'static { } impl PyNativeFn for F where - F: Fn(&VirtualMachine, FuncArgs) -> PyResult + PyThreadingConstraint + 'static + F: Fn(&VirtualMachine, FuncArgs, Callee) -> PyResult + PyThreadingConstraint + 'static { } @@ -37,7 +37,7 @@ impl PyNativeFn for F where /// just pass an unconstrained generic type, e.g. /// `fn foo(f: F) where F: IntoPyNativeFn` pub trait IntoPyNativeFn: Sized + PyThreadingConstraint + 'static { - fn call(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult; + fn call(&self, vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult; /// `IntoPyNativeFn::into_func()` generates a PyNativeFn that performs the /// appropriate type and arity checking, any requested conversions, and then if @@ -48,7 +48,7 @@ pub trait IntoPyNativeFn: Sized + PyThreadingConstraint + 'static { } const fn into_func, Kind>(f: F) -> impl PyNativeFn { - move |vm: &VirtualMachine, args| f.call(vm, args) + move |vm: &VirtualMachine, args, callee| f.call(vm, args, callee) } const fn zst_ref_out_of_thin_air(x: T) -> &'static T { @@ -90,15 +90,15 @@ where F: PyNativeFnInternal, { #[inline(always)] - fn call(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult { - self.call_(vm, args) + fn call(&self, vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult { + self.call_(vm, args, callee) } } mod sealed { use super::*; pub trait PyNativeFnInternal: Sized + PyThreadingConstraint + 'static { - fn call_(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult; + fn call_(&self, vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult; } } use sealed::PyNativeFnInternal; @@ -124,8 +124,8 @@ macro_rules! into_py_native_fn_tuple { $($T: FromArgs,)* R: ToPyResult, { - fn call_(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult { - let ($($n,)*) = args.bind::<($($T,)*)>(vm)?; + fn call_(&self, vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult { + let ($($n,)*) = args.bind_for::<($($T,)*)>(vm, callee)?; (self)($($n,)* vm).to_pyresult(vm) } @@ -138,8 +138,8 @@ macro_rules! into_py_native_fn_tuple { $($T: FromArgs,)* R: ToPyResult, { - fn call_(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult { - let (zelf, $($n,)*) = args.bind::<(PyRef, $($T,)*)>(vm)?; + fn call_(&self, vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult { + let (zelf, $($n,)*) = args.bind_for::<(PyRef, $($T,)*)>(vm, callee)?; (self)(&zelf, $($n,)* vm).to_pyresult(vm) } @@ -152,8 +152,8 @@ macro_rules! into_py_native_fn_tuple { $($T: FromArgs,)* R: ToPyResult, { - fn call_(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult { - let (zelf, $($n,)*) = args.bind::<(PyRef, $($T,)*)>(vm)?; + fn call_(&self, vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult { + let (zelf, $($n,)*) = args.bind_for::<(PyRef, $($T,)*)>(vm, callee)?; (self)(&zelf, $($n,)* vm).to_pyresult(vm) } @@ -165,8 +165,8 @@ macro_rules! into_py_native_fn_tuple { $($T: FromArgs,)* R: ToPyResult, { - fn call_(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult { - let ($($n,)*) = args.bind::<($($T,)*)>(vm)?; + fn call_(&self, vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult { + let ($($n,)*) = args.bind_for::<($($T,)*)>(vm, callee)?; (self)($($n,)*).to_pyresult(vm) } @@ -179,8 +179,8 @@ macro_rules! into_py_native_fn_tuple { $($T: FromArgs,)* R: ToPyResult, { - fn call_(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult { - let (zelf, $($n,)*) = args.bind::<(PyRef, $($T,)*)>(vm)?; + fn call_(&self, vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult { + let (zelf, $($n,)*) = args.bind_for::<(PyRef, $($T,)*)>(vm, callee)?; (self)(&zelf, $($n,)*).to_pyresult(vm) } @@ -193,8 +193,8 @@ macro_rules! into_py_native_fn_tuple { $($T: FromArgs,)* R: ToPyResult, { - fn call_(&self, vm: &VirtualMachine, args: FuncArgs) -> PyResult { - let (zelf, $($n,)*) = args.bind::<(PyRef, $($T,)*)>(vm)?; + fn call_(&self, vm: &VirtualMachine, args: FuncArgs, callee: Callee) -> PyResult { + let (zelf, $($n,)*) = args.bind_for::<(PyRef, $($T,)*)>(vm, callee)?; (self)(&zelf, $($n,)*).to_pyresult(vm) } diff --git a/crates/vm/src/function/method.rs b/crates/vm/src/function/method.rs index f0497fb384e..5c19aa112a6 100644 --- a/crates/vm/src/function/method.rs +++ b/crates/vm/src/function/method.rs @@ -234,7 +234,7 @@ impl PyMethodDef { ) -> [Self; SUM_LEN] { const NULL_METHOD: PyMethodDef = PyMethodDef { name: "", - func: &|_, _| unreachable!(), + func: &|_, _, _| unreachable!(), flags: PyMethodFlags::empty(), doc: None, }; diff --git a/crates/vm/src/function/mod.rs b/crates/vm/src/function/mod.rs index eb77c42e4c6..31774292a21 100644 --- a/crates/vm/src/function/mod.rs +++ b/crates/vm/src/function/mod.rs @@ -12,7 +12,7 @@ mod protocol; mod time; pub use argument::{ - ArgumentError, FromArgOptional, FromArgs, FuncArgs, IntoFuncArgs, KwArgs, KwArgsMap, + ArgumentError, Callee, FromArgOptional, FromArgs, FuncArgs, IntoFuncArgs, KwArgs, KwArgsMap, OptionalArg, OptionalOption, PosArgs, }; pub use arithmetic::{PyArithmeticValue, PyComparisonValue}; From 0d4e960195a39cb1000cb5713deeeb46f974015d Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Tue, 1 Sep 2026 00:05:55 +0900 Subject: [PATCH 2/2] Name the type whose constructor or initializer bound the arguments `Constructor::slot_new` and `Initializer::slot_init` bound their arguments without a name, and so did the slots that override them, so a constructor that was called wrongly said only `expected 1 argument, got 0`. Both defaults and every override now name the type the slot was written for, not the subclass being constructed and not the module `tp_name` carries: `float expected at most 1 argument, got 2`, and `deque expected at most 2 arguments, got 3` for a `deque` subclass. `slice()` and `range()` counted their own arguments and raised messages of their own; they now raise the one `_PyArg_CheckPositional` raises, which makes `test_range_constructor_error_messages` pass. Assisted-by: Claude --- Lib/test/test_range.py | 1 - crates/stdlib/src/contextvars.rs | 4 +- crates/vm/src/builtins/bool.rs | 4 +- crates/vm/src/builtins/bytes.rs | 6 +- crates/vm/src/builtins/classmethod.rs | 4 +- crates/vm/src/builtins/complex.rs | 4 +- crates/vm/src/builtins/float.rs | 5 +- crates/vm/src/builtins/int.rs | 6 +- crates/vm/src/builtins/range.rs | 10 ++- crates/vm/src/builtins/set.rs | 6 +- crates/vm/src/builtins/singletons.rs | 6 +- crates/vm/src/builtins/slice.rs | 10 +-- crates/vm/src/builtins/staticmethod.rs | 4 +- crates/vm/src/builtins/str.rs | 6 +- crates/vm/src/builtins/tuple.rs | 4 +- crates/vm/src/builtins/weakproxy.rs | 6 +- crates/vm/src/exception_group.rs | 2 +- crates/vm/src/stdlib/_ctypes/simple.rs | 2 +- crates/vm/src/stdlib/_typing.rs | 2 +- crates/vm/src/stdlib/itertools.rs | 9 +- crates/vm/src/stdlib/os.rs | 4 +- crates/vm/src/stdlib/posix.rs | 3 +- crates/vm/src/stdlib/time.rs | 4 +- crates/vm/src/types/slot.rs | 8 +- crates/vm/src/types/structseq.rs | 11 ++- extra_tests/snippets/vm_argument_errors.py | 97 ++++++++++++++++++++++ 26 files changed, 173 insertions(+), 55 deletions(-) create mode 100644 extra_tests/snippets/vm_argument_errors.py diff --git a/Lib/test/test_range.py b/Lib/test/test_range.py index f69054180be..c08234a5179 100644 --- a/Lib/test/test_range.py +++ b/Lib/test/test_range.py @@ -91,7 +91,6 @@ def test_range(self): r = range(-sys.maxsize, sys.maxsize, 2) self.assertEqual(len(r), sys.maxsize) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_range_constructor_error_messages(self): with self.assertRaisesRegex( TypeError, diff --git a/crates/stdlib/src/contextvars.rs b/crates/stdlib/src/contextvars.rs index e3823f6ac59..b14440e228c 100644 --- a/crates/stdlib/src/contextvars.rs +++ b/crates/stdlib/src/contextvars.rs @@ -20,7 +20,7 @@ mod _contextvars { lock::{LazyLock, PyMutex}, wtf8::Wtf8Buf, }, - function::{ArgCallable, FuncArgs, OptionalArg}, + function::{ArgCallable, Callee, FuncArgs, OptionalArg}, protocol::{PyMappingMethods, PySequenceMethods}, types::{AsMapping, AsSequence, Constructor, Hashable, Iterable, Representable}, }; @@ -484,7 +484,7 @@ mod _contextvars { type Args = ContextVarOptions; fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let args: Self::Args = args.bind(vm)?; + let args: Self::Args = args.bind_for(vm, Callee::of::(vm))?; let var = Self { name: args.name.to_string(), default: args.default.into_option(), diff --git a/crates/vm/src/builtins/bool.rs b/crates/vm/src/builtins/bool.rs index 26e52eff791..dab71a1e047 100644 --- a/crates/vm/src/builtins/bool.rs +++ b/crates/vm/src/builtins/bool.rs @@ -4,7 +4,7 @@ use crate::{ AsObject, Context, Py, PyObject, PyObjectRef, PyResult, TryFromBorrowedObject, VirtualMachine, class::PyClassImpl, convert::{IntoPyException, ToPyObject, ToPyResult}, - function::{FuncArgs, OptionalArg}, + function::{Callee, FuncArgs, OptionalArg}, protocol::PyNumberMethods, types::{AsNumber, Constructor, Representable}, }; @@ -87,7 +87,7 @@ impl Constructor for PyBool { type Args = OptionalArg; fn slot_new(zelf: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let x: Self::Args = args.bind(vm)?; + let x: Self::Args = args.bind_for(vm, Callee::of::(vm))?; if !zelf.fast_isinstance(vm.ctx.types.type_type) { return Err(vm.new_type_error(format!( "requires a 'type' object but received a '{}'", diff --git a/crates/vm/src/builtins/bytes.rs b/crates/vm/src/builtins/bytes.rs index 1dc6264976b..6c06e714367 100644 --- a/crates/vm/src/builtins/bytes.rs +++ b/crates/vm/src/builtins/bytes.rs @@ -17,7 +17,9 @@ use crate::{ class::PyClassImpl, common::{hash::PyHash, lock::PyMutex}, convert::{ToPyObject, ToPyResult}, - function::{ArgBytesLike, ArgIndex, FuncArgs, OptionalArg, OptionalOption, PyComparisonValue}, + function::{ + ArgBytesLike, ArgIndex, Callee, FuncArgs, OptionalArg, OptionalOption, PyComparisonValue, + }, protocol::{ BufferDescriptor, BufferFlags, BufferMethods, PyBuffer, PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods, @@ -95,7 +97,7 @@ impl Constructor for PyBytes { type Args = Vec; fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let options: ByteInnerNewOptions = args.bind(vm)?; + let options: ByteInnerNewOptions = args.bind_for(vm, Callee::of::(vm))?; // Optimizations for exact bytes type if cls.is(vm.ctx.types.bytes_type) { diff --git a/crates/vm/src/builtins/classmethod.rs b/crates/vm/src/builtins/classmethod.rs index 26dcd251251..1d7e78deb84 100644 --- a/crates/vm/src/builtins/classmethod.rs +++ b/crates/vm/src/builtins/classmethod.rs @@ -3,7 +3,7 @@ use crate::{ AsObject, Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, class::PyClassImpl, common::lock::PyMutex, - function::{FuncArgs, PySetterValue}, + function::{Callee, FuncArgs, PySetterValue}, types::{Constructor, GetDescriptor, Initializer, Representable}, }; @@ -70,7 +70,7 @@ impl Constructor for PyClassMethod { // copying its attributes to `__init__` so that subclasses overriding // `__init__` without calling `super().__init__()` see `__func__` as // `None`, matching CPython. - let _: Self::Args = args.bind(vm)?; + let _: Self::Args = args.bind_for(vm, Callee::of::(vm))?; let classmethod = Self { callable: PyMutex::new(vm.ctx.none()), }; diff --git a/crates/vm/src/builtins/complex.rs b/crates/vm/src/builtins/complex.rs index 25d8d45cc4e..4d0c8115081 100644 --- a/crates/vm/src/builtins/complex.rs +++ b/crates/vm/src/builtins/complex.rs @@ -5,7 +5,7 @@ use crate::{ class::PyClassImpl, common::{format::FormatSpec, wtf8::Wtf8Buf}, convert::{IntoPyException, ToPyObject, ToPyResult}, - function::{FuncArgs, OptionalArg, PyComparisonValue}, + function::{Callee, FuncArgs, OptionalArg, PyComparisonValue}, protocol::PyNumberMethods, stdlib::_warnings, types::{AsNumber, Callable, Comparable, Constructor, Hashable, PyComparisonOp, Representable}, @@ -395,7 +395,7 @@ impl Constructor for PyComplex { return Ok(func_args.args[0].clone()); } - let args: Self::Args = func_args.bind(vm)?; + let args: Self::Args = func_args.bind_for(vm, Callee::of::(vm))?; let payload = Self::py_new(&cls, args, vm)?; payload.into_ref_with_type(vm, cls).map(Into::into) } diff --git a/crates/vm/src/builtins/float.rs b/crates/vm/src/builtins/float.rs index 30c6e3c1f38..c986e23f04e 100644 --- a/crates/vm/src/builtins/float.rs +++ b/crates/vm/src/builtins/float.rs @@ -9,7 +9,8 @@ use crate::{ common::{float_ops, format::FormatSpec, hash, wtf8::Wtf8Buf}, convert::{IntoPyException, ToPyObject, ToPyResult}, function::{ - ArgBytesLike, FuncArgs, OptionalArg, OptionalOption, PyArithmeticValue, PyComparisonValue, + ArgBytesLike, Callee, FuncArgs, OptionalArg, OptionalOption, PyArithmeticValue, + PyComparisonValue, }, protocol::PyNumberMethods, types::{AsNumber, Callable, Comparable, Constructor, Hashable, PyComparisonOp, Representable}, @@ -178,7 +179,7 @@ impl Constructor for PyFloat { fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { // Bind before the fast path so FromArgs::arity decides how many arguments // are acceptable, rather than a count repeated here. - let arg: Self::Args = args.bind(vm)?; + let arg: Self::Args = args.bind_for(vm, Callee::of::(vm))?; // Optimization: return exact float as-is if cls.is(vm.ctx.types.float_type) diff --git a/crates/vm/src/builtins/int.rs b/crates/vm/src/builtins/int.rs index 3a4c18a3cdd..c01a774c5d4 100644 --- a/crates/vm/src/builtins/int.rs +++ b/crates/vm/src/builtins/int.rs @@ -13,8 +13,8 @@ use crate::{ }, convert::{IntoPyException, ToPyObject, ToPyResult}, function::{ - ArgByteOrder, ArgIntoBool, FuncArgs, OptionalArg, OptionalOption, PyArithmeticValue, - PyComparisonValue, + ArgByteOrder, ArgIntoBool, Callee, FuncArgs, OptionalArg, OptionalOption, + PyArithmeticValue, PyComparisonValue, }, protocol::{PyNumberMethods, handle_bytes_to_int_err, numeric_literal_from_str}, types::{AsNumber, Comparable, Constructor, Hashable, PyComparisonOp, Representable}, @@ -259,7 +259,7 @@ impl Constructor for PyInt { return Ok(args.args[0].clone()); } - let options: IntOptions = args.bind(vm)?; + let options: IntOptions = args.bind_for(vm, Callee::of::(vm))?; let value = if let OptionalArg::Present(val) = options.val_options { if let OptionalArg::Present(base) = options.base { let base = base diff --git a/crates/vm/src/builtins/range.rs b/crates/vm/src/builtins/range.rs index a315cef9d10..b361ec57750 100644 --- a/crates/vm/src/builtins/range.rs +++ b/crates/vm/src/builtins/range.rs @@ -8,7 +8,7 @@ use crate::{ VirtualMachine, atomic_func, class::PyClassImpl, common::hash::PyHash, - function::{ArgIndex, FuncArgs, OptionalArg, PyComparisonValue}, + function::{ArgIndex, Callee, FuncArgs, OptionalArg, PyComparisonValue}, protocol::{PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods}, types::{ AsMapping, AsNumber, AsSequence, Comparable, Hashable, IterNext, Iterable, PyComparisonOp, @@ -351,11 +351,13 @@ impl PyRange { #[pyslot] fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let range = if args.args.len() <= 1 { - let stop = args.bind(vm)?; + let range = if args.args.is_empty() { + return Err(Callee::of::(vm).arity_error(1..=3, 0, vm)); + } else if args.args.len() == 1 { + let stop = args.bind_for(vm, Callee::of::(vm))?; Self::new(cls, stop, vm) } else { - let (start, stop, step) = args.bind(vm)?; + let (start, stop, step) = args.bind_for(vm, Callee::of::(vm))?; Self::new_from(cls, start, stop, step, vm) }?; diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index 7e6f43bbb4c..ff2253ae13e 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -18,7 +18,9 @@ use crate::{ }, convert::ToPyResult, dict_inner::{self, DictSize}, - function::{ArgIterable, FuncArgs, OptionalArg, PosArgs, PyArithmeticValue, PyComparisonValue}, + function::{ + ArgIterable, Callee, FuncArgs, OptionalArg, PosArgs, PyArithmeticValue, PyComparisonValue, + }, protocol::{PyIterReturn, PyNumberMethods, PySequenceMethods}, recursion::ReprGuard, types::AsNumber, @@ -1077,7 +1079,7 @@ impl Constructor for PyFrozenSet { // Optimizations for exact frozenset type let iterable_opt = if is_exact_frozenset || is_frozenset_init { - let iterable: OptionalArg = args.bind(vm)?; + let iterable: OptionalArg = args.bind_for(vm, Callee::of::(vm))?; // Return exact frozenset as-is if is_exact_frozenset diff --git a/crates/vm/src/builtins/singletons.rs b/crates/vm/src/builtins/singletons.rs index 2928c523b1b..40c55138f72 100644 --- a/crates/vm/src/builtins/singletons.rs +++ b/crates/vm/src/builtins/singletons.rs @@ -4,7 +4,7 @@ use crate::{ class::PyClassImpl, common::hash::PyHash, convert::ToPyObject, - function::{FuncArgs, PyComparisonValue}, + function::{Callee, FuncArgs, PyComparisonValue}, protocol::PyNumberMethods, types::{AsNumber, Comparable, Constructor, Hashable, PyComparisonOp, Representable}, }; @@ -41,7 +41,7 @@ impl Constructor for PyNone { type Args = (); fn slot_new(_cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let _: () = args.bind(vm)?; + let _: () = args.bind_for(vm, Callee::of::(vm))?; Ok(vm.ctx.none.clone().into()) } @@ -110,7 +110,7 @@ impl Constructor for PyNotImplemented { type Args = (); fn slot_new(_cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let _: () = args.bind(vm)?; + let _: () = args.bind_for(vm, Callee::of::(vm))?; Ok(vm.ctx.not_implemented.clone().into()) } diff --git a/crates/vm/src/builtins/slice.rs b/crates/vm/src/builtins/slice.rs index 4eb6c0f9b0e..8f2eb6bfb18 100644 --- a/crates/vm/src/builtins/slice.rs +++ b/crates/vm/src/builtins/slice.rs @@ -10,7 +10,7 @@ use crate::{ class::PyClassImpl, common::hash::{PyHash, PyUHash}, convert::ToPyObject, - function::{ArgIndex, FuncArgs, OptionalArg, PyComparisonValue}, + function::{ArgIndex, Callee, FuncArgs, OptionalArg, PyComparisonValue}, sliceable::SaturatedSlice, types::{Comparable, Constructor, Hashable, PyComparisonOp, Representable}, }; @@ -128,10 +128,10 @@ impl PySlice { fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { let slice: Self = match args.args.len() { 0 => { - return Err(vm.new_type_error("slice() must have at least one arguments.")); + return Err(Callee::of::(vm).arity_error(1..=3, 0, vm)); } 1 => { - let stop = args.bind(vm)?; + let stop = args.bind_for(vm, Callee::of::(vm))?; Self { start: None, stop, @@ -140,7 +140,7 @@ impl PySlice { } _ => { let (start, stop, step): (PyObjectRef, PyObjectRef, OptionalArg) = - args.bind(vm)?; + args.bind_for(vm, Callee::of::(vm))?; Self { start: Some(start), stop, @@ -388,7 +388,7 @@ impl Constructor for PyEllipsis { type Args = (); fn slot_new(_cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let _: () = args.bind(vm)?; + let _: () = args.bind_for(vm, Callee::of::(vm))?; Ok(vm.ctx.ellipsis.clone().into()) } diff --git a/crates/vm/src/builtins/staticmethod.rs b/crates/vm/src/builtins/staticmethod.rs index 8ae31b67b5c..2af183b4102 100644 --- a/crates/vm/src/builtins/staticmethod.rs +++ b/crates/vm/src/builtins/staticmethod.rs @@ -3,7 +3,7 @@ use crate::{ AsObject, Context, Py, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, class::PyClassImpl, common::lock::PyMutex, - function::{FuncArgs, PySetterValue}, + function::{Callee, FuncArgs, PySetterValue}, types::{Callable, Constructor, GetDescriptor, Initializer, Representable}, }; @@ -48,7 +48,7 @@ impl Constructor for PyStaticMethod { // copying its attributes to `__init__` so that subclasses overriding // `__init__` without calling `super().__init__()` see `__func__` as // `None`, matching CPython. - let _: Self::Args = args.bind(vm)?; + let _: Self::Args = args.bind_for(vm, Callee::of::(vm))?; let result = Self { callable: PyMutex::new(vm.ctx.none()), } diff --git a/crates/vm/src/builtins/str.rs b/crates/vm/src/builtins/str.rs index 29a1a9addeb..3bc948d5647 100644 --- a/crates/vm/src/builtins/str.rs +++ b/crates/vm/src/builtins/str.rs @@ -17,7 +17,9 @@ use crate::{ }, convert::{IntoPyException, ToPyException, ToPyObject, ToPyResult}, format::{format, format_map}, - function::{ArgIterable, ArgSize, FuncArgs, OptionalArg, OptionalOption, PyComparisonValue}, + function::{ + ArgIterable, ArgSize, Callee, FuncArgs, OptionalArg, OptionalOption, PyComparisonValue, + }, intern::PyInterned, object::{MaybeTraverse, Traverse, TraverseFn}, protocol::{ @@ -409,7 +411,7 @@ impl Constructor for PyStr { return Ok(func_args.args[0].clone()); } - let args: Self::Args = func_args.bind(vm)?; + let args: Self::Args = func_args.bind_for(vm, Callee::of::(vm))?; // CPython parity: when cls is exactly str, return the __str__ / __repr__ // result as-is so any str subclass type the user returned is preserved diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index 200a9054d8d..b4f2c2adf8c 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -10,7 +10,7 @@ use crate::{ atomic_func, class::PyClassImpl, convert::{ToPyObject, TransmuteFromObject}, - function::{ArgSize, FuncArgs, OptionalArg, PyArithmeticValue, PyComparisonValue}, + function::{ArgSize, Callee, FuncArgs, OptionalArg, PyArithmeticValue, PyComparisonValue}, iter::PyExactSizeIterator, protocol::{PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods}, recursion::ReprGuard, @@ -221,7 +221,7 @@ impl Constructor for PyTuple { type Args = Vec; fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let iterable: OptionalArg = args.bind(vm)?; + let iterable: OptionalArg = args.bind_for(vm, Callee::of::(vm))?; // Optimizations for exact tuple type if cls.is(vm.ctx.types.tuple_type) { diff --git a/crates/vm/src/builtins/weakproxy.rs b/crates/vm/src/builtins/weakproxy.rs index 7f39044b4f8..22a40448ef1 100644 --- a/crates/vm/src/builtins/weakproxy.rs +++ b/crates/vm/src/builtins/weakproxy.rs @@ -4,7 +4,9 @@ use crate::{ Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, atomic_func, class::PyClassImpl, common::hash::PyHash, - function::{FuncArgs, OptionalArg, PyArithmeticValue, PyComparisonValue, PySetterValue}, + function::{ + Callee, FuncArgs, OptionalArg, PyArithmeticValue, PyComparisonValue, PySetterValue, + }, protocol::{PyIter, PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods}, stdlib::builtins::reversed, types::{ @@ -48,7 +50,7 @@ impl Constructor for PyWeakProxy { fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { let _ = cls; - let Self::Args { referent, callback } = args.bind(vm)?; + let Self::Args { referent, callback } = args.bind_for(vm, Callee::of::(vm))?; let callback = callback .into_option() .filter(|callback| !vm.is_none(callback)); diff --git a/crates/vm/src/exception_group.rs b/crates/vm/src/exception_group.rs index 333c268fbc5..f8d50b5a68f 100644 --- a/crates/vm/src/exception_group.rs +++ b/crates/vm/src/exception_group.rs @@ -245,7 +245,7 @@ pub(super) mod types { type Args = crate::function::PosArgs; fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let args: Self::Args = args.bind(vm)?; + let args: Self::Args = args.bind_for(vm, crate::function::Callee::of::(vm))?; let args = args.into_vec(); // Validate exactly 2 positional arguments if args.len() != 2 { diff --git a/crates/vm/src/stdlib/_ctypes/simple.rs b/crates/vm/src/stdlib/_ctypes/simple.rs index 9699cef984b..69dc2a858da 100644 --- a/crates/vm/src/stdlib/_ctypes/simple.rs +++ b/crates/vm/src/stdlib/_ctypes/simple.rs @@ -984,7 +984,7 @@ impl Constructor for PyCSimple { type Args = (OptionalArg,); fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let args: Self::Args = args.bind(vm)?; + let args: Self::Args = args.bind_for(vm, crate::function::Callee::of::(vm))?; let _type_ = cls .type_code(vm) .ok_or_else(|| vm.new_type_error("abstract class"))?; diff --git a/crates/vm/src/stdlib/_typing.rs b/crates/vm/src/stdlib/_typing.rs index 0b19d8e3c32..1b05e048fbd 100644 --- a/crates/vm/src/stdlib/_typing.rs +++ b/crates/vm/src/stdlib/_typing.rs @@ -78,7 +78,7 @@ pub(crate) mod decl { type Args = (); fn slot_new(_cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let _: () = args.bind(vm)?; + let _: () = args.bind_for(vm, crate::function::Callee::of::(vm))?; Ok(vm.ctx.typing_no_default.clone().into()) } diff --git a/crates/vm/src/stdlib/itertools.rs b/crates/vm/src/stdlib/itertools.rs index 5f85f1b7238..4c86b61d67a 100644 --- a/crates/vm/src/stdlib/itertools.rs +++ b/crates/vm/src/stdlib/itertools.rs @@ -9,7 +9,7 @@ mod decl { }, common::lock::{PyMutex, PyRwLock, PyRwLockWriteGuard}, convert::ToPyObject, - function::{FuncArgs, OptionalArg, OptionalOption, PosArgs}, + function::{Callee, FuncArgs, OptionalArg, OptionalOption, PosArgs}, protocol::{PyIter, PyIterReturn, PyNumber}, raise_if_stop, stdlib::sys, @@ -761,13 +761,14 @@ mod decl { ))); } 2 => { - let (iter, stop): (PyObjectRef, PyObjectRef) = args.bind(vm)?; + let (iter, stop): (PyObjectRef, PyObjectRef) = + args.bind_for(vm, Callee::of::(vm))?; (iter, 0usize, stop, 1usize) } _ => { let (iter, start, stop, step) = if args.args.len() == 3 { let (iter, start, stop): (PyObjectRef, PyObjectRef, PyObjectRef) = - args.bind(vm)?; + args.bind_for(vm, Callee::of::(vm))?; (iter, start, stop, 1usize) } else { let (iter, start, stop, step): ( @@ -775,7 +776,7 @@ mod decl { PyObjectRef, PyObjectRef, PyObjectRef, - ) = args.bind(vm)?; + ) = args.bind_for(vm, Callee::of::(vm))?; let step = if !vm.is_none(&step) { pyobject_to_opt_usize(step, "Step", vm)? diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index 523097ad971..48565b52817 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -1345,7 +1345,7 @@ pub(super) mod _os { fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { let result = crate::types::struct_sequence_new( cls.clone(), - args.bind(vm)?, + args.bind_for(vm, crate::function::Callee::of::(vm))?, StatResultData::OPTIONAL_FIELD_NAMES, vm, )?; @@ -2002,7 +2002,7 @@ pub(super) mod _os { fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { crate::types::struct_sequence_new( cls, - args.bind(vm)?, + args.bind_for(vm, crate::function::Callee::of::(vm))?, StatvfsResultData::OPTIONAL_FIELD_NAMES, vm, ) diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index e175d99ced4..a5113d7cfd9 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -2456,7 +2456,8 @@ mod posix_sched { vm: &VirtualMachine, ) -> PyResult { use crate::PyPayload; - let SchedParamArgs { sched_priority } = args.bind(vm)?; + let SchedParamArgs { sched_priority } = + args.bind_for(vm, crate::function::Callee::of::(vm))?; let items = vec![sched_priority]; crate::builtins::PyTuple::new_unchecked(items.into_boxed_slice()) .into_ref_with_type(vm, cls) diff --git a/crates/vm/src/stdlib/time.rs b/crates/vm/src/stdlib/time.rs index a5daa9cd2ff..f3a3c7c56c9 100644 --- a/crates/vm/src/stdlib/time.rs +++ b/crates/vm/src/stdlib/time.rs @@ -17,7 +17,7 @@ mod decl { use crate::{ AsObject, Py, PyObjectRef, PyResult, VirtualMachine, builtins::{PyStrRef, PyTypeRef}, - function::{Either, FuncArgs, OptionalArg}, + function::{Callee, Either, FuncArgs, OptionalArg}, types::{PyStructSequence, PyStructSequenceData, struct_sequence_new}, }; #[cfg(any(unix, windows))] @@ -813,7 +813,7 @@ mod decl { fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { struct_sequence_new( cls, - args.bind(vm)?, + args.bind_for(vm, Callee::of::(vm))?, StructTimeData::OPTIONAL_FIELD_NAMES, vm, ) diff --git a/crates/vm/src/types/slot.rs b/crates/vm/src/types/slot.rs index c3180a5c60a..6dfd2190df9 100644 --- a/crates/vm/src/types/slot.rs +++ b/crates/vm/src/types/slot.rs @@ -7,7 +7,7 @@ use crate::{ bytecode::ComparisonOperator, common::hash::{PyHash, fix_sentinel, hash_bigint}, convert::ToPyObject, - function::{Either, FromArgs, FuncArgs, PyComparisonValue, PyMethodDef, PySetterValue}, + function::{Callee, Either, FromArgs, FuncArgs, PyComparisonValue, PyMethodDef, PySetterValue}, protocol::{ BufferFlags, PyBuffer, PyIterReturn, PyMapping, PyMappingMethods, PyMappingSlots, PyNumber, PyNumberMethods, PyNumberSlots, PySequence, PySequenceMethods, PySequenceSlots, @@ -1736,7 +1736,9 @@ pub trait Constructor: PyPayload + core::fmt::Debug { #[inline] #[pyslot] fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - let args: Self::Args = args.bind(vm)?; + // The name is the type the slot was written for, not the subclass being + // constructed, so a subclass reports what its base declares. + let args: Self::Args = args.bind_for(vm, Callee::of::(vm))?; let payload = Self::py_new(&cls, args, vm)?; payload.into_ref_with_type(vm, cls).map(Into::into) } @@ -1802,7 +1804,7 @@ pub trait Initializer: PyPayload { return Err(err); } }; - let args: Self::Args = args.bind(vm)?; + let args: Self::Args = args.bind_for(vm, Callee::of::(vm))?; Self::init(zelf, args, vm) } diff --git a/crates/vm/src/types/structseq.rs b/crates/vm/src/types/structseq.rs index 812a8a1fc57..76a0413d5b4 100644 --- a/crates/vm/src/types/structseq.rs +++ b/crates/vm/src/types/structseq.rs @@ -6,7 +6,9 @@ use crate::{ PyBaseExceptionRef, PyDict, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef, }, class::{PyClassImpl, StaticType}, - function::{Either, FuncArgs, OptionalArg, PyComparisonValue, PyMethodDef, PyMethodFlags}, + function::{ + Callee, Either, FuncArgs, OptionalArg, PyComparisonValue, PyMethodDef, PyMethodFlags, + }, iter::PyExactSizeIterator, protocol::{PyMappingMethods, PySequenceMethods}, sliceable::{SequenceIndex, SliceableSequenceOp}, @@ -250,7 +252,12 @@ pub trait PyStructSequence: StaticType + PyClassImpl + Sized + 'static { #[pyslot] fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { - struct_sequence_new(cls, args.bind(vm)?, Self::Data::OPTIONAL_FIELD_NAMES, vm) + struct_sequence_new( + cls, + args.bind_for(vm, Callee::for_type(Self::static_type()))?, + Self::Data::OPTIONAL_FIELD_NAMES, + vm, + ) } /// Convert a Data struct into a PyStructSequence instance. diff --git a/extra_tests/snippets/vm_argument_errors.py b/extra_tests/snippets/vm_argument_errors.py new file mode 100644 index 00000000000..6de9de8a811 --- /dev/null +++ b/extra_tests/snippets/vm_argument_errors.py @@ -0,0 +1,97 @@ +# A call that binds badly says which function it was binding for, counts only +# the arguments the caller wrote, and agrees with itself about singular and +# plural. Every message here is the one CPython raises, so this file is run +# against both. +import array +import collections + +from testutils import assert_raises + + +def check(exc_type, message, fn, *args, **kwargs): + try: + fn(*args, **kwargs) + except exc_type as e: + assert str(e) == message, f"{str(e)!r} != {message!r}" + else: + raise AssertionError(f"{fn} did not raise {exc_type.__name__}") + + +def test_the_message_names_the_function(): + check(TypeError, "find expected at least 1 argument, got 0", "".find) + check(TypeError, "center expected at least 1 argument, got 0", "a".center) + check(TypeError, "fromkeys expected at least 1 argument, got 0", dict.fromkeys) + check(TypeError, "divmod expected 2 arguments, got 1", divmod, 1) + check(TypeError, "insert expected 2 arguments, got 1", [].insert, 1) + check(TypeError, "sorted expected 1 argument, got 0", sorted) + check(TypeError, "min expected at least 1 argument, got 0", min) + + +def test_a_method_does_not_count_its_instance(): + # `self` fills the instance parameter, so it is in neither number, whether + # the call bound it or wrote it. + check(TypeError, "insert expected 2 arguments, got 1", [].insert, 1) + check(TypeError, "insert expected 2 arguments, got 1", list.insert, [], 1) + + +def test_one_argument_is_singular(): + check(TypeError, "sorted expected 1 argument, got 0", sorted) + check(TypeError, "divmod expected 2 arguments, got 1", divmod, 1) + check(TypeError, "float expected at most 1 argument, got 2", float, 1, 2) + check(TypeError, "int expected at most 2 arguments, got 3", int, 1, 2, 3) + + +def test_a_keyword_it_did_not_expect_is_quoted(): + check( + TypeError, + "split() got an unexpected keyword argument 'bogus'", + "".split, + bogus=1, + ) + check( + TypeError, + "round() got an unexpected keyword argument 'bogus'", + round, + 1, + bogus=2, + ) + + +def test_a_parameter_a_call_may_name_is_named_back(): + check( + TypeError, + "memoryview() missing required argument 'object' (pos 1)", + memoryview, + ) + check( + TypeError, + "cast() missing required argument 'format' (pos 1)", + memoryview(b"a").cast, + ) + + +def test_a_type_is_named_by_the_type_it_builds(): + # The name is the type the slot was written for, without the module and + # without the subclass being constructed. + class Deque(collections.deque): + pass + + check(TypeError, "range expected at least 1 argument, got 0", range) + check(TypeError, "slice expected at least 1 argument, got 0", slice) + check(TypeError, "filter expected 2 arguments, got 0", filter) + check(TypeError, "set expected at most 1 argument, got 2", set, 1, 2) + check(TypeError, "tuple expected at most 1 argument, got 2", tuple, 1, 2) + check(TypeError, "staticmethod expected 1 argument, got 0", staticmethod) + check(TypeError, "classmethod expected 1 argument, got 0", classmethod) + with assert_raises(TypeError): + Deque(1, 2, 3) + with assert_raises(TypeError): + array.array() + + +test_the_message_names_the_function() +test_a_method_does_not_count_its_instance() +test_one_argument_is_singular() +test_a_keyword_it_did_not_expect_is_quoted() +test_a_parameter_a_call_may_name_is_named_back() +test_a_type_is_named_by_the_type_it_builds()