Skip to content

Name the function a failed argument binding was binding for - #8630

Merged
youknowone merged 2 commits into
RustPython:mainfrom
youknowone:RustPython-3
Aug 31, 2026
Merged

youknowone merged 2 commits into
RustPython:mainfrom
youknowone:RustPython-3

Conversation

@youknowone

@youknowone youknowone commented Aug 31, 2026

Copy link
Copy Markdown
Member

A built-in call that binds its arguments badly said neither which function it
was binding for nor, for a method, the right numbers.

>>> [].append()
TypeError: expected at least 2 arguments, got 1     # 2 and 1 count `self`
>>> [].append(1, 2)
TypeError: expected at most 2 arguments, got 3
>>> "".split(bogus=1)
TypeError: Unexpected keyword argument bogus
>>> sorted()
TypeError: expected at least 1 arguments, got 0     # "1 arguments"
>>> float(1, 2)
TypeError: expected at most 1 arguments, got 2

self was prepended to the arguments before binding, so it was counted as
both an expected parameter and a given argument: every method reported
numbers one too high.

What this does

Callee carries what a message says about the function being called: the
name, and whether the leading argument fills the instance parameter.
PyNativeFn takes one; PyNativeFunction, PyMethodDescriptor and the four
method-descriptor call specializations in frame.rs supply it.
FuncArgs::bind_for subtracts the instance from both numbers, the way
descrobject.c reports nargs - 1 whether the caller wrote self or an
attribute lookup bound it.

Constructor::slot_new and Initializer::slot_init name the type the slot
was written for — not the subclass being constructed, and not the module
tp_name carries — and so do the 23 slots that override them.

The messages are the ones their counterparts raise:

source message
_PyArg_CheckPositional find expected at least 1 argument, got 0
_PyArg_UnpackKeywords split() got an unexpected keyword argument 'bogus'
_PyArg_UnpackKeywords cast() 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.fname is NULL.

slice() and range() counted their own arguments and raised messages of
their own (slice() must have at least one arguments.); they now raise the
one _PyArg_CheckPositional raises.

After

>>> [].append()
TypeError: append expected 1 argument, got 0
>>> [].append(1, 2)
TypeError: append expected 1 argument, got 2
>>> "".split(bogus=1)
TypeError: split() got an unexpected keyword argument 'bogus'
>>> sorted()
TypeError: sorted expected 1 argument, got 0
>>> float(1, 2)
TypeError: float expected at most 1 argument, got 2
>>> memoryview()
TypeError: memoryview() missing required argument 'object' (pos 1)

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.

  • 131 regrtest suites, 25,202 tests: SUCCESS.
  • Three @unittest.expectedFailure decorators go away, all of them tests
    this fixes: test_find_etc_raise_correct_error_messages in test_bytes
    and string_tests (CPython issue 11828), and
    test_range_constructor_error_messages in test_range.
  • extra_tests/snippets/vm_argument_errors.py passes under RustPython and
    under CPython 3.14.6.
  • cargo fmt --check clean; clippy with CI's arguments exits 0; the first
    commit compiles on its own.

Left out

  • CPython's other message families. len() takes exactly one argument (0 given) and deque() takes at most 2 arguments (3 given) come from the
    METH_O and PyArg_ParseTuple paths. RustPython has one binding path, so
    every message here is the _PyArg_CheckPositional form. The counts and the
    name are right; the wording differs for 12 of the 43 calls. Matching those
    needs the METH_* families themselves.
  • takes no keyword arguments for abs(x=1), [].count(x=1),
    range(stop=5). _PyArg_NoKeywords needs to know the function accepts no
    keywords. infer_native_call_flags computes that, but its own comment
    calls it a best-effort mapping, and using it to reject a call would break
    working code wherever it is wrong.
  • Counting the arguments before converting them. b"".hex(1, 2, 3) still
    says unexpected type int where CPython says hex() takes at most 2 arguments (3 given), because binding converts as it goes. Checking up
    front needs FromArgs to report whether it is variadic — PosArgs has
    arity 0..=0 today, so an early check would reject every varargs call.

Summary by CodeRabbit

  • Bug Fixes

    • Improved argument validation across built-in functions, methods, and constructors.
    • Error messages now identify the called function or type and accurately report expected and provided arguments.
    • Missing required arguments now include the parameter name and position.
    • Corrected zero-argument errors for range() and slice().
    • Improved handling of unexpected keyword arguments and bound method arguments.
  • Tests

    • Added coverage verifying CPython-compatible argument error messages across built-ins, methods, and types.

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
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: ca4ef0d3-e4d2-4d2d-8105-f074fb412d07

