| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
📝 Walkthrough
WalkthroughRuntime execution methods were moved out of compile.rs into a new python_run.rs module (conditional on the compiler feature). Call sites in src/lib.rs were updated to use the new execution helpers and a new run_file/importer flow was added for script vs module handling. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / src/lib.rs
participant VM as VirtualMachine
participant FS as Filesystem
participant ImportLib as importlib (path hooks/cache)
participant Compiler as Compiler
Note over CLI,VM: CLI decides run_file or run_string
CLI->>VM: run_file(path) or run_string(source)
alt run_file -> directory with __main__.py
VM->>ImportLib: get_importer(path)
ImportLib->>VM: importer (maybe package)
VM->>FS: read "__main__.py"
FS-->>VM: source bytes
VM->>Compiler: compile source -> code object
Compiler-->>VM: code object
VM->>VM: set_main_loader(...) (set __loader__, __file__, __cached__)
VM->>VM: execute code object in module dict
VM->>CLI: return / propagate exceptions
else run_file -> simple file or run_string
VM->>FS: read file (or take provided source)
FS-->>VM: source bytes
VM->>Compiler: compile source -> code object
Compiler-->>VM: code object
VM->>VM: execute code object in scope (flush_io on finish)
VM->>CLI: return / propagate exceptions
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)crates/vm/src/vm/python_run.rs (1)📜 Review details115-118: Consider uncommenting or removing the deprecation notice.
The commented-out #[deprecated] attribute suggests this method may be deprecated in the future. Either uncomment it to signal intent to callers, or remove the comment entirely if deprecation is not planned.
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between fd27c2d and a1c5ec6.
📒 Files selected for processing (4)📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by running cargo fmt to 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:
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
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
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`
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
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:
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:
Learnt from: moreal Repo: RustPython/RustPython PR: 5847 File: vm/src/stdlib/stat.rs:547-567 Timestamp: 2025-06-27T14:47:28.810Z Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
Applied to files:
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:
crates/vm/src/vm/compile.rs (1)crates/vm/src/vm/mod.rs (1)1-9: LGTM!
Clean separation of concerns. The module now focuses solely on compilation, with a helpful doc comment directing users to python_run.rs for execution functions. The imports are appropriately minimal for the remaining functionality.
src/lib.rs (4)11-12: LGTM!
The new python_run module is correctly feature-gated with rustpython-compiler, consistent with the compile module. This ensures execution functionality is only available when compilation is enabled.
crates/vm/src/vm/python_run.rs (7)52-52: LGTM!
Import additions are appropriate for the new functionality.
189-218: LGTM!
The get_importer function correctly mirrors CPython's path importer resolution logic: checking the cache first, iterating through path_hooks, catching ImportError to try the next hook, and caching successful results.
277-292: LGTM!
The execution flow correctly uses the new APIs:
- run_string for -c command execution with the main scope
- run_file helper for script/directory execution with proper importer detection
366-381: LGTM!
Tests appropriately cover both execution paths: direct file execution via run_any_file and directory module execution via run_file.
1-8: LGTM!
Clear module purpose documented. Imports are appropriate for the execution functionality.
23-54: LGTM!
The run_simple_file implementation correctly:
- Tracks whether __file__ was set (for proper cleanup)
- Sets both __file__ and __cached__ attributes
- Flushes IO after execution regardless of result
- Cleans up attributes after execution (silently ignoring errors, matching CPython behavior)
56-90: LGTM!
The inner execution logic properly handles both .pyc files (via SourcelessFileLoader.get_code) and source files (via compilation). The <stdin> special case correctly skips loader setup, and error mapping is appropriate.
92-113: LGTM!
Clean API design following CPython naming conventions (PyRun_SimpleString, PyRun_String). The deprecated alias maintains backward compatibility.
127-137: LGTM!
Correctly flushes both stdout and stderr, silently ignoring errors to match CPython's cleanup behavior.
140-154: LGTM!
The set_main_loader function correctly sets up the appropriate loader (SourceFileLoader or SourcelessFileLoader) for the __main__ module, matching CPython's behavior.
156-186: LGTM!
The pyc detection is well-implemented:
- Fast path via extension check
- Fallback to magic number comparison (only first 2 bytes, as documented)
- Handles edge cases: directories, missing files, and short files
Sorry, something went wrong.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)src/lib.rs (2)📜 Review details168-174: Prefer is_some() over unused pattern binding.
The _importer variable is bound but never used. Consider a more idiomatic check:
🔎 Proposed refactoring- if let Some(_importer) = get_importer(path, vm)? { + if get_importer(path, vm)?.is_some() { vm.insert_sys_path(vm.new_pyobj(path))?; let runpy = vm.import("runpy", 0)?; let run_module_as_main = runpy.get_attr("_run_module_as_main", vm)?; run_module_as_main.call((vm::identifier!(vm, __main__).to_owned(), false), vm)?; return Ok(()); }
172-172: Add comment explaining the false parameter.
The second parameter to _run_module_as_main (which corresponds to alter_sys in Python's runpy module) lacks explanation. A brief comment would improve readability.
🔎 Proposed addition+ // alter_sys=false: don't modify sys.argv for module execution run_module_as_main.call((vm::identifier!(vm, __main__).to_owned(), false), vm)?;
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between a1c5ec6 and 50098de.
📒 Files selected for processing (1)📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by running cargo fmt to 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:
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
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
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
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`
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 : Follow Rust best practices for error handling and memory management
Applied to files:
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:
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:
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/stdlib/sys.rs (4)⏰ 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)
- path (520-528)
- std (900-900)
- path_importer_cache (536-538)
- path_hooks (531-533)
src/lib.rs (8)52-52: LGTM: Imports align with new functionality.
The added imports support the new execution helpers and updated VM APIs.
147-147: LGTM: API update applied correctly.
The change from run_code_string to run_string is consistent with the new VM API.
176-183: Past review concern addressed.
The code now uses .and_then(|p| p.to_str()).unwrap_or("") which safely handles edge cases where parent() returns None (root paths) or to_str() fails (non-UTF-8 paths). This matches the previously suggested fix.
256-256: LGTM: API update applied correctly.
The change from run_code_string to run_simple_string is appropriate for this simple one-liner initialization.
279-279: LGTM: API update applied correctly.
The change to run_string is consistent with the new VM API.
287-290: LGTM: Script execution now uses the new helper.
The delegation to run_file properly handles both package/directory and regular file execution, matching CPython's pymain_run_file_obj behavior.
371-375: LGTM: Tests validate both execution modes.
The tests appropriately verify direct file execution and directory module execution with __main__.py, covering the new run_file functionality.
188-217: sys.path_hooks is already properly initialized during VM startup.
The code does not rely on site.py (which doesn't exist in this repository) to populate path_hooks. Instead, initialization happens automatically in init_importlib_package during VM construction, where importlib._install_external_importers() is called and zipimporter is registered as the first path hook. This occurs before any user code—including get_importer—is executed. Additionally, get_importer safely handles empty path hooks by falling back to vm.run_any_file.
Likely an incorrect or invalid review comment.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary by CodeRabbit
Refactor
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.