Skip to content

Avoid duplicate AST initialization and lazy-import locals allocation - #9183

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix/ast-initialization-lazy-locals-315
Oct 18, 2026
Merged

youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix/ast-initialization-lazy-locals-315

Conversation

@youknowdot

Copy link
Copy Markdown
Contributor

Summary

Follow up on the two review findings in #9179 after its merge.

  • Let PyType::call invoke the AST initializer once. NodeAst::slot_new only allocates, matching CPython's PyType_GenericNew. This also preserves custom subclass initializers that deliberately omit super().__init__() and allocation-only direct __new__ calls.
  • Check existing frame-locals identity without materializing a fresh locals dictionary during ordinary eager imports. Keep the module-level check before the __lazy_modules__ lookup.

Only two Rust files change. No library copies, test expectations, or skip markers change.

Validation

Fresh release binary built from this follow-up on main 9f6f7719.

  • Eight bounded observations exactly match pinned CPython 3.15 d220409: abstract warning count, inherited field writes, custom/delegating initialization, and direct __new__ allocation.
  • Full canonical test_ast: 220 total, 205 pass, 9 skip, 6 existing expected failures.
  • Full canonical test_lazy_import: 161 total, 157 pass, 4 skip.
  • Corresponding pinned-CPython oracle suites pass.
  • Fresh workspace native tests: 1,373 pass across 22 targets.
  • Fresh C-API native tests: 123 pass, 4 existing ignored tests.
  • Strict workspace and C-API Clippy pass with -D warnings.

The existing missing-C-API skips and AST expected failures remain visible; these results do not claim complete AST or compiler parity.

Let type.__call__ own AST initialization, matching CPython generic allocation, and inspect existing frame locals without materializing them for lazy policy checks.

Assisted-by: OpenAI Codex:model version unavailable
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a1be83f1-c452-451f-abac-9a09dba37f2f
📥 Commits

Reviewing files that changed from the base of the PR and between 9f6f771 and ad543f2.

📒 Files selected for processing: 2
  • crates/vm/src/lazy_import.rs
  • crates/vm/src/stdlib/_ast/python.rs
+--------------------------------------------+
| 99 little bugs in the code... I filed 100. |
+--------------------------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ 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.

@youknowone
youknowone marked this pull request as ready for review October 18, 2026 13:34
@youknowone
youknowone merged commit 18c1358 into RustPython:main Oct 18, 2026
21 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

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-18T13:37:48.860807Z ad543f2 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.

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