Name the function a failed argument binding was binding for - #8630
Conversation
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
`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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (34)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangesCallee-aware argument handling
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The change improves built-in and method argument error messages without changing accepted call behavior; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Caller
participant NativeFunction
participant Callee
participant FuncArgs
participant TypeError
Caller->>NativeFunction: invoke with arguments
NativeFunction->>Callee: create call metadata
NativeFunction->>FuncArgs: bind_for with Callee
FuncArgs->>TypeError: construct callee-specific error when binding fails
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] test: cpython/Lib/test/test_range.py (TODO: 2) dependencies: dependent tests: (no tests depend on range) [ ] test: cpython/Lib/test/test_bytes.py (TODO: 17) dependencies: dependent tests: (no tests depend on bytes) Legend:
|
* 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 * 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
A built-in call that binds its arguments badly said neither which function it
was binding for nor, for a method, the right numbers.
selfwas prepended to the arguments before binding, so it was counted asboth an expected parameter and a given argument: every method reported
numbers one too high.
What this does
Calleecarries what a message says about the function being called: thename, and whether the leading argument fills the instance parameter.
PyNativeFntakes one;PyNativeFunction,PyMethodDescriptorand the fourmethod-descriptor call specializations in
frame.rssupply it.FuncArgs::bind_forsubtracts the instance from both numbers, the waydescrobject.creportsnargs - 1whether the caller wroteselfor anattribute lookup bound it.
Constructor::slot_newandInitializer::slot_initname the type the slotwas written for — not the subclass being constructed, and not the module
tp_namecarries — and so do the 23 slots that override them.The messages are the ones their counterparts raise:
_PyArg_CheckPositionalfind expected at least 1 argument, got 0_PyArg_UnpackKeywordssplit() got an unexpected keyword argument 'bogus'_PyArg_UnpackKeywordscast() missing required argument 'format' (pos 1)A parameter a call may pass by name is named back when it is missing; a
positional-only one 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.fnameis NULL.slice()andrange()counted their own arguments and raised messages oftheir own (
slice() must have at least one arguments.); they now raise theone
_PyArg_CheckPositionalraises.After
Verification
Against CPython 3.14.6 over 43 calls, 23 now produce the identical string;
of the 16 measured first, none matched before and 9 do now.
@unittest.expectedFailuredecorators go away, all of them teststhis fixes:
test_find_etc_raise_correct_error_messagesintest_bytesand
string_tests(CPython issue 11828), andtest_range_constructor_error_messagesintest_range.extra_tests/snippets/vm_argument_errors.pypasses under RustPython andunder CPython 3.14.6.
cargo fmt --checkclean; clippy with CI's arguments exits 0; the firstcommit compiles on its own.
Left out
len() takes exactly one argument (0 given)anddeque() takes at most 2 arguments (3 given)come from theMETH_OandPyArg_ParseTuplepaths. RustPython has one binding path, soevery message here is the
_PyArg_CheckPositionalform. The counts and thename are right; the wording differs for 12 of the 43 calls. Matching those
needs the
METH_*families themselves.takes no keyword argumentsforabs(x=1),[].count(x=1),range(stop=5)._PyArg_NoKeywordsneeds to know the function accepts nokeywords.
infer_native_call_flagscomputes that, but its own commentcalls it a best-effort mapping, and using it to reject a call would break
working code wherever it is wrong.
b"".hex(1, 2, 3)stillsays
unexpected type intwhere CPython sayshex() takes at most 2 arguments (3 given), because binding converts as it goes. Checking upfront needs
FromArgsto report whether it is variadic —PosArgshasarity
0..=0today, so an early check would reject every varargs call.Summary by CodeRabbit
Bug Fixes
range()andslice().Tests