crates/vm/src/stdlib/time.rs (3)

1350-1367: pyobj_to_time_t is duplicated verbatim across Unix and Windows platform modules.

The Unix (line 1020) and Windows (line 1350) implementations are identical. Consider extracting this into a shared #[cfg(any(unix, windows))] helper to reduce duplication.

---

1415-1419: get_tz_info() is duplicated between the decl module (line 202) and the platform module (line 1415).

Both are identical functions wrapping GetTimeZoneInformation. Consider making one call the other, or extracting it to a shared location.

---

1390-1396: current_time_t() uses .unwrap() which panics if the system clock is before UNIX_EPOCH.

This is extremely unlikely in practice, but the Unix counterpart (line 1064) uses libc::time() which doesn't panic. Consider using unwrap_or(0) or saturating to 0 for defensive consistency.

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

470-486: Potential panic on out-of-range tm_wday / tm_mon indices.

asctime_from_tm indexes arrays of size 7 and 12 using tm.tm_wday as usize and tm.tm_mon as usize with no bounds check. While callers typically validate these, if asctime_from_tm is ever reached with invalid values (e.g., via a future code path), it will panic with an index-out-of-bounds. Consider a defensive .get() or a debug assertion.

This is a minor concern since checked_tm_from_struct_time validates month in [0, 11] and wday >= 0 (plus % 7 caps it at 6), and localtime_from_timestamp delegates to the C library which produces valid values.

---

1408-1465: Significant code duplication with unix_mktime and checked_tm_from_struct_time.

The field extraction pattern (try_into_value + map_err(|_| invalid_tuple())) is repeated nearly identically in win_mktime (here), unix_mktime (Line 1080), and checked_tm_from_struct_time (Line 332). While the validation semantics differ (mktime normalizes, strftime validates), the extraction logic is the same.

Consider extracting a shared helper that converts StructTimeData fields into raw (year, mon, mday, hour, min, sec, wday, yday, isdst) tuple, then let each caller apply its own validation/adjustments. This would remove ~50 lines of duplicated code. 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."

crates/vm/src/stdlib/time.rs (3)

1424-1481: win_mktime and unix_mktime are near-identical; extract shared logic.

Both functions parse the same StructTimeData fields into a libc::tm, differing only in the final mktime call (libc::mktime vs c_mktime with suppress_iph!). 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."

Consider extracting the common StructTimeData → libc::tm parsing into a shared helper (e.g., tm_from_struct_time_for_mktime), then have each platform function call it and dispatch to its respective mktime.

Also applies to: 1096-1155

---

956-971: Lossy i64 → f64 → time_t round-trip for integer timestamps.

Line 965 converts i64 to f64, losing precision for values > 2⁵³. The subsequent secs as libc::time_t on line 970 then converts back, potentially yielding a different timestamp than the user passed in. For the integer case, you could short-circuit directly:

Suggested fix

     fn pyobj_to_time_t(value: Either<f64, i64>, vm: &VirtualMachine) -> PyResult<libc::time_t> {
-        let secs = match value {
+        match value {
             Either::A(float) => {
                 if !float.is_finite() {
                     return Err(vm.new_value_error("Invalid value for timestamp"));
                 }
-                float.floor()
+                let secs = float.floor();
+                if secs < libc::time_t::MIN as f64 || secs > libc::time_t::MAX as f64 {
+                    return Err(
+                        vm.new_overflow_error("timestamp out of range for platform time_t"),
+                    );
+                }
+                Ok(secs as libc::time_t)
             }
-            Either::B(int) => int as f64,
-        };
-        if secs < libc::time_t::MIN as f64 || secs > libc::time_t::MAX as f64 {
-            return Err(vm.new_overflow_error("timestamp out of range for platform time_t"));
+            Either::B(int) => Ok(int as libc::time_t),
         }
-        Ok(secs as libc::time_t)
     }

---

470-486: asctime_from_tm indexes arrays without bounds checks — panics on invalid tm_wday/tm_mon.

While current callers go through checked_tm_from_struct_time which validates ranges, asctime_from_tm itself is pub-accessible within the module and will panic with an index-out-of-bounds if given a tm with tm_wday >= 7 or tm_mon >= 12. A defensive .get().unwrap_or(...) or debug assertion would harden this against future misuse.

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

1408-1465: win_mktime largely duplicates unix_mktime — consider extracting shared logic.

The field extraction and error handling in win_mktime (lines 1408–1465) mirrors unix_mktime (lines 1080–1139) almost identically. As per coding guidelines, shared logic should be extracted to avoid duplication. The platform-specific parts (extra tm_gmtoff/tm_zone fields on Unix, suppress_iph! on Windows) could be handled in small platform-conditional blocks after a shared field-extraction helper.

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

940-997: Consider extracting shared field-extraction logic from tm_from_struct_time and checked_tm_from_struct_time.

Both functions independently extract the same ~8 fields from StructTimeData into libc::tm using the identical clone().try_into_value(vm).map_err(...) pattern. A shared helper that populates a libc::tm from StructTimeData (with a configurable error-message prefix) could be called by both, with checked_tm_from_struct_time adding its range validation on top.

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."