From e7800603cf72cd4f4e8369364dafc9816cb0a57e Mon Sep 17 00:00:00 2001 From: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Tue, 30 Jun 2026 17:34:08 +0300 Subject: [PATCH] clippy `iter_over_hash_type` (#8182) --- Cargo.toml | 1 + crates/derive-impl/src/pyclass.rs | 12 ++++++++++++ crates/stdlib/src/faulthandler.rs | 5 +++++ crates/vm/src/builtins/frame.rs | 4 ++++ crates/vm/src/gc_state.rs | 14 ++++++++++++++ crates/vm/src/vm/mod.rs | 25 +++++++++++++++++++++++++ 6 files changed, 61 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 4781c95e1..756d9ec13 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -350,6 +350,7 @@ similar_names = "allow" # restriction lints alloc_instead_of_core = "warn" cfg_not_test = "warn" +iter_over_hash_type = "warn" redundant_test_prefix = "warn" std_instead_of_alloc = "warn" std_instead_of_core = "warn" diff --git a/crates/derive-impl/src/pyclass.rs b/crates/derive-impl/src/pyclass.rs index d5bae7eeb..aed2c5d8c 100644 --- a/crates/derive-impl/src/pyclass.rs +++ b/crates/derive-impl/src/pyclass.rs @@ -1433,6 +1433,11 @@ impl GetSetNursery { fn validate(&mut self) -> Result<()> { let mut errors = Vec::new(); + + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for ((name, _cfgs), (getter, setter)) in &self.map { if getter.is_none() { errors.push(err_span!( @@ -1442,6 +1447,7 @@ impl GetSetNursery { )); }; } + errors.into_result()?; self.validated = true; Ok(()) @@ -1525,6 +1531,11 @@ impl MemberNursery { fn validate(&mut self) -> Result<()> { let mut errors = Vec::new(); + + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for (name, entry) in &self.map { if entry.getter.is_none() { errors.push(err_span!( @@ -1534,6 +1545,7 @@ impl MemberNursery { )); }; } + errors.into_result()?; self.validated = true; Ok(()) diff --git a/crates/stdlib/src/faulthandler.rs b/crates/stdlib/src/faulthandler.rs index b3af501d8..e890cecfa 100644 --- a/crates/stdlib/src/faulthandler.rs +++ b/crates/stdlib/src/faulthandler.rs @@ -304,10 +304,15 @@ mod decl { let registry = vm.state.thread_frames.lock(); // First dump non-current threads, then current thread last + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for (&tid, slot) in registry.iter() { if tid == current_tid { continue; } + let frames_guard = slot.frames.lock(); dump_traceback_thread_frames(fd, tid, false, &frames_guard); puts(fd, "\n"); diff --git a/crates/vm/src/builtins/frame.rs b/crates/vm/src/builtins/frame.rs index 0cb16b235..ab45f6867 100644 --- a/crates/vm/src/builtins/frame.rs +++ b/crates/vm/src/builtins/frame.rs @@ -738,6 +738,10 @@ impl Py { #[cfg(feature = "threading")] { let registry = vm.state.thread_frames.lock(); + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for slot in registry.values() { let frames = slot.frames.lock(); // SAFETY: the owning thread can't pop while we hold the Mutex, diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index 93596a9da..e20cbcb8e 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -455,6 +455,11 @@ impl GcState { // Step 2: Build gc_refs map (copy reference counts) let mut gc_refs: std::collections::HashMap = std::collections::HashMap::new(); + + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for &ptr in &collecting { let obj = unsafe { ptr.0.as_ref() }; gc_refs.insert(ptr, obj.strong_count()); @@ -468,6 +473,11 @@ impl GcState { // results, causing live objects to be incorrectly collected. let mut referents_map: std::collections::HashMap>> = std::collections::HashMap::new(); + + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for &ptr in &collecting { let obj = unsafe { ptr.0.as_ref() }; if obj.strong_count() == 0 { @@ -489,6 +499,10 @@ impl GcState { let mut reachable: HashSet = HashSet::new(); let mut worklist: Vec = Vec::new(); + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for (&ptr, &refs) in &gc_refs { if refs > 0 { reachable.insert(ptr); diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 32a2906fa..5766c5b8a 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -273,10 +273,16 @@ impl StopTheWorldState { let registry = vm.state.thread_frames.lock(); let mut attached_seen = 0u64; let mut forced_parks = 0u64; + + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for (&id, slot) in registry.iter() { if id == requester { continue; } + let state = slot.state.load(Ordering::Relaxed); if state == THREAD_DETACHED { // CAS DETACHED → SUSPENDED (park without thread cooperation) @@ -412,10 +418,16 @@ impl StopTheWorldState { // thread-slot initialization. self.requested.store(false, Ordering::Release); self.world_stopped.store(false, Ordering::Release); + + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for (&id, slot) in registry.iter() { if id == requester { continue; } + slot.stop_requested.store(false, Ordering::Release); let state = slot.state.load(Ordering::Relaxed); debug_assert!( @@ -427,6 +439,7 @@ impl StopTheWorldState { slot.thread.unpark(); } } + drop(registry); self.thread_countdown.store(0, Ordering::Release); self.requester.store(0, Ordering::Relaxed); @@ -506,10 +519,16 @@ impl StopTheWorldState { use thread::THREAD_SUSPENDED; let requester = self.requester.load(Ordering::Relaxed); let registry = vm.state.thread_frames.lock(); + + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for (&id, slot) in registry.iter() { if id == requester { continue; } + let state = slot.state.load(Ordering::Relaxed); debug_assert!( state == THREAD_SUSPENDED, @@ -523,10 +542,16 @@ impl StopTheWorldState { use thread::THREAD_SUSPENDED; let requester = self.requester.load(Ordering::Relaxed); let registry = vm.state.thread_frames.lock(); + + #[expect( + clippy::iter_over_hash_type, + reason = "Iteration order doesn't matter here" + )] for (&id, slot) in registry.iter() { if id == requester { continue; } + let state = slot.state.load(Ordering::Relaxed); debug_assert!( state != THREAD_SUSPENDED,