Rollup merge of #142520 - hkBst:less-static-mut, r=tgross35

alloc: less static mut + some cleanup

I'm looking into #125035 and would like some feedback on my approach.

Author: jhpratt

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;
 
@@ -58,48 +56,33 @@ fn list_from<T: Clone>(v: &[T]) -> LinkedList<T> {
     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<T>(list: &LinkedList<T>) {
-    unsafe {
-        let mut len = 0;
-        let mut last_ptr: Option<&Node<T>> = None;
-        let mut node_ptr: &Node<T>;
-        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<T>, pptr.as_ptr() as *const Node<T>);
-                }
-                _ => 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;
-                }
-            }
-        }
-
-        // 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<T>, node_ptr as *const Node<T>);
-        // check that len matches interior links.
-        assert_eq!(len, list.len);
+    let mut node: &Node<T> = 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;
+    };
+
+    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,21 +1010,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<u32> = Cell::new(0);}
+
+        struct $struct_name$(($elt_ty))?;
 
-    #[derive(Debug)]
-    struct D(u32);
+        impl Drop for $struct_name {
+            fn drop(&mut self) {
+                $drop_counter.set($drop_counter.get() + 1);
 
-    impl Drop for D {
-        fn drop(&mut self) {
-            unsafe {
-                DROPS += 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 +1041,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 +1060,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,54 +1075,32 @@ 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);
     ring.push_front(Elem);
     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 +1114,7 @@ fn test_drop_panic() {
 
     catch_unwind(move || drop(q)).ok();
 
-    assert_eq!(unsafe { DROPS }, 8);
+    assert_eq!(DROPS.get(), 8);
 }
 
 #[test]