From e8dfd812dd888cc738035c4d9edd3222138583a7 Mon Sep 17 00:00:00 2001 From: Marijn Schouten Date: Mon, 16 Jun 2025 04:46:06 +0000 Subject: [PATCH 1/2] linked_list tests: less static mut --- .../src/collections/linked_list/tests.rs | 96 ++++++------------- 1 file changed, 30 insertions(+), 66 deletions(-) diff --git a/library/alloc/src/collections/linked_list/tests.rs b/library/alloc/src/collections/linked_list/tests.rs index 812fe229a0fc5..493352447c43c 100644 --- a/library/alloc/src/collections/linked_list/tests.rs +++ b/library/alloc/src/collections/linked_list/tests.rs @@ -1,6 +1,4 @@ -// FIXME(static_mut_refs): Do not allow `static_mut_refs` lint -#![allow(static_mut_refs)] - +use std::cell::Cell; use std::panic::{AssertUnwindSafe, catch_unwind}; use std::thread; @@ -1027,21 +1025,26 @@ fn extract_if_drop_panic_leak() { assert_eq!(d7.dropped(), 1); } -#[test] -#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] -fn extract_if_pred_panic_leak() { - static mut DROPS: i32 = 0; +macro_rules! struct_with_counted_drop { + ($struct_name:ident$(($elt_ty:ty))?, $drop_counter:ident $(=> $drop_stmt:expr)?) => { + thread_local! {static $drop_counter: Cell = Cell::new(0);} - #[derive(Debug)] - struct D(u32); + struct $struct_name$(($elt_ty))?; - impl Drop for D { - fn drop(&mut self) { - unsafe { - DROPS += 1; + impl Drop for $struct_name { + fn drop(&mut self) { + $drop_counter.set($drop_counter.get() + 1); + + $($drop_stmt(self))? } } - } + }; +} + +#[test] +#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] +fn extract_if_pred_panic_leak() { + struct_with_counted_drop!(D(u32), DROPS); let mut q = LinkedList::new(); q.push_back(D(3)); @@ -1053,26 +1056,17 @@ fn extract_if_pred_panic_leak() { q.push_front(D(1)); q.push_front(D(0)); - catch_unwind(AssertUnwindSafe(|| { + _ = catch_unwind(AssertUnwindSafe(|| { q.extract_if(|item| if item.0 >= 2 { panic!() } else { true }).for_each(drop) - })) - .ok(); + })); - assert_eq!(unsafe { DROPS }, 2); // 0 and 1 + assert_eq!(DROPS.get(), 2); // 0 and 1 assert_eq!(q.len(), 6); } #[test] fn test_drop() { - static mut DROPS: i32 = 0; - struct Elem; - impl Drop for Elem { - fn drop(&mut self) { - unsafe { - DROPS += 1; - } - } - } + struct_with_counted_drop!(Elem, DROPS); let mut ring = LinkedList::new(); ring.push_back(Elem); @@ -1081,20 +1075,12 @@ fn test_drop() { ring.push_front(Elem); drop(ring); - assert_eq!(unsafe { DROPS }, 4); + assert_eq!(DROPS.get(), 4); } #[test] fn test_drop_with_pop() { - static mut DROPS: i32 = 0; - struct Elem; - impl Drop for Elem { - fn drop(&mut self) { - unsafe { - DROPS += 1; - } - } - } + struct_with_counted_drop!(Elem, DROPS); let mut ring = LinkedList::new(); ring.push_back(Elem); @@ -1104,23 +1090,15 @@ fn test_drop_with_pop() { drop(ring.pop_back()); drop(ring.pop_front()); - assert_eq!(unsafe { DROPS }, 2); + assert_eq!(DROPS.get(), 2); drop(ring); - assert_eq!(unsafe { DROPS }, 4); + assert_eq!(DROPS.get(), 4); } #[test] fn test_drop_clear() { - static mut DROPS: i32 = 0; - struct Elem; - impl Drop for Elem { - fn drop(&mut self) { - unsafe { - DROPS += 1; - } - } - } + struct_with_counted_drop!(Elem, DROPS); let mut ring = LinkedList::new(); ring.push_back(Elem); @@ -1128,30 +1106,16 @@ fn test_drop_clear() { ring.push_back(Elem); ring.push_front(Elem); ring.clear(); - assert_eq!(unsafe { DROPS }, 4); + assert_eq!(DROPS.get(), 4); drop(ring); - assert_eq!(unsafe { DROPS }, 4); + assert_eq!(DROPS.get(), 4); } #[test] #[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")] fn test_drop_panic() { - static mut DROPS: i32 = 0; - - struct D(bool); - - impl Drop for D { - fn drop(&mut self) { - unsafe { - DROPS += 1; - } - - if self.0 { - panic!("panic in `drop`"); - } - } - } + struct_with_counted_drop!(D(bool), DROPS => |this: &D| if this.0 { panic!("panic in `drop`"); } ); let mut q = LinkedList::new(); q.push_back(D(false)); @@ -1165,7 +1129,7 @@ fn test_drop_panic() { catch_unwind(move || drop(q)).ok(); - assert_eq!(unsafe { DROPS }, 8); + assert_eq!(DROPS.get(), 8); } #[test] From 2d3a37d1e50e61daa1db8d1ee7198ded15bb370e Mon Sep 17 00:00:00 2001 From: Marijn Schouten Date: Mon, 16 Jun 2025 09:42:55 +0000 Subject: [PATCH 2/2] linked_list tests: buff check_links --- .../src/collections/linked_list/tests.rs | 65 +++++++------------ 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/library/alloc/src/collections/linked_list/tests.rs b/library/alloc/src/collections/linked_list/tests.rs index 493352447c43c..410e67d3fdb08 100644 --- a/library/alloc/src/collections/linked_list/tests.rs +++ b/library/alloc/src/collections/linked_list/tests.rs @@ -56,48 +56,33 @@ fn list_from(v: &[T]) -> LinkedList { v.iter().cloned().collect() } +/// Starting from the head of the LinkedList, +/// follow the next links, while checking the prev links, +/// and check that length equals the count of visited nodes. fn check_links(list: &LinkedList) { - unsafe { - let mut len = 0; - let mut last_ptr: Option<&Node> = None; - let mut node_ptr: &Node; - match list.head { - None => { - // tail node should also be None. - assert!(list.tail.is_none()); - assert_eq!(0, list.len); - return; - } - Some(node) => node_ptr = &*node.as_ptr(), - } - loop { - match (last_ptr, node_ptr.prev) { - (None, None) => {} - (None, _) => panic!("prev link for head"), - (Some(p), Some(pptr)) => { - assert_eq!(p as *const Node, pptr.as_ptr() as *const Node); - } - _ => panic!("prev link is none, not good"), - } - match node_ptr.next { - Some(next) => { - last_ptr = Some(node_ptr); - node_ptr = &*next.as_ptr(); - len += 1; - } - None => { - len += 1; - break; - } - } - } + let mut node: &Node = if let Some(node) = list.head { + // SAFETY: depends on correctness of LinkedList + unsafe { &*node.as_ptr() } + } else { + assert!(list.tail.is_none(), "empty list should have no tail node"); + assert_eq!(list.len, 0, "empty list should have length 0"); + return; + }; - // verify that the tail node points to the last node. - let tail = list.tail.as_ref().expect("some tail node").as_ref(); - assert_eq!(tail as *const Node, node_ptr as *const Node); - // check that len matches interior links. - assert_eq!(len, list.len); + assert!(node.prev.is_none(), "head node should not have a prev link"); + let mut prev; + let mut len = 1; + while let Some(next) = node.next { + prev = node; + // SAFETY: depends on correctness of LinkedList + node = unsafe { &*next.as_ptr() }; + len += 1; + assert_eq!(node.prev.expect("missing prev link"), prev.into(), "bad prev link"); } + + let tail = list.tail.expect("list is non-empty, so there should be a tail node"); + assert_eq!(tail, node.into(), "tail node points to the last node"); + assert_eq!(len, list.len, "len matches interior links"); } #[test] @@ -1027,7 +1012,7 @@ fn extract_if_drop_panic_leak() { macro_rules! struct_with_counted_drop { ($struct_name:ident$(($elt_ty:ty))?, $drop_counter:ident $(=> $drop_stmt:expr)?) => { - thread_local! {static $drop_counter: Cell = Cell::new(0);} + thread_local! {static $drop_counter: Cell = Cell::new(0);} struct $struct_name$(($elt_ty))?;