Resolve class attribute docs at macro time and drop rustpython-doc from vm by youknowone · Pull Request #8830 · RustPython/RustPython

youknowone · 2026-09-26T07:38:20Z

Follow-up to #8815.

Doc lookup moves out of `rustpython-vm`

#[pyclass] and #[pystruct_sequence] now emit a sorted PyClassDef::ATTR_DOCS table. The table is built from the doc database while the macro expands.
Attribute docs and SLOT_DOCS look up only that small per-class table. No const context searches the full database any more.
rustpython-vm no longer depends on rustpython-doc. You can check this with cargo tree -p rustpython-vm -e normal,no-proc-macro.
The hardcoded class signature docs in class.rs for object and _operator.attrgetter/itemgetter/methodcaller are removed. Their constructor Args now declare FromArgs::PARAMS, so the class signature comes from the normal path. Parsing and error messages are unchanged, except for one fix: methodcaller() with no arguments now raises methodcaller needs at least one argument, the method name.

Smaller cleanups

default = 0 and default = false are replaced with bare default. A bare default on integer and bool fields already renders the zero value in the text signature.
Context::new_str("") returns the empty_str singleton, so a default = "" field yields the interned empty string.

__doc__ and __text_signature__ for every native callable are unchanged: the whats_left --doc and --signature JSON dumps are identical before and after.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Class attribute documentation is now available through generated class definitions, with documentation lookups handling module aliases and qualified names.
Bug Fixes
- Argument defaults are handled consistently across built-in modules and functions.
- The fcntl.lockf start position must now be provided instead of defaulting to zero.
- Empty strings reuse a shared instance, without changing their visible value or behavior.

…om vm - #[pyclass] and #[pystruct_sequence] emit a sorted PyClassDef::ATTR_DOCS table from the doc database; attribute and slot docs look up that table. - rustpython-vm no longer depends on rustpython-doc. - _operator.attrgetter, itemgetter, methodcaller and object get constructor Args with FromArgs::PARAMS, replacing the hardcoded class signature docs in class.rs. - methodcaller() with no arguments raises "methodcaller needs at least one argument, the method name". Assisted-by: Claude:claude-opus-5-5 Assisted-by: Grok:grok-4.7

Bare `default` on integer and bool fields already renders the zero value in the text signature, so `default = 0` and `default = false` are replaced with bare `default`. Assisted-by: Claude:claude-opus-5-5 Assisted-by: Grok:grok-4.7

Assisted-by: Claude:claude-opus-5-5

chatgpt-codex-connector · 2026-09-26T07:38:24Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

coderabbitai · 2026-09-26T07:38:51Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 48b3d7f5-468c-4dde-8107-7bef28a18f68

📥 Commits

Reviewing files that changed from the base of the PR and between 7f314dd and f92d63e.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (44)

crates/derive-impl/src/class_docs.rs
crates/derive-impl/src/lib.rs
crates/derive-impl/src/pyclass.rs
crates/derive-impl/src/pystructseq.rs
crates/stdlib/src/binascii.rs
crates/stdlib/src/blake2.rs
crates/stdlib/src/elementtree.rs
crates/stdlib/src/faulthandler.rs
crates/stdlib/src/fcntl.rs
crates/stdlib/src/hashlib.rs
crates/stdlib/src/mmap.rs
crates/stdlib/src/openssl.rs
crates/stdlib/src/pystruct.rs
crates/stdlib/src/select.rs
crates/stdlib/src/socket.rs
crates/stdlib/src/ssl.rs
crates/stdlib/src/syslog.rs
crates/vm/Cargo.toml
crates/vm/src/anystr.rs
crates/vm/src/builtins/bytearray.rs
crates/vm/src/builtins/list.rs
crates/vm/src/builtins/map.rs
crates/vm/src/builtins/memory.rs
crates/vm/src/builtins/object.rs
crates/vm/src/builtins/zip.rs
crates/vm/src/class.rs
crates/vm/src/lib.rs
crates/vm/src/stdlib/_codecs.rs
crates/vm/src/stdlib/_ctypes.rs
crates/vm/src/stdlib/_imp.rs
crates/vm/src/stdlib/_io.rs
crates/vm/src/stdlib/_operator.rs
crates/vm/src/stdlib/_sre.rs
crates/vm/src/stdlib/_winapi.rs
crates/vm/src/stdlib/builtins.rs
crates/vm/src/stdlib/itertools.rs
crates/vm/src/stdlib/os.rs
crates/vm/src/stdlib/posix.rs
crates/vm/src/stdlib/posix_compat.rs
crates/vm/src/stdlib/sys.rs
crates/vm/src/stdlib/winreg.rs
crates/vm/src/stdlib/winsound.rs
crates/vm/src/vm/context.rs
crates/wasm/src/ssl.rs

