GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — LoadClosure as pseudo op by youknowone · Pull Request #6789 · RustPython/RustPython

Conversation

@youknowone

@youknowone youknowone commented Jan 18, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Refactor
    • Optimized internal bytecode instruction handling for closure variable loading, improving the compiler's variable scope management and frame initialization during runtime.

✏️ Tip: You can customize this high-level summary in your review settings.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@coderabbitai

coderabbitai Bot commented Jan 18, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The PR migrates the LoadClosure opcode from the concrete Instruction enum to the PseudoInstruction enum, updating bytecode emission, IR lowering, decoding logic, instruction metadata, and VM frame execution paths to accommodate this structural change.

Changes

Cohort / File(s) Summary
Bytecode Definition and Conversion
crates/compiler-core/src/bytecode/instruction.rs		 Moves LoadClosure(Arg<NameIdx>) from Instruction to PseudoInstruction (discriminant 268); updates TryFrom bounds and removes LoadClosure handling from Instruction paths; removes stack effects and formatting for LoadClosure from Instruction, adds stack effect of 1 to PseudoInstruction.
Code Generation and IR Lowering
crates/codegen/src/compile.rs, crates/codegen/src/ir.rs		 Updates closure variable emission to use PseudoInstruction::LoadClosure instead of Instruction::LoadClosure; adds IR lowering to convert PseudoInstruction::LoadClosure to LOAD_FAST with index adjusted by varnames count.
Opcode Metadata
crates/stdlib/src/opcode.rs		 Removes LoadClosure(_) from the has_free instruction pattern match, no longer treating it as a free-variable instruction.
Frame Execution and Locals
crates/vm/src/frame.rs		 Restructures frame locals initialization to combine varnames, cellvars, and freevars into a single fastlocals array with pre-filled cell objects; removes the Instruction::LoadClosure execution path from frame dispatch.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • Bytecode instrumented placeholder #6741: Directly migrates the same LoadClosure opcode out of Instruction into PseudoInstruction with coordinated enum and handling updates.
  • LoadClassDeref -> LoadFromDictOrDeref #6692: Performs analogous opcode migration (e.g., LoadClassDeref) affecting bytecode enums, codegen, IR, and frame execution.
  • Pseudo ops #6678: Refactors the broader pseudo-instruction pipeline with similar opcode relocation and IR lowering patterns.

Suggested reviewers

  • ShaharNaveh

Poem

🐰 A closure hops from Instruction's gate,
To Pseudo's burrow, a lighter fate!
Fastlocals swell with cells in place,
No more dispatch—just stack-bound grace.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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 describes the main change: moving LoadClosure from a regular Instruction to a PseudoInstruction, which is the primary focus across all modified files.

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

✨ Finishing touches
  • 📝 Generate docstrings

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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

Comment @coderabbitai help to get the list of available commands and usage tips.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Jan 18, 2026

Copy link
Copy Markdown
Contributor

Code has been automatically formatted

The code in this PR has been formatted using:

  • python scripts/generate_opcode_metadata.py
    Please pull the latest changes before pushing again:
git pull origin LoadClosure
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@youknowone
youknowone force-pushed the LoadClosure branch from 59f2bde to 430ed08 Compare January 18, 2026 15:39
@youknowone
LoadClosure as pseudo op
8fb5bdb
@youknowone
youknowone force-pushed the LoadClosure branch from 5311565 to 8fb5bdb Compare January 18, 2026 16:20
@youknowone
youknowone marked this pull request as ready for review January 18, 2026 17:37
@youknowone
youknowone merged commit 130bb82 into RustPython:main Jan 18, 2026
13 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@youknowone
youknowone deleted the LoadClosure branch January 18, 2026 17:45
This was referenced Jan 29, 2026
More Load instructions #6886
Merged
Split stack_effect into pushed & popped #6893
Merged
Use stack_effect_info for getting pop&push count #6913
Merged
This was referenced Mar 2, 2026
Bytecode enum named oparg #7294
Merged
Remove cells_frees duplicate storage from Frame #7351
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Mar 25, 2026
Bytecode parity #7504
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@youknowone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.