Actionable comments posted: 0

crates/vm/src/stdlib/os.rs (1)

1442-1450: LGTM! Windows implementation correctly extracts exit code.

The >> 8 shift and u32 validation match CPython's Windows behavior. The distinct signature (u64 input, u32 output) appropriately reflects Windows semantics where exit codes are always non-negative.

Nitpick: Minor inconsistency in error message capitalization—line 1439 uses "Invalid" while line 1449 uses "invalid". Consider aligning for consistency.

-        .map_err(|_| vm.new_value_error(format!("invalid exit code: {exitcode}")))
+        .map_err(|_| vm.new_value_error(format!("Invalid exit code: {exitcode}")))

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between a3d638a and c5b2b41.

• Lib/test/test_os.py is excluded by !Lib/**

• crates/vm/src/stdlib/os.rs (1 hunks)

📄 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 Rust 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/stdlib/os.rs

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.

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 rather than providing fallback values for other platforms.

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:

• crates/vm/src/stdlib/os.rs

crates/vm/src/stdlib/os.rs (1)

1424-1440: LGTM! Unix implementation correctly handles wait status.

The implementation properly uses libc macros (WIFEXITED/WEXITSTATUS/WIFSIGNALED/WTERMSIG) and correctly returns negative values for signal-terminated processes. The validation flow (i32 → u32 → c_int) ensures non-negative input while maintaining compatibility with libc macro expectations.

Based on learnings, this follows the recommended pattern of platform-specific conditional compilation.