auto merge of #13934 : huonw/rust/transmute-mut, r=alexcrichton

Turning a `&T` into an `&mut T` is undefined behaviour, and needs to be
done very very carefully. Providing a convenience function for exactly
this task is a bad idea, just tempting people into doing the wrong
thing.

(The right thing is to use types like `Cell`, `RefCell` or `Unsafe`.)

cc https://github.com/mozilla/rust/issues/13933

4 files changed, 64 insertions, 39 deletions

diff --git a/src/libstd/cast.rs b/src/libstd/cast.rs
index bcbf804d7b3..7a8f517bbf9 100644
--- a/src/libstd/cast.rs
+++ b/src/libstd/cast.rs
@@ -60,6 +60,7 @@ pub unsafe fn transmute<L, G>(thing: L) -> G {
 
 /// Coerce an immutable reference to be mutable.
 #[inline]
+#[deprecated="casting &T to &mut T is undefined behaviour: use Cell<T>, RefCell<T> or Unsafe<T>"]
 pub unsafe fn transmute_mut<'a,T>(ptr: &'a T) -> &'a mut T { transmute(ptr) }
 
 /// Coerce a reference to have an arbitrary associated lifetime.
diff --git a/src/libstd/comm/mod.rs b/src/libstd/comm/mod.rs
index 92e3e82c1c5..c7849892465 100644
--- a/src/libstd/comm/mod.rs
+++ b/src/libstd/comm/mod.rs
@@ -271,7 +271,6 @@
 // And now that you've seen all the races that I found and attempted to fix,
 // here's the code for you to find some more!
 
-use cast;
 use cell::Cell;
 use clone::Clone;
 use iter::Iterator;
@@ -284,6 +283,7 @@ use result::{Ok, Err, Result};
 use rt::local::Local;
 use rt::task::{Task, BlockedTask};
 use sync::arc::UnsafeArc;
+use ty::Unsafe;
 
 pub use comm::select::{Select, Handle};
 
@@ -325,7 +325,7 @@ static RESCHED_FREQ: int = 256;
 /// The receiving-half of Rust's channel type. This half can only be owned by
 /// one task
 pub struct Receiver<T> {
-    inner: Flavor<T>,
+    inner: Unsafe<Flavor<T>>,
     receives: Cell<uint>,
     // can't share in an arc
     marker: marker::NoShare,
@@ -341,7 +341,7 @@ pub struct Messages<'a, T> {
 /// The sending-half of Rust's asynchronous channel type. This half can only be
 /// owned by one task, but it can be cloned to send to other tasks.
 pub struct Sender<T> {
-    inner: Flavor<T>,
+    inner: Unsafe<Flavor<T>>,
     sends: Cell<uint>,
     // can't share in an arc
     marker: marker::NoShare,
@@ -390,6 +390,27 @@ enum Flavor<T> {
     Sync(UnsafeArc<sync::Packet<T>>),
 }
 
+#[doc(hidden)]
+trait UnsafeFlavor<T> {
+    fn inner_unsafe<'a>(&'a self) -> &'a Unsafe<Flavor<T>>;
+    unsafe fn mut_inner<'a>(&'a self) -> &'a mut Flavor<T> {
+        &mut *self.inner_unsafe().get()
+    }
+    unsafe fn inner<'a>(&'a self) -> &'a Flavor<T> {
+        &*self.inner_unsafe().get()
+    }
+}
+impl<T> UnsafeFlavor<T> for Sender<T> {
+    fn inner_unsafe<'a>(&'a self) -> &'a Unsafe<Flavor<T>> {
+        &self.inner
+    }
+}
+impl<T> UnsafeFlavor<T> for Receiver<T> {
+    fn inner_unsafe<'a>(&'a self) -> &'a Unsafe<Flavor<T>> {
+        &self.inner
+    }
+}
+
 /// Creates a new asynchronous channel, returning the sender/receiver halves.
 ///
 /// All data sent on the sender will become available on the receiver, and no
