Materialize code constants through the VM by youknowone · Pull Request #8867 · RustPython/RustPython · GitHub

This PR follows our AI policy

Summary

PyObjBag built code object constants from a Context alone. Its frozenset arm was unimplemented!(), so ctx.new_code() panicked on any code containing a frozenset constant, e.g. x in {1, 2, 3} or for e in {1, 2, 3}. This affected embedders passing compiled or py_freeze! code to ctx.new_code, as examples/freeze and examples/mini_repl do.

Every caller already has a VirtualMachine, and PyVmBag already builds all constants. This PR routes code object construction through the VM:

IntoCodeObject::into_code_object takes &VirtualMachine and uses PyVmBag for bytecode::CodeObject and FrozenCodeObject.
Context::new_code, PyObjBag and AsBag for &Context are removed. VirtualMachine::new_code replaces them.

API change: embedders replace vm.ctx.new_code(code) with vm.new_code(code).

Alternative to #8823, which builds frozensets without a VM.

Tests

New test vm::interpreter::tests::new_code_materializes_frozenset_constants.
cargo build --examples --features freeze-stdlib.
-m test test_compile test_code test_frozen test_importlib test_marshal test_dis passes.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Code objects now correctly materialize frozenset constants, supporting set membership checks and iteration in compiled code. Code object creation is also handled consistently across the runtime and related tools, improving compatibility when compiling and executing code through different entry points.

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: 8fa2a349-3005-46b8-8f61-0e678257f73e

📥 Commits

Reviewing files that changed from the base of the PR and between 1736c0d and d2e0200.

📒 Files selected for processing (4)

crates/vm/src/builtins/code.rs
crates/vm/src/stdlib/_testinternalcapi.rs
crates/vm/src/vm/context.rs
crates/vm/src/vm/interpreter.rs

💤 Files with no reviewable changes (1)

crates/vm/src/vm/context.rs

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

📝 Walkthrough

Walkthrough

Code-object conversion now uses the VM to handle bytecode and frozen-code constants. Code-object construction moves from Context to VirtualMachine, and existing callers are updated. A compiler-feature-gated test checks set constants in membership and iteration.

Changes

Code-object creation

Layer / File(s)	Summary
VM-backed constant conversion crates/vm/src/builtins/code.rs	IntoCodeObject now accepts a VirtualMachine and converts bytecode and frozen constants through PyVmBag. The Context-backed PyObjBag conversions are removed.
VM code-object API and caller migration crates/vm/src/vm/vm_new.rs, crates/vm/src/vm/context.rs, crates/vm/src/builtins/code.rs, crates/vm/src/stdlib/_testinternalcapi.rs, crates/vm/src/vm/interpreter.rs, examples/freeze/main.rs, examples/mini_repl.rs, src/lib.rs	VirtualMachine::new_code replaces Context::new_code, and existing callers use the VM method. A compiler-feature-gated test checks set membership and iteration constants.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to d2e02

No actionable issue remains in the supplied review evidence; this change is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d2e02

The public embedding API requires migration, but inspected construction paths remain separate from code execution. No introduced security weakness was established; external consumers and malformed-input failure containment remain incompletely assessed.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The demonstrated exposure is the embedding process constructing bytecode or frozen-code objects. FrozenCodeObject permits caller-supplied bytes, but the inspected scope does not establish remote reachability, tenant isolation, or panic containment at external embedding boundaries.

Trust Boundaries and Controls

observed — Code construction and execution remain separate operations. The new helper converts constants and allocates PyCode; the regression test invokes execution only after construction returns.
observed — Frozen decoding retains panic-based handling for decompression or deserialization errors, and PyVmBag unwraps frozenset construction errors. These are not newly introduced implementations: the old Context route also panicked on frozensets, and VM-backed constructors already used PyVmBag. The comparison does not establish a worsened security condition.

Resilience and Maintainability Implications

observed — Frozensets are built locally before publication, and the outer PyCode is published only after conversion succeeds. Interning is synchronized and resolves duplicate insertions to the existing identity, but interned strings are permanently retained rather than transactionally rolled back after failed construction.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: code constants are materialized through the VirtualMachine.

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.

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1

Comparing youknowone:frozenset-const-vm (1736c0d) with main (456a8dc)

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

PyObjBag built code object constants from a Context alone, and its frozenset arm was unimplemented!(), so ctx.new_code() panicked on code containing a frozenset constant such as `x in {1, 2, 3}`. IntoCodeObject now takes a VirtualMachine and uses PyVmBag for bytecode and frozen code objects. Context::new_code, PyObjBag and AsBag for &Context are removed; callers use VirtualMachine::new_code. Assisted-by: Claude:claude-opus-5-5 Assisted-by: Grok:grok-4.7

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

youknowone marked this pull request as draft September 27, 2026 17:11

youknowone force-pushed the frozenset-const-vm branch from 1736c0d to d2e0200 Compare October 2, 2026 05:39

youknowone marked this pull request as ready for review October 2, 2026 05:43

youknowone mentioned this pull request Oct 2, 2026

Support frozenset literal constants without a VirtualMachine #8823

Closed

1 task done

youknowone merged commit c8d63e8 into RustPython:main Oct 2, 2026
29 checks passed

youknowone deleted the frozenset-const-vm branch October 2, 2026 08:24

Materialize code constants through the VM by youknowone · Pull Request #8867 · RustPython/RustPython · GitHub

Repository navigation

Materialize code constants through the VM - #8867

Materialize code constants through the VM#8867
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:frozenset-const-vm

youknowone commented Sep 27, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Sep 27, 2026 •
edited
Loading

Uh oh!

Uh oh!

codspeed Bot commented Sep 27, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Materialize code constants through the VM by youknowone · Pull Request #8867 · RustPython/RustPython · GitHub

Repository navigation

Materialize code constants through the VM - #8867

Materialize code constants through the VM#8867youknowone merged 1 commit intoRustPython:mainRustPython/RustPython:mainfrom youknowone:frozenset-const-vmyouknowone/RustPython:frozenset-const-vmCopy head branch name to clipboard

Conversation

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

Uh oh!

Summary

Tests

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

codspeed Bot commented Sep 27, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

chatgpt-codex-connector Bot commented Oct 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Materialize code constants through the VM#8867
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:frozenset-const-vm

youknowone commented Sep 27, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Sep 27, 2026 •
edited
Loading