Improve the SAFETY: comments and use the Arc::{into/from}_raw functions

Author: poliorcetics

src/libstd/sync/mpsc/blocking.rs

@@ -1,6 +1,5 @@
 //! Generic support for building blocking abstractions.
 
-use crate::mem;
 use crate::sync::atomic::{AtomicBool, Ordering};
 use crate::sync::Arc;
 use crate::thread::{self, Thread};
@@ -47,9 +46,7 @@ impl SignalToken {
     /// flag.
     #[inline]
     pub unsafe fn cast_to_usize(self) -> usize {
-        // SAFETY: this ok because of the internal represention of Arc, which
-        // is a pointer and phantom data (so the size of a pointer -> a usize).
-        unsafe { mem::transmute(self.inner) }
+        Arc::into_raw(self.inner) as usize
     }
 
     /// Converts from an unsafe usize value. Useful for retrieving a pipe's state
@@ -58,7 +55,10 @@ impl SignalToken {
     pub unsafe fn cast_from_usize(signal_ptr: usize) -> SignalToken {
         // SAFETY: this ok because of the internal represention of Arc, which
         // is a pointer and phantom data (so the size of a pointer -> a usize).
-        SignalToken { inner: unsafe { mem::transmute(signal_ptr) } }
+        //
+        // NOTE: The call to `from_raw` is used in conjuction with the call to
+        // `into_raw` made in the `SignalToken::cast_to_usize` method.
+        SignalToken { inner: unsafe { Arc::from_raw(signal_ptr as *const _) } }
     }
 }
 

src/libstd/sync/mpsc/mod.rs

@@ -651,7 +651,7 @@ trait UnsafeFlavor<T> {
         // SAFETY: We are sure the inner value will never be NUL when this is
         // called, the invariants of the module make it so.
         //
-        // It is also ensured that no other (mutable) reference will be handed
+        // The caller ensures that no other (mutable) reference will be handed
         // out while the one returned here is in action.
         unsafe { &mut *self.inner_unsafe().get() }
     }

src/libstd/sync/mpsc/spsc_queue.rs

@@ -99,8 +99,8 @@ impl<T, ProducerAddition, ConsumerAddition> Queue<T, ProducerAddition, ConsumerA
     ) -> Self {
         let n1 = Node::new();
         let n2 = Node::new();
-        // SAFETY: At this point we know the pointer is not NUL since it was
-        // build just above.
+        // SAFETY: At this point we know the pointer is valid: it was built
+        // just above.
         unsafe { (*n1).next.store(n2, Ordering::Relaxed) }
         Queue {
             consumer: CacheAligned::new(Consumer {
@@ -135,13 +135,10 @@ impl<T, ProducerAddition, ConsumerAddition> Queue<T, ProducerAddition, ConsumerA
     }
 
     unsafe fn alloc(&self) -> *mut Node<T> {
-        // First try to see if we can consume the 'first' node for our uses.
-        // SAFEY: Since `self` is a valid reference, accessing its inner values
-        // can be considered safe (without checking for NUL pointers).
-        //
-        // Dereferencing of `ret` are safe too because the pointer comes from
-        // `self` once again.
+        // SAFEY: ??? Explain why the pointers are safe to dereference and why
+        // returning a mutable pointer is ok. ???
         unsafe {
+            // First try to see if we can consume the 'first' node for our uses.
             if *self.producer.first.get() != *self.producer.tail_copy.get() {
                 let ret = *self.producer.first.get();
                 *self.producer.0.first.get() = (*ret).next.load(Ordering::Relaxed);
@@ -322,7 +319,7 @@ mod tests {
         }
 
         unsafe fn stress_bound(bound: usize) {
-            let q = Arc::new(Queue::with_additions(bound, (), ()));
+            let q = Arc::new(unsafe { Queue::with_additions(bound, (), ()) });
 
             let (tx, rx) = channel();
             let q2 = q.clone();