Mark LRU cache wrappers as method descriptors by youknowdot · Pull Request #9072 · RustPython/RustPython · GitHub

Summary

Extract the independent _lru_cache_wrapper method-descriptor flag from #8954. CPython 3.14.7 and 3.14.8 already set Py_TPFLAGS_METHOD_DESCRIPTOR on this type. RustPython already implements the required class/instance descriptor binding, so this enables its existing guarded method-call path without changing the wrapper's call implementation.

The partial/vectorcall rewrite and other Python 3.15 changes remain separate. Checked the relevant open functools/ownership work for overlap; this is the single wrapper flag, not a cache or ownership redesign.

Validation

Independent source/diff review against CPython 3.14.7 and 3.14.8, plus RustPython's descriptor binding and guarded consumers.
Existing stdlib_functools.py gains one focused Python assertion on the actual C-style wrapper type's flag. Full snippet passes on CPython 3.14.7, CPython 3.14.8, and the patched native RustPython build. The pre-patch native binary fails that new assertion.
Six unchanged native TestLRUC tests pass: method binding, copy, deepcopy, pickle, keyword arguments, and unlimited cache.
Selected snippet pytest: 2 passed using the CPython 3.14.7 driver and exact patched native binary.
Normal commit hooks; native debug build; VM and separately configured C-API Clippy with warnings denied.

This validates the flag contract and existing behavior, not a measured performance improvement. Local validation is Linux and bounded; the full hosted matrix remains required.

Summary by CodeRabbit

Bug Fixes
- Improved method-descriptor lookup and binding support for LRU cache wrappers, including when used with methods.

Match the Python 3.14 wrapper type flag and enable the existing guarded method-call fast path. Keep the partial vectorcall changes separate. Assisted-by: OpenAI Codex:model identifier unavailable

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID: af8ba231-70c8-4425-bef9-7cb049f1c650

📥 Commits

Reviewing files that changed from the base of the PR and between 9d6bf4d and c251a89.

📒 Files selected for processing (2)

crates/vm/src/stdlib/_functools.rs
extra_tests/snippets/stdlib_functools.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Walkthrough

PyLruCacheWrapper now declares the METHOD_DESCRIPTOR type flag. The functools test imports _lru_cache_wrapper and checks that flag bit 17 is set.

Changes

LRU Cache Method Descriptor Flag

Layer / File(s)	Summary
Add and check the method-descriptor flag crates/vm/src/stdlib/_functools.rs, extra_tests/snippets/stdlib_functools.py	PyLruCacheWrapper adds the METHOD_DESCRIPTOR type flag. The test imports _lru_cache_wrapper and asserts that flag bit 17 is set.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: youknowone, 1ndahous3

Merge Risk: ⚪ Minimal · up to c251a

The flag enables the wrapper’s existing method-descriptor behavior, with no concrete regression identified. The change presents no actionable merge-blocking risk.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: marking LRU cache wrappers as method descriptors.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		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 a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T14:37:09.834316Z	c251a89	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c251a89292

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Remove the forbidden test assertion

For this change, the added line modifies an existing snippet test by introducing a new assertion. The root AGENTS.md marks these restrictions as critical, forbids changing test assertions/logic/data, and limits permitted test-file edits to expected-failure decorator updates, so this assertion (and its supporting import/comment) must be removed from the patch.

AGENTS.md reference: AGENTS.md:L275-L279

Useful? React with 👍 / 👎.

Mark LRU cache wrappers as method descriptors

c251a89

Match the Python 3.14 wrapper type flag and enable the existing guarded method-call fast path. Keep the partial vectorcall changes separate. Assisted-by: OpenAI Codex:model identifier unavailable

youknowone approved these changes Oct 10, 2026

View reviewed changes

youknowone marked this pull request as ready for review October 10, 2026 14:33

youknowone enabled auto-merge (squash) October 10, 2026 14:33

chatgpt-codex-connector Bot reviewed Oct 10, 2026

View reviewed changes

Mark LRU cache wrappers as method descriptors by youknowdot · Pull Request #9072 · RustPython/RustPython · GitHub

Repository navigation

Mark LRU cache wrappers as method descriptors - #9072

Mark LRU cache wrappers as method descriptors#9072
youknowdot wants to merge 1 commit into
RustPython:mainfrom
youknowdot:fix-lru-cache-method-descriptor

youknowdot commented Oct 10, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 10, 2026 •
edited
Loading

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026 •
edited
Loading

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

               from testutils import assert_raises
+              # Cached wrappers support method-descriptor lookup and binding.
+              assert _lru_cache_wrapper.__flags__ & (1 << 17)

Mark LRU cache wrappers as method descriptors by youknowdot · Pull Request #9072 · RustPython/RustPython · GitHub

Repository navigation

Mark LRU cache wrappers as method descriptors - #9072

Mark LRU cache wrappers as method descriptors#9072youknowdot wants to merge 1 commit intoRustPython:mainRustPython/RustPython:mainfrom youknowdot:fix-lru-cache-method-descriptoryouknowdot/RustPython:fix-lru-cache-method-descriptorCopy head branch name to clipboard

Conversation

youknowdot commented Oct 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mark LRU cache wrappers as method descriptors#9072
youknowdot wants to merge 1 commit into
RustPython:mainfrom
youknowdot:fix-lru-cache-method-descriptor

youknowdot commented Oct 10, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Oct 10, 2026 •
edited
Loading

chatgpt-codex-connector Bot commented Oct 10, 2026 •
edited
Loading