💤 Files with no reviewable changes (2)

crates/vm/Cargo.toml
crates/vm/src/lib.rs

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

📝 Walkthrough

Walkthrough

The change adds generated class attribute-documentation tables and uses them for slot documentation lookup. It also updates argument parsing declarations across the standard library and VM, adds constructor argument wrappers, and returns a shared empty-string singleton for empty input.

Changes

Class attribute documentation

Layer / File(s)	Summary
Generate and use attribute documentation tables `crates/derive-impl/src/class_docs.rs`, `crates/derive-impl/src/pyclass.rs`, `crates/derive-impl/src/pystructseq.rs`, `crates/vm/src/class.rs`, `crates/derive-impl/src/lib.rs`, `crates/vm/Cargo.toml`, `crates/vm/src/lib.rs`	Derive macros collect attribute docs using module aliases and emit an `ATTR_DOCS` table. `PyClassImpl` uses the table for slot documentation. The VM removes its `rustpython_doc` dependency and re-export.

Argument parsing updates

Layer / File(s)	Summary
Update argument default declarations `crates/stdlib/src/`, `crates/vm/src/anystr.rs`, `crates/vm/src/builtins/`, `crates/vm/src/stdlib/*`, `crates/wasm/src/ssl.rs`	Many argument declarations replace explicit zero or false defaults with shorthand defaults. `LockfArgs.start` becomes required. SSL argument defaults are expressed with shorthand syntax.
Add explicit constructor argument wrappers `crates/vm/src/builtins/object.rs`, `crates/vm/src/stdlib/_operator.rs`	`PyBaseObject`, `attrgetter`, `itemgetter`, and `methodcaller` use dedicated argument types for parsing.

Empty string interning

Layer / File(s)	Summary
Return the empty-string singleton `crates/vm/src/vm/context.rs`	`Context::new_str` returns `empty_str` when the input is empty.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant rustpython_doc
  participant attr_docs_tokens
  participant PyClassDef
  participant SLOT_DOCS
  rustpython_doc->>attr_docs_tokens: supplies documentation entries
  attr_docs_tokens->>PyClassDef: emits ATTR_DOCS table
  PyClassDef->>SLOT_DOCS: provides attribute docs for slot lookup

Suggested reviewers: shaharnaveh, leehanjeong

Merge Risk: ⚪ Minimal · up to f92d6

No merge-blocking behavior change was found; the PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f92d6

The documentation change appears confined to generated, immutable metadata, and the inspected file-locking arguments retain their prior defaults. No new privilege path was identified. The broad callable surface and incomplete coverage leave some uncertainty.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The visible effects are within generated class metadata and in-process Python-callable argument handling; the supplied impact map establishes no cross-service edge.

Trust Boundaries and Controls

observed — The inspected lockf entrypoint converts arguments before calling the host file-lock operation; its length, start, and whence declarations all retain zero defaults.

Resilience and Maintainability Implications

inferred — The argument-wrapper change does not introduce a new Rust-side lock-ownership lifecycle in the inspected path; host-specific behavior after an interrupted lock syscall is not established by this source evidence.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 42 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 summarizes the primary changes: moving class attribute documentation resolution to macro expansion and removing the vm dependency on rustpython-doc.
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 a new PR

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.}

youknowone added 3 commits September 26, 2026 13:05

Return the empty str singleton from Context::new_str

f92d63e

Assisted-by: Claude:claude-opus-5-5

youknowone merged commit 00dfed8 into RustPython:main Sep 26, 2026
30 checks passed

youknowone deleted the whats-left-signatures branch September 26, 2026 12:59

youknowone mentioned this pull request Sep 26, 2026

Store database docstrings xz-compressed behind an optional doc feature #8836

Merged

Navigation Menu

Repository navigation

Resolve class attribute docs at macro time and drop rustpython-doc from vm - #8830

Resolve class attribute docs at macro time and drop rustpython-doc from vm#8830
youknowone merged 3 commits into
RustPython:mainfrom
youknowone:whats-left-signatures

youknowone commented Sep 26, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

chatgpt-codex-connector Bot commented Sep 26, 2026

Uh oh!

coderabbitai Bot commented Sep 26, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

youknowone commented Sep 26, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Doc lookup moves out of rustpython-vm

Smaller cleanups

Summary by CodeRabbit

Uh oh!

chatgpt-codex-connector Bot commented Sep 26, 2026

Uh oh!

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

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Sep 26, 2026 •

edited by coderabbitai Bot

Loading

Doc lookup moves out of `rustpython-vm`

coderabbitai Bot commented Sep 26, 2026 •

edited

Loading