Merge pull request #288 from japaric/safe-droppable

japaric · web-flow · commit 56d2a37f76d7 · 2022-05-09T13:23:30.000Z
remove unsafe from Droppable test helper
diff --git a/src/binary_heap.rs b/src/binary_heap.rs
@@ -582,15 +582,15 @@ mod tests {
             v.pop().unwrap();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
 
         {
             let mut v: BinaryHeap<Droppable, Max, 2> = BinaryHeap::new();
             v.push(Droppable::new()).ok().unwrap();
             v.push(Droppable::new()).ok().unwrap();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
 
         {
             let mut v: BinaryHeap<Droppable, Min, 2> = BinaryHeap::new();
@@ -599,15 +599,15 @@ mod tests {
             v.pop().unwrap();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
 
         {
             let mut v: BinaryHeap<Droppable, Min, 2> = BinaryHeap::new();
             v.push(Droppable::new()).ok().unwrap();
             v.push(Droppable::new()).ok().unwrap();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
     }
 
     #[test]
diff --git a/src/deque.rs b/src/deque.rs
@@ -565,29 +565,6 @@ mod tests {
         let mut _v: Deque<i32, 4> = Deque::new();
     }
 
-    macro_rules! droppable {
-        () => {
-            struct Droppable;
-            impl Droppable {
-                fn new() -> Self {
-                    unsafe {
-                        COUNT += 1;
-                    }
-                    Droppable
-                }
-            }
-            impl Drop for Droppable {
-                fn drop(&mut self) {
-                    unsafe {
-                        COUNT -= 1;
-                    }
-                }
-            }
-
-            static mut COUNT: i32 = 0;
-        };
-    }
-
     #[test]
     fn drop() {
         droppable!();
@@ -599,22 +576,22 @@ mod tests {
             v.pop_front().unwrap();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
 
         {
             let mut v: Deque<Droppable, 2> = Deque::new();
             v.push_back(Droppable::new()).ok().unwrap();
             v.push_back(Droppable::new()).ok().unwrap();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
         {
             let mut v: Deque<Droppable, 2> = Deque::new();
             v.push_front(Droppable::new()).ok().unwrap();
             v.push_front(Droppable::new()).ok().unwrap();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
     }
 
     #[test]
@@ -754,7 +731,7 @@ mod tests {
             let _ = items.next();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
 
         {
             let mut deque: Deque<Droppable, 2> = Deque::new();
@@ -764,7 +741,7 @@ mod tests {
             // Move none
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
 
         {
             let mut deque: Deque<Droppable, 2> = Deque::new();
@@ -774,7 +751,7 @@ mod tests {
             let _ = items.next(); // Move partly
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
     }
 
     #[test]
diff --git a/src/test_helpers.rs b/src/test_helpers.rs
@@ -1,23 +1,23 @@
 macro_rules! droppable {
     () => {
+        static COUNT: core::sync::atomic::AtomicI32 = core::sync::atomic::AtomicI32::new(0);
+
         #[derive(Eq, Ord, PartialEq, PartialOrd)]
         struct Droppable(i32);
         impl Droppable {
             fn new() -> Self {
-                unsafe {
-                    COUNT += 1;
-                    Droppable(COUNT)
-                }
+                COUNT.fetch_add(1, core::sync::atomic::Ordering::Relaxed);
+                Droppable(Self::count())
+            }
+
+            fn count() -> i32 {
+                COUNT.load(core::sync::atomic::Ordering::Relaxed)
             }
         }
         impl Drop for Droppable {
             fn drop(&mut self) {
-                unsafe {
-                    COUNT -= 1;
-                }
+                COUNT.fetch_sub(1, core::sync::atomic::Ordering::Relaxed);
             }
         }
-
-        static mut COUNT: i32 = 0;
     };
 }
diff --git a/src/vec.rs b/src/vec.rs
@@ -912,15 +912,15 @@ mod tests {
             v.pop().unwrap();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
 
         {
             let mut v: Vec<Droppable, 2> = Vec::new();
             v.push(Droppable::new()).ok().unwrap();
             v.push(Droppable::new()).ok().unwrap();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
     }
 
     #[test]
@@ -1055,7 +1055,7 @@ mod tests {
             let _ = items.next();
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
 
         {
             let mut vec: Vec<Droppable, 2> = Vec::new();
@@ -1065,7 +1065,7 @@ mod tests {
             // Move none
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
 
         {
             let mut vec: Vec<Droppable, 2> = Vec::new();
@@ -1075,7 +1075,7 @@ mod tests {
             let _ = items.next(); // Move partly
         }
 
-        assert_eq!(unsafe { COUNT }, 0);
+        assert_eq!(Droppable::count(), 0);
     }
 
     #[test]

Original file line number	Diff line number	Diff line change
`@@ -582,15 +582,15 @@ mod tests {`
`582`	`582`	`v.pop().unwrap();`
`583`	`583`	`}`
`584`	`584`
`585`		`- assert_eq!(unsafe { COUNT }, 0);`
	`585`	`+ assert_eq!(Droppable::count(), 0);`
`586`	`586`
`587`	`587`	`{`
`588`	`588`	`let mut v: BinaryHeap<Droppable, Max, 2> = BinaryHeap::new();`
`589`	`589`	`v.push(Droppable::new()).ok().unwrap();`
`590`	`590`	`v.push(Droppable::new()).ok().unwrap();`
`591`	`591`	`}`
`592`	`592`
`593`		`- assert_eq!(unsafe { COUNT }, 0);`
	`593`	`+ assert_eq!(Droppable::count(), 0);`
`594`	`594`
`595`	`595`	`{`
`596`	`596`	`let mut v: BinaryHeap<Droppable, Min, 2> = BinaryHeap::new();`
`@@ -599,15 +599,15 @@ mod tests {`
`599`	`599`	`v.pop().unwrap();`
`600`	`600`	`}`
`601`	`601`
`602`		`- assert_eq!(unsafe { COUNT }, 0);`
	`602`	`+ assert_eq!(Droppable::count(), 0);`
`603`	`603`
`604`	`604`	`{`
`605`	`605`	`let mut v: BinaryHeap<Droppable, Min, 2> = BinaryHeap::new();`
`606`	`606`	`v.push(Droppable::new()).ok().unwrap();`
`607`	`607`	`v.push(Droppable::new()).ok().unwrap();`
`608`	`608`	`}`
`609`	`609`
`610`		`- assert_eq!(unsafe { COUNT }, 0);`
	`610`	`+ assert_eq!(Droppable::count(), 0);`
`611`	`611`	`}`
`612`	`612`
`613`	`613`	`#[test]`
Original file line number	Diff line number	Diff line change
`@@ -565,29 +565,6 @@ mod tests {`
`565`	`565`	`let mut _v: Deque<i32, 4> = Deque::new();`
`566`	`566`	`}`
`567`	`567`
`568`		`- macro_rules! droppable {`
`569`		`- () => {`
`570`		`- struct Droppable;`
`571`		`- impl Droppable {`
`572`		`- fn new() -> Self {`
`573`		`- unsafe {`
`574`		`- COUNT += 1;`
`575`		`- }`
`576`		`- Droppable`
`577`		`- }`
`578`		`- }`
`579`		`- impl Drop for Droppable {`
`580`		`- fn drop(&mut self) {`
`581`		`- unsafe {`
`582`		`- COUNT -= 1;`
`583`		`- }`
`584`		`- }`
`585`		`- }`
`586`		`-`
`587`		`- static mut COUNT: i32 = 0;`
`588`		`- };`
`589`		`- }`
`590`		`-`
`591`	`568`	`#[test]`
`592`	`569`	`fn drop() {`
`593`	`570`	`droppable!();`
`@@ -599,22 +576,22 @@ mod tests {`
`599`	`576`	`v.pop_front().unwrap();`
`600`	`577`	`}`
`601`	`578`
`602`		`- assert_eq!(unsafe { COUNT }, 0);`
	`579`	`+ assert_eq!(Droppable::count(), 0);`
`603`	`580`
`604`	`581`	`{`
`605`	`582`	`let mut v: Deque<Droppable, 2> = Deque::new();`
`606`	`583`	`v.push_back(Droppable::new()).ok().unwrap();`
`607`	`584`	`v.push_back(Droppable::new()).ok().unwrap();`
`608`	`585`	`}`
`609`	`586`
`610`		`- assert_eq!(unsafe { COUNT }, 0);`
	`587`	`+ assert_eq!(Droppable::count(), 0);`
`611`	`588`	`{`
`612`	`589`	`let mut v: Deque<Droppable, 2> = Deque::new();`
`613`	`590`	`v.push_front(Droppable::new()).ok().unwrap();`
`614`	`591`	`v.push_front(Droppable::new()).ok().unwrap();`
`615`	`592`	`}`
`616`	`593`
`617`		`- assert_eq!(unsafe { COUNT }, 0);`
	`594`	`+ assert_eq!(Droppable::count(), 0);`
`618`	`595`	`}`
`619`	`596`
`620`	`597`	`#[test]`
`@@ -754,7 +731,7 @@ mod tests {`
`754`	`731`	`let _ = items.next();`
`755`	`732`	`}`
`756`	`733`
`757`		`- assert_eq!(unsafe { COUNT }, 0);`
	`734`	`+ assert_eq!(Droppable::count(), 0);`
`758`	`735`
`759`	`736`	`{`
`760`	`737`	`let mut deque: Deque<Droppable, 2> = Deque::new();`
`@@ -764,7 +741,7 @@ mod tests {`
`764`	`741`	`// Move none`
`765`	`742`	`}`
`766`	`743`
`767`		`- assert_eq!(unsafe { COUNT }, 0);`
	`744`	`+ assert_eq!(Droppable::count(), 0);`
`768`	`745`
`769`	`746`	`{`
`770`	`747`	`let mut deque: Deque<Droppable, 2> = Deque::new();`
`@@ -774,7 +751,7 @@ mod tests {`
`774`	`751`	`let _ = items.next(); // Move partly`
`775`	`752`	`}`
`776`	`753`
`777`		`- assert_eq!(unsafe { COUNT }, 0);`
	`754`	`+ assert_eq!(Droppable::count(), 0);`
`778`	`755`	`}`
`779`	`756`
`780`	`757`	`#[test]`
Original file line number	Diff line number	Diff line change
`@@ -912,15 +912,15 @@ mod tests {`
`912`	`912`	`v.pop().unwrap();`
`913`	`913`	`}`
`914`	`914`
`915`		`- assert_eq!(unsafe { COUNT }, 0);`
	`915`	`+ assert_eq!(Droppable::count(), 0);`
`916`	`916`
`917`	`917`	`{`
`918`	`918`	`let mut v: Vec<Droppable, 2> = Vec::new();`
`919`	`919`	`v.push(Droppable::new()).ok().unwrap();`
`920`	`920`	`v.push(Droppable::new()).ok().unwrap();`
`921`	`921`	`}`
`922`	`922`
`923`		`- assert_eq!(unsafe { COUNT }, 0);`
	`923`	`+ assert_eq!(Droppable::count(), 0);`
`924`	`924`	`}`
`925`	`925`
`926`	`926`	`#[test]`
`@@ -1055,7 +1055,7 @@ mod tests {`
`1055`	`1055`	`let _ = items.next();`
`1056`	`1056`	`}`
`1057`	`1057`
`1058`		`- assert_eq!(unsafe { COUNT }, 0);`
	`1058`	`+ assert_eq!(Droppable::count(), 0);`
`1059`	`1059`
`1060`	`1060`	`{`
`1061`	`1061`	`let mut vec: Vec<Droppable, 2> = Vec::new();`
`@@ -1065,7 +1065,7 @@ mod tests {`
`1065`	`1065`	`// Move none`
`1066`	`1066`	`}`
`1067`	`1067`
`1068`		`- assert_eq!(unsafe { COUNT }, 0);`
	`1068`	`+ assert_eq!(Droppable::count(), 0);`
`1069`	`1069`
`1070`	`1070`	`{`
`1071`	`1071`	`let mut vec: Vec<Droppable, 2> = Vec::new();`
`@@ -1075,7 +1075,7 @@ mod tests {`
`1075`	`1075`	`let _ = items.next(); // Move partly`
`1076`	`1076`	`}`
`1077`	`1077`
`1078`		`- assert_eq!(unsafe { COUNT }, 0);`
	`1078`	`+ assert_eq!(Droppable::count(), 0);`
`1079`	`1079`	`}`
`1080`	`1080`
`1081`	`1081`	`#[test]`