vm.run_pyc_bytes #6645
vm.run_pyc_bytes #6645
Conversation
📝 WalkthroughWalkthroughThis PR adds APIs and helpers to load and execute .pyc bytecode from memory or files: new PyCode constructors to build code objects from pyc bytes/paths, a VM API to run pyc bytes, a centralized pyc magic-number check, and refactors main-scope/module initialization to a VM-provided helper. Changes
Sequence DiagramsequenceDiagram
autonumber
actor Client
participant VM as VirtualMachine
participant Scope
participant MainMod as __main__ Module
participant PyCode
participant FrozenImportlib as frozen_importlib._compile_bytecode
Client->>VM: run_pyc_bytes(pyc_bytes, scope)
activate VM
VM->>VM: with_simple_run("__main__", closure)
VM->>Scope: ensure scope / builtins (new_scope_with_main)
VM->>MainMod: create/register __main__, init __annotations__
Note right of VM: Inject __file__ / __cached__ into MainMod
VM->>PyCode: from_pyc(pyc_bytes, metadata, vm)
activate PyCode
PyCode->>PyCode: check_pyc_magic_number_bytes(pyc_bytes)
PyCode->>FrozenImportlib: load & call _compile_bytecode(bytes, metadata)
FrozenImportlib-->>PyCode: code object
deactivate PyCode
PyCode-->>VM: return PyCode
VM->>MainMod: execute code object in __main__ dict
Note right of VM: flush IO / cleanup injected state
deactivate VM
VM-->>Client: PyResult<()>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
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. Comment |
0b1cd18 to
5e90531
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/vm/src/vm/vm_new.rs (1)
65-80: Consider propagating error instead of panicking on__annotations__initialization.Line 71 uses
.expect()which will panic if setting__annotations__fails, while the rest of the method uses?for error propagation. For consistency and robustness, consider using?here as well.🔎 Proposed fix
pub fn new_scope_with_main(&self) -> PyResult<Scope> { let scope = self.new_scope_with_builtins(); let main_module = self.new_module("__main__", scope.globals.clone(), None); main_module .dict() - .set_item("__annotations__", self.ctx.new_dict().into(), self) - .expect("Failed to initialize __main__.__annotations__"); + .set_item("__annotations__", self.ctx.new_dict().into(), self)?; self.sys_module.get_attr("modules", self)?.set_item( "__main__", main_module.into(), self, )?; Ok(scope) }crates/vm/src/builtins/code.rs (1)
339-352: Consider using actual source path instead of hardcoded"<source>".The
source_pathis hardcoded to"<source>"which may be confusing in tracebacks. Consider deriving it from the path or allowing it to beNone.🔎 Proposed fix
Self::from_pyc( &content, Some(&name), Some(&path.display().to_string()), - Some("<source>"), + None, // Let the loader determine the source path vm, )
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
crates/vm/src/builtins/code.rscrates/vm/src/import.rscrates/vm/src/vm/mod.rscrates/vm/src/vm/python_run.rscrates/vm/src/vm/vm_new.rssrc/lib.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/vm/src/import.rssrc/lib.rscrates/vm/src/vm/vm_new.rscrates/vm/src/vm/mod.rscrates/vm/src/vm/python_run.rscrates/vm/src/builtins/code.rs
🧠 Learnings (4)
📚 Learning: 2025-12-27T14:03:49.034Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: When testing Python code, always use RustPython instead of the standard `python` command; use `cargo run -- script.py` or `cargo run` for interactive REPL
Applied to files:
src/lib.rscrates/vm/src/vm/mod.rscrates/vm/src/vm/python_run.rs
📚 Learning: 2025-12-27T14:03:49.034Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: When modifying bytecode instructions, perform a full clean build by running `rm -r target/debug/build/rustpython-* && find . | grep -E '\.pyc$' | xargs rm -r`
Applied to files:
crates/vm/src/vm/mod.rscrates/vm/src/vm/python_run.rs
📚 Learning: 2025-12-27T14:03:49.034Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: RustPython is a Python 3 interpreter written in Rust implementing Python 3.13.0+ compatibility with complete Python environment in Rust (not CPython bindings), clean implementation without compatibility hacks, and cross-platform support including WebAssembly compilation
Applied to files:
crates/vm/src/vm/mod.rs
📚 Learning: 2025-12-27T14:03:49.034Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
Applied to files:
crates/vm/src/vm/python_run.rs
🧬 Code graph analysis (3)
crates/vm/src/vm/mod.rs (1)
crates/vm/src/builtins/code.rs (1)
from_pyc(353-370)
crates/vm/src/vm/python_run.rs (2)
crates/vm/src/vm/mod.rs (1)
import(689-693)crates/vm/src/import.rs (1)
check_pyc_magic_number_bytes(11-13)
crates/vm/src/builtins/code.rs (1)
crates/vm/src/import.rs (1)
check_pyc_magic_number_bytes(11-13)
🔇 Additional comments (7)
src/lib.rs (2)
209-209: LGTM!The refactoring to use
vm.new_scope_with_main()centralizes the main module initialization logic and improves maintainability.
354-360: LGTM!Test code correctly updated to use the new
new_scope_with_main()API.crates/vm/src/vm/mod.rs (3)
463-497: Well-documented API with clear usage example.The
run_pyc_bytesimplementation correctly:
- Provides clear documentation with usage examples
- Delegates to
PyCode::from_pycfor loading- Uses
with_simple_runfor proper main module setupOne minor note: the path
"<source>"passed towith_simple_runis generic. Consider allowing callers to specify a more descriptive path in a future enhancement.
539-571: LGTM!The
with_simple_runhelper properly:
- Sets
__file__and__cached__only if not already set- Cleans up these attributes after execution
- Flushes IO streams
573-583: LGTM!The
flush_iohelper appropriately ignores errors during cleanup, which is the correct behavior for post-execution flushing.crates/vm/src/vm/python_run.rs (2)
27-31: LGTM!The refactoring to use
with_simple_runproperly centralizes the__file__and__cached__management, reducing code duplication.
133-150: LGTM!The magic number validation is now properly centralized through
check_pyc_magic_number_bytes. The function correctly validates that exactly 2 bytes were read before checking the magic.
ecfaed3 to
8634a91
Compare
* rustpython_vm::import::check_pyc_magic_number_bytes * vm.new_scope_with_main * PyCode::from_pyc * vm.run_pyc_bytes * add boundary check
* rustpython_vm::import::check_pyc_magic_number_bytes * vm.new_scope_with_main * PyCode::from_pyc * vm.run_pyc_bytes * add boundary check
* rustpython_vm::import::check_pyc_magic_number_bytes * vm.new_scope_with_main * PyCode::from_pyc * vm.run_pyc_bytes * add boundary check
Fix #6239
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.