@@ -458,7 +479,7 @@ pub fn sync_channel<T: Send>(bound: uint) -> (SyncSender<T>, Receiver<T>) {
 
 impl<T: Send> Sender<T> {
     fn new(inner: Flavor<T>) -> Sender<T> {
-        Sender { inner: inner, sends: Cell::new(0), marker: marker::NoShare }
+        Sender { inner: Unsafe::new(inner), sends: Cell::new(0), marker: marker::NoShare }
     }
 
     /// Sends a value along this channel to be received by the corresponding
@@ -532,7 +553,7 @@ impl<T: Send> Sender<T> {
             task.map(|t| t.maybe_yield());
         }
 
-        let (new_inner, ret) = match self.inner {
+        let (new_inner, ret) = match *unsafe { self.inner() } {
             Oneshot(ref p) => {
                 let p = p.get();
                 unsafe {
@@ -564,8 +585,8 @@ impl<T: Send> Sender<T> {
         };
 
         unsafe {
-            let mut tmp = Sender::new(Stream(new_inner));
-            mem::swap(&mut cast::transmute_mut(self).inner, &mut tmp.inner);
+            let tmp = Sender::new(Stream(new_inner));
+            mem::swap(self.mut_inner(), tmp.mut_inner());
         }
         return ret;
     }
@@ -573,7 +594,7 @@ impl<T: Send> Sender<T> {
 
 impl<T: Send> Clone for Sender<T> {
     fn clone(&self) -> Sender<T> {
-        let (packet, sleeper) = match self.inner {
+        let (packet, sleeper) = match *unsafe { self.inner() } {
             Oneshot(ref p) => {
                 let (a, b) = UnsafeArc::new2(shared::Packet::new());
                 match unsafe { (*p.get()).upgrade(Receiver::new(Shared(a))) } {
@@ -598,8 +619,8 @@ impl<T: Send> Clone for Sender<T> {
         unsafe {
             (*packet.get()).inherit_blocker(sleeper);
 
-            let mut tmp = Sender::new(Shared(packet.clone()));
-            mem::swap(&mut cast::transmute_mut(self).inner, &mut tmp.inner);
+            let tmp = Sender::new(Shared(packet.clone()));
+            mem::swap(self.mut_inner(), tmp.mut_inner());
         }
         Sender::new(Shared(packet))
     }
@@ -608,7 +629,7 @@ impl<T: Send> Clone for Sender<T> {
 #[unsafe_destructor]
 impl<T: Send> Drop for Sender<T> {
     fn drop(&mut self) {
-        match self.inner {
+        match *unsafe { self.mut_inner() } {
             Oneshot(ref mut p) => unsafe { (*p.get()).drop_chan(); },
             Stream(ref mut p) => unsafe { (*p.get()).drop_chan(); },
             Shared(ref mut p) => unsafe { (*p.get()).drop_chan(); },
@@ -705,7 +726,7 @@ impl<T: Send> Drop for SyncSender<T> {
 
 impl<T: Send> Receiver<T> {
     fn new(inner: Flavor<T>) -> Receiver<T> {
-        Receiver { inner: inner, receives: Cell::new(0), marker: marker::NoShare }
+        Receiver { inner: Unsafe::new(inner), receives: Cell::new(0), marker: marker::NoShare }
     }
 
     /// Blocks waiting for a value on this receiver
@@ -757,7 +778,7 @@ impl<T: Send> Receiver<T> {
         }
 
         loop {
-            let mut new_port = match self.inner {
+            let new_port = match *unsafe { self.inner() } {
                 Oneshot(ref p) => {
                     match unsafe { (*p.get()).try_recv() } {
                         Ok(t) => return Ok(t),
@@ -790,8 +811,8 @@ impl<T: Send> Receiver<T> {
                 }
             };
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
-                          &mut new_port.inner);
+                mem::swap(self.mut_inner(),
+                          new_port.mut_inner());
             }
         }
     }
@@ -810,7 +831,7 @@ impl<T: Send> Receiver<T> {
     /// the value found on the receiver is returned.
     pub fn recv_opt(&self) -> Result<T, ()> {
         loop {
-            let mut new_port = match self.inner {
+            let new_port = match *unsafe { self.inner() } {
                 Oneshot(ref p) => {
                     match unsafe { (*p.get()).recv() } {
                         Ok(t) => return Ok(t),
@@ -837,8 +858,7 @@ impl<T: Send> Receiver<T> {
                 Sync(ref p) => return unsafe { (*p.get()).recv() }
             };
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
-                          &mut new_port.inner);
+                mem::swap(self.mut_inner(), new_port.mut_inner());
             }
         }
     }
@@ -853,7 +873,7 @@ impl<T: Send> Receiver<T> {
 impl<T: Send> select::Packet for Receiver<T> {
     fn can_recv(&self) -> bool {
         loop {
-            let mut new_port = match self.inner {
+            let new_port = match *unsafe { self.inner() } {
                 Oneshot(ref p) => {
                     match unsafe { (*p.get()).can_recv() } {
                         Ok(ret) => return ret,
@@ -874,15 +894,15 @@ impl<T: Send> select::Packet for Receiver<T> {
                 }
             };
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
-                          &mut new_port.inner);
+                mem::swap(self.mut_inner(),
+                          new_port.mut_inner());
             }
         }
     }
 
     fn start_selection(&self, mut task: BlockedTask) -> Result<(), BlockedTask>{
         loop {
-            let (t, mut new_port) = match self.inner {
+            let (t, new_port) = match *unsafe { self.inner() } {
                 Oneshot(ref p) => {
                     match unsafe { (*p.get()).start_selection(task) } {
                         oneshot::SelSuccess => return Ok(()),
@@ -906,8 +926,8 @@ impl<T: Send> select::Packet for Receiver<T> {
             };
             task = t;
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
-                          &mut new_port.inner);
+                mem::swap(self.mut_inner(),
+                          new_port.mut_inner());
             }
         }
     }
@@ -915,7 +935,7 @@ impl<T: Send> select::Packet for Receiver<T> {
     fn abort_selection(&self) -> bool {
         let mut was_upgrade = false;
         loop {
-            let result = match self.inner {
+            let result = match *unsafe { self.inner() } {
                 Oneshot(ref p) => unsafe { (*p.get()).abort_selection() },
                 Stream(ref p) => unsafe {
                     (*p.get()).abort_selection(was_upgrade)
@@ -927,11 +947,11 @@ impl<T: Send> select::Packet for Receiver<T> {
                     (*p.get()).abort_selection()
                 },
             };
-            let mut new_port = match result { Ok(b) => return b, Err(p) => p };
+            let new_port = match result { Ok(b) => return b, Err(p) => p };
             was_upgrade = true;
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
-                          &mut new_port.inner);
+                mem::swap(self.mut_inner(),
+                          new_port.mut_inner());
             }
         }
     }
@@ -944,7 +964,7 @@ impl<'a, T: Send> Iterator<T> for Messages<'a, T> {
 #[unsafe_destructor]
 impl<T: Send> Drop for Receiver<T> {
     fn drop(&mut self) {
-        match self.inner {
+        match *unsafe { self.mut_inner() } {
             Oneshot(ref mut p) => unsafe { (*p.get()).drop_port(); },
             Stream(ref mut p) => unsafe { (*p.get()).drop_port(); },
             Shared(ref mut p) => unsafe { (*p.get()).drop_port(); },
diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs
index bc6e324a5f7..e5c7ba4aa54 100644
--- a/src/libstd/local_data.rs
+++ b/src/libstd/local_data.rs
@@ -196,11 +196,11 @@ pub fn get_mut<T: 'static, U>(key: Key<T>, f: |Option<&mut T>| -> U) -> U {
         match x {
             None => f(None),
             // We're violating a lot of compiler guarantees with this
-            // invocation of `transmute_mut`, but we're doing runtime checks to
+            // invocation of `transmute`, but we're doing runtime checks to
             // ensure that it's always valid (only one at a time).
             //
             // there is no need to be upset!
-            Some(x) => { f(Some(unsafe { cast::transmute_mut(x) })) }
+            Some(x) => { f(Some(unsafe { cast::transmute::<&_, &mut _>(x) })) }
         }
     })
 }
diff --git a/src/libstd/slice.rs b/src/libstd/slice.rs
index cb8550d248e..d8c866ef44a 100644
--- a/src/libstd/slice.rs
+++ b/src/libstd/slice.rs
@@ -1739,7 +1739,9 @@ impl<'a,T> MutableVector<'a, T> for &'a mut [T] {
         if self.len() == 0 { return None; }
         unsafe {
             let s: &mut Slice<T> = transmute(self);
-            Some(cast::transmute_mut(&*raw::shift_ptr(s)))
+            // FIXME #13933: this `&` -> `&mut` cast is a little
+            // dubious
+            Some(&mut *(raw::shift_ptr(s) as *mut _))
         }
     }
 
@@ -1747,7 +1749,9 @@ impl<'a,T> MutableVector<'a, T> for &'a mut [T] {
         if self.len() == 0 { return None; }
         unsafe {
             let s: &mut Slice<T> = transmute(self);
-            Some(cast::transmute_mut(&*raw::pop_ptr(s)))
+            // FIXME #13933: this `&` -> `&mut` cast is a little
+            // dubious
+            Some(&mut *(raw::pop_ptr(s) as *mut _))
         }
     }
 
@@ -3108,23 +3112,23 @@ mod tests {
     #[should_fail]
     fn test_from_elem_fail() {
         use cast;
+        use cell::Cell;
         use rc::Rc;
 
         struct S {
-            f: int,
+            f: Cell<int>,
             boxes: (~int, Rc<int>)
         }
 
         impl Clone for S {
             fn clone(&self) -> S {
-                let s = unsafe { cast::transmute_mut(self) };
-                s.f += 1;
-                if s.f == 10 { fail!() }
-                S { f: s.f, boxes: s.boxes.clone() }
+                self.f.set(self.f.get() + 1);
+                if self.f.get() == 10 { fail!() }
+                S { f: self.f, boxes: self.boxes.clone() }
             }
         }
 
-        let s = S { f: 0, boxes: (box 0, Rc::new(0)) };
+        let s = S { f: Cell::new(0), boxes: (box 0, Rc::new(0)) };
         let _ = Vec::from_elem(100, s);
     }