Actionable comments posted: 1

crates/stdlib/src/multiprocessing.rs (1)

721-744: Extract the repeated wait/errno wrapper.

Lines 721-744 and 781-791 duplicate the same allow_threads + (res, errno) packaging. A tiny local helper would keep the platform branches focused on which wait primitive they call and reduce the chance of one path drifting from the others.

As per coding guidelines, "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code."

Also applies to: 781-791

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/multiprocessing.rs` around lines 721 - 744, The
sem_wait/sem_timedwait branches duplicate the same vm.allow_threads wrapper and
(r, errno) packaging; create a small local helper (e.g., run_wait_and_errno or a
closure) that takes a zero-argument FnOnce calling the desired wait primitive
(call sites: libc::sem_timedwait and libc::sem_wait) and returns the (r, Errno)
tuple after running it inside vm.allow_threads, then replace both duplicated
blocks with calls to that helper so only the differing wait call remains in each
branch.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/stdlib/src/multiprocessing.rs`:
- Around line 714-748: The initial assignment let mut saved_errno =
Errno::UnknownErrno is never read because every code path that uses saved_errno
assigns it first; remove the dead initialization and declare saved_errno as an
uninitialized variable that is assigned in the branches that set it (or use
Option<Errno>), i.e., change the declaration near saved_errno and ensure the
non-Apple loop (where vm.allow_threads calls calling
libc::sem_timedwait/sem_wait), and the Apple non-deadline loop assign
saved_errno before it is used in the subsequent match; reference saved_errno,
Errno::UnknownErrno, vm.allow_threads, libc::sem_timedwait, libc::sem_wait and
res when making the adjustment.

---

Nitpick comments:
In `@crates/stdlib/src/multiprocessing.rs`:
- Around line 721-744: The sem_wait/sem_timedwait branches duplicate the same
vm.allow_threads wrapper and (r, errno) packaging; create a small local helper
(e.g., run_wait_and_errno or a closure) that takes a zero-argument FnOnce
calling the desired wait primitive (call sites: libc::sem_timedwait and
libc::sem_wait) and returns the (r, Errno) tuple after running it inside
vm.allow_threads, then replace both duplicated blocks with calls to that helper
so only the differing wait call remains in each branch.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 87fae19d-e9e9-4f6a-a4fe-1c4ebf7a9693

Reviewing files that changed from the base of the PR and between 2266ba7 and bb5fa6b.

• crates/stdlib/src/multiprocessing.rs

crates/stdlib/src/multiprocessing.rs (1)

714-714: ⚠️ Potential issue | 🟡 Minor

Drop the dead saved_errno initializer.

Line 714 is overwritten on every path that can reach the later match, so the Errno::UnknownErrno assignment is never read and will keep clippy::unused_assignments noisy.

♻️ Proposed fix

-                let mut saved_errno = Errno::UnknownErrno;
+                let mut saved_errno = None;
@@
-                        saved_errno = e;
+                        saved_errno = Some(e);
@@
-                        if saved_errno == Errno::EINTR {
+                        if saved_errno == Some(Errno::EINTR) {
@@
-                            saved_errno = e;
+                            saved_errno = Some(e);
@@
-                            if saved_errno == Errno::EINTR {
+                            if saved_errno == Some(Errno::EINTR) {
@@
-                    match saved_errno {
+                    match saved_errno.expect("errno must be captured when sem_*wait fails") {

As per coding guidelines "Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints that are introduced by your changes".

Also applies to: 747-747, 752-752, 793-793, 797-797, 808-808

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/multiprocessing.rs` at line 714, The local variable
saved_errno is being assigned Errno::UnknownErrno but that value is always
overwritten before use; remove the dead initializer and declare the variable
with its type only (e.g., change "let mut saved_errno = Errno::UnknownErrno;" to
"let mut saved_errno: Errno;") so the initial assignment is not emitted and
clippy's unused_assignments warning is resolved; make the same change for the
other occurrences of saved_errno in multiprocessing.rs (the other mentioned
spots) so each is declared with a type but without the unused default value.

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@crates/stdlib/src/multiprocessing.rs`:
- Line 714: The local variable saved_errno is being assigned Errno::UnknownErrno
but that value is always overwritten before use; remove the dead initializer and
declare the variable with its type only (e.g., change "let mut saved_errno =
Errno::UnknownErrno;" to "let mut saved_errno: Errno;") so the initial
assignment is not emitted and clippy's unused_assignments warning is resolved;
make the same change for the other occurrences of saved_errno in
multiprocessing.rs (the other mentioned spots) so each is declared with a type
but without the unused default value.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: cbadff0b-b4e7-40e7-a2e2-3a77e7645a7b

Reviewing files that changed from the base of the PR and between bb5fa6b and cd304a5.

• crates/stdlib/src/multiprocessing.rs

crates/stdlib/src/multiprocessing.rs (1)

783-795: Consider extracting the duplicated sem_wait closure pattern.

The vm.allow_threads closure wrapping sem_wait with errno capture (lines 783-793) is identical to the non-Apple version (lines 733-744). This could be extracted into a helper function to reduce duplication:

♻️ Suggested helper extraction

fn sem_wait_with_errno(
    vm: &VirtualMachine,
    sem_ptr: *mut sem_t,
) -> (libc::c_int, Errno) {
    vm.allow_threads(|| {
        let r = unsafe { libc::sem_wait(sem_ptr) };
        (r, if r < 0 { Errno::last() } else { Errno::from_raw(0) })
    })
}

Then both paths would call:

let (r, e) = sem_wait_with_errno(vm, sem_ptr);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/multiprocessing.rs` around lines 783 - 795, Extract the
duplicated vm.allow_threads closure that calls libc::sem_wait and captures errno
into a helper function (e.g., fn sem_wait_with_errno(vm: &VirtualMachine,
sem_ptr: *mut sem_t) -> (libc::c_int, Errno)) and replace both occurrences with
let (r, e) = sem_wait_with_errno(vm, sem_ptr); so the logic inside
sem_wait_with_errno performs the unsafe libc::sem_wait(sem_ptr) call and returns
(r, if r < 0 { Errno::last() } else { Errno::from_raw(0) }) to eliminate the
duplicated closure in multiprocessing.rs while preserving VirtualMachine,
sem_ptr, and Errno behavior.

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@crates/stdlib/src/multiprocessing.rs`:
- Around line 783-795: Extract the duplicated vm.allow_threads closure that
calls libc::sem_wait and captures errno into a helper function (e.g., fn
sem_wait_with_errno(vm: &VirtualMachine, sem_ptr: *mut sem_t) -> (libc::c_int,
Errno)) and replace both occurrences with let (r, e) = sem_wait_with_errno(vm,
sem_ptr); so the logic inside sem_wait_with_errno performs the unsafe
libc::sem_wait(sem_ptr) call and returns (r, if r < 0 { Errno::last() } else {
Errno::from_raw(0) }) to eliminate the duplicated closure in multiprocessing.rs
while preserving VirtualMachine, sem_ptr, and Errno behavior.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 2bf61cbb-6a55-4009-b155-70cbf3eb6c08

Reviewing files that changed from the base of the PR and between cd304a5 and 8ec74ef.

• crates/stdlib/src/multiprocessing.rs
• crates/vm/src/builtins/tuple.rs

• crates/vm/src/builtins/tuple.rs