📥 Commits

Reviewing files that changed from the base of the PR and between 6a3a8b0 and 0d4e960.

⛔ Files ignored due to path filters (3)
  • Lib/test/string_tests.py is excluded by !Lib/**
  • Lib/test/test_bytes.py is excluded by !Lib/**
  • Lib/test/test_range.py is excluded by !Lib/**
📒 Files selected for processing (34)
  • crates/derive-impl/src/from_args.rs
  • crates/stdlib/src/contextvars.rs
  • crates/vm/src/builtins/bool.rs
  • crates/vm/src/builtins/builtin_func.rs
  • crates/vm/src/builtins/bytes.rs
  • crates/vm/src/builtins/classmethod.rs
  • crates/vm/src/builtins/complex.rs
  • crates/vm/src/builtins/descriptor.rs
  • crates/vm/src/builtins/float.rs
  • crates/vm/src/builtins/int.rs
  • crates/vm/src/builtins/object.rs
  • crates/vm/src/builtins/range.rs
  • crates/vm/src/builtins/set.rs
  • crates/vm/src/builtins/singletons.rs
  • crates/vm/src/builtins/slice.rs
  • crates/vm/src/builtins/staticmethod.rs
  • crates/vm/src/builtins/str.rs
  • crates/vm/src/builtins/tuple.rs
  • crates/vm/src/builtins/weakproxy.rs
  • crates/vm/src/exception_group.rs
  • crates/vm/src/frame.rs
  • crates/vm/src/function/argument.rs
  • crates/vm/src/function/builtin.rs
  • crates/vm/src/function/method.rs
  • crates/vm/src/function/mod.rs
  • crates/vm/src/stdlib/_ctypes/simple.rs
  • crates/vm/src/stdlib/_typing.rs
  • crates/vm/src/stdlib/itertools.rs
  • crates/vm/src/stdlib/os.rs
  • crates/vm/src/stdlib/posix.rs
  • crates/vm/src/stdlib/time.rs
  • crates/vm/src/types/slot.rs
  • crates/vm/src/types/structseq.rs
  • extra_tests/snippets/vm_argument_errors.py

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change adds Callee metadata to native argument binding. It updates argument error construction, native invocation paths, built-in constructors, and slot handling. New tests validate CPython-style error messages.

Changes

Callee-aware argument handling

Layer / File(s) Summary
Argument binding and error contract
crates/derive-impl/src/from_args.rs, crates/vm/src/function/argument.rs, crates/vm/src/function/mod.rs
Callee now supplies function names and instance-argument metadata. Missing required arguments carry names and positions. Error messages use callee context.
Native call plumbing
crates/vm/src/function/builtin.rs, crates/vm/src/function/method.rs, crates/vm/src/builtins/builtin_func.rs, crates/vm/src/builtins/descriptor.rs, crates/vm/src/frame.rs
Native function traits and invocation paths now forward Callee. Method calls mark prepended instance arguments.
Constructor binding migration
crates/vm/src/builtins/*, crates/vm/src/stdlib/*, crates/vm/src/types/*, crates/stdlib/src/contextvars.rs, crates/vm/src/exception_group.rs
Constructors and slots now use bind_for with the declared type or method callee. range() and slice() use callee-aware arity errors for zero-argument calls.
Error validation
extra_tests/snippets/vm_argument_errors.py
Tests assert exact function names, argument counts, keyword errors, required parameters, and slot type names.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 0d4e9

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
Loading

Suggested reviewers: shaharnaveh, moreal

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.79% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 63 functions across 34 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: include the callable name when argument binding fails. The wording is somewhat awkward, but it is specific and related to the primary objective.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The 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:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@codspeed

codspeed Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing youknowone:RustPython-3 (0d4e960) with main (6a3a8b0)

Open in CodSpeed

@youknowone
youknowone marked this pull request as ready for review August 31, 2026 17:36

@ShaharNaveh ShaharNaveh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm:)

@youknowone
youknowone merged commit 5494a3a into RustPython:main Aug 31, 2026
31 checks passed
@youknowone
youknowone deleted the RustPython-3 branch August 31, 2026 21:58
youknowone added a commit that referenced this pull request Sep 16, 2026
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants