Actionable comments posted: 0

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

23-44: Confirm libc::strerror semantics on Windows targets; consider centralizing helper

• libc::strerror’s thread-safety and returned “Unknown error” text can differ by CRT/toolchain/locale; please confirm this behaves as intended on the Windows targets RustPython supports (e.g., MSVC/UCRT vs MinGW) and that the "Unknown error" guard is robust enough.
• There’s already a strerror(e: i32) -> String helper in crates/vm/src/stdlib/os.rs:1542-1546 (per provided snippet); consider reusing/relocating a shared helper to avoid duplicating FFI + string handling logic.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between c11a72a and d44baf9.

• Lib/test/test_argparse.py is excluded by !Lib/**
• Lib/test/test_py_compile.py is excluded by !Lib/**

• crates/vm/src/stdlib/io.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/io.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6358
File: crates/vm/src/exception_group.rs:173-185
Timestamp: 2025-12-09T08:46:58.660Z
Learning: In crates/vm/src/exception_group.rs, the derive() method intentionally always creates a BaseExceptionGroup instance rather than preserving the original exception class type. This is a deliberate design decision that differs from CPython's behavior.

Applied to files:

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

crates/vm/src/exceptions.rs (2)

• errno (1450-1451)
• errno (1452-1452)

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

• strerror (1543-1547)
• std (1207-1207)

• GitHub Check: Run tests under miri
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Run rust tests (windows-latest)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Check the WASM package and demo

Actionable comments posted: 0

crates/vm/src/stdlib/io.rs (2)

23-41: Consider extracting strerror logic to a shared helper.

The Windows block correctly handles POSIX errno translation with appropriate bounds checking and fallback logic. However, libc::strerror is not thread-safe—it returns a pointer to a static buffer that can be overwritten by concurrent calls. This matches the existing pattern in os.rs (lines 1542-1546), so it's consistent with the codebase.

Consider consolidating the strerror logic with the existing strerror helper in crates/vm/src/stdlib/os.rs to reduce duplication and centralize any future thread-safety improvements.

---

42-52: Unix strerror logic duplicates existing helper in os.rs.

This block is nearly identical to the strerror function at crates/vm/src/stdlib/os.rs:1542-1546. Consider reusing that function to avoid duplication:

 #[cfg(unix)]
 let msg = {
-    let ptr = unsafe { libc::strerror(errno) };
-    if !ptr.is_null() {
-        unsafe { std::ffi::CStr::from_ptr(ptr) }
-            .to_string_lossy()
-            .into_owned()
-    } else {
-        self.to_string()
-    }
+    // Reuse existing strerror helper from os module
+    crate::stdlib::os::strerror(errno)
 };

Note: This requires making the strerror function in os.rs public or moving it to a shared location.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 7c59654 and b093b19.

• Lib/test/test_argparse.py is excluded by !Lib/**
• Lib/test/test_py_compile.py is excluded by !Lib/**

• crates/vm/src/stdlib/io.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/io.rs

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6358
File: crates/vm/src/exception_group.rs:173-185
Timestamp: 2025-12-09T08:46:58.660Z
Learning: In crates/vm/src/exception_group.rs, the derive() method intentionally always creates a BaseExceptionGroup instance rather than preserving the original exception class type. This is a deliberate design decision that differs from CPython's behavior.

Applied to files:

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

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

• strerror (1543-1547)
• std (1207-1207)

• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Check the WASM package and demo
• GitHub Check: Run tests under miri
• GitHub Check: Run snippets and cpython tests on wasm-wasi

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

20-55: Implementation correctly handles platform-specific error messages.

The overall approach is sound: using C runtime strerror for better POSIX compatibility on Windows, with appropriate null checks and fallbacks. The labeled block pattern for early return is idiomatic Rust.