Skip to content

Improve CPython compatibility related with PyBoundMethod#7476

Merged
youknowone merged 3 commits intoRustPython:mainfrom
moreal:method-fixes
Mar 20, 2026
Merged

Improve CPython compatibility related with PyBoundMethod#7476
youknowone merged 3 commits intoRustPython:mainfrom
moreal:method-fixes

Conversation

@moreal
Copy link
Contributor

@moreal moreal commented Mar 20, 2026

This pull request adjusts PyBoundMethod for compatibility with CPython.

Behavior changes

  1. Correct PyBoundMethod initialization validation
  2. Implement missing __get__
  3. Raise error when __name__ when calling PyBoundMethod __reduce__
    class A:
        def f(): ...
    
    class B:
        def __call__(self): ...
    
    print(type(A().f)(B(), 1).__reduce__())

CPython (3.14.3)

>>> class A:
...     def f(): ...
...
>>> A().f
<bound method A.f of <__main__.A object at 0x1084270e0>>
>>> type(A().f)
<class 'method'>
>>> type(A().f)(None)
Traceback (most recent call last):
  File "<python-input-3>", line 1, in <module>
    type(A().f)(None)
    ~~~~~~~~~~~^^^^^^
TypeError: method expected 2 arguments, got 1
>>> type(A().f)(None, None)
Traceback (most recent call last):
  File "<python-input-4>", line 1, in <module>
    type(A().f)(None, None)
    ~~~~~~~~~~~^^^^^^^^^^^^
TypeError: first argument must be callable
>>> type(A().f)(lambda x: x, None)
Traceback (most recent call last):
  File "<python-input-5>", line 1, in <module>
    type(A().f)(lambda x: x, None)
    ~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^
TypeError: instance must not be None
>>> type(A().f)(lambda x: x, 1)
<bound method <lambda> of 1>
>>> type(A().f).__get__
<slot wrapper '__get__' of 'method' objects>
Traceback (most recent call last):
  File "/path/to/RustPython/hello.py", line 10, in <module>
    print(type(A().f)(B(), 1).__reduce__())
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
AttributeError: 'B' object has no attribute '__name__'. Did you mean: '__ne__'?

RustPython (before this pull request)

>>>>> class A:
.....   def f(): ...
.....
>>>>> A().f
<bound method A.f of <__main__.A object at 0x9e0c380a8>>
>>>>> type(A().f)
<class 'method'>
>>>>> type(A().f)(None, None)
<bound method ? of None>
>>>>> type(A().f)(lambda x: x, None)
<bound method <lambda> of None>
>>>>> type(A().f)(lambda x: x, 1)
<bound method <lambda> of 1>
>>>>> type(A().f).__get__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: type object 'method' has no attribute '__get__'. Did you mean: '__ge__'?
(<built-in function getattr>, (1, None))

RustPython (after this pull request)

>>>>> class A:
.....   def f(): ...
.....
>>>>> A().f
<bound method A.f of <__main__.A object at 0xb795d8818>>
>>>>> type(A().f)
<class 'method'>
>>>>> type(A().f)(None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: expected at least 2 arguments, got 1
>>>>> type(A().f)(None, None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: first argument must be callable
>>>>> type(A().f)(lambda x: x, None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: instance must not be None
>>>>> type(A().f)(lambda x: x, 1)
<bound method <lambda> of 1>
>>>>> type(A().f).__get__
<slot wrapper '__get__' of 'method' objects>
Traceback (most recent call last):
  File "hello.py", line 9, in <module>
    print(type(A().f)(B(), 1).__reduce__())
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
AttributeError: 'B' object has no attribute '__name__'. Did you mean: '__ne__'?

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced validation when creating bound methods; passing a non-callable function or None as the object parameter now correctly raises a TypeError.
    • Improved error handling in bound method serialization; errors during attribute access are now properly propagated instead of being silently ignored.

moreal and others added 3 commits March 20, 2026 22:43
CPython's method_descr_get always returns the bound method unchanged.
This preserves the original binding when __get__ is called on an
already-bound method (e.g. a.meth.__get__(b, B) still returns a).

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Reject non-callable functions and None instances, matching CPython's
method_new which checks PyCallable_Check(func) and instance != Py_None.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Previously swallowed errors from get_attr with .ok(), silently
returning None. Now propagates errors matching CPython's method_reduce.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@github-actions
Copy link
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] test: cpython/Lib/test/test_descr.py (TODO: 42)
[ ] test: cpython/Lib/test/test_descrtut.py (TODO: 3)

dependencies:

dependent tests: (no tests depend on descr)

Legend:

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 00f64966-4287-4f2a-a2ea-5075ed8dd1b0

📥 Commits

Reviewing files that changed from the base of the PR and between 38f742a and b725ca7.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_descr.py is excluded by !Lib/**
📒 Files selected for processing (1)
  • crates/vm/src/builtins/function.rs

📝 Walkthrough

Walkthrough

This PR enhances PyBoundMethod with stricter validation and improved error handling. It adds a GetDescriptor implementation, enforces that the function argument is callable and object argument is non-None during construction, and modifies __reduce__ to propagate errors instead of silently returning None values.

Changes

Cohort / File(s) Summary
PyBoundMethod enhancements
crates/vm/src/builtins/function.rs
Added GetDescriptor trait implementation for descriptor protocol support; enhanced py_new constructor with validation for callable function and non-None object arguments; modified __reduce__ to propagate errors via PyResult instead of converting failures to None.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • youknowone

Poem

🐰 A bound method now knows its own worth,
Checking that functions are truly callable from birth,
Descriptors dance with the new GetDescriptor trait,
No more silent None—errors propagate, never too late! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: improving CPython compatibility for PyBoundMethod. It is specific and clearly communicates the primary focus of the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

@moreal moreal marked this pull request as ready for review March 20, 2026 15:00
@moreal moreal changed the title WIP: Improve CPython compatibility related with PyBoundMethod Improve CPython compatibility related with PyBoundMethod Mar 20, 2026
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

👍

@youknowone youknowone merged commit a1203ae into RustPython:main Mar 20, 2026
33 of 34 checks passed
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