]> git.lizzy.rs Git - rust.git/commitdiff
std: deprecate cast::transmute_mut.
authorHuon Wilson <dbau.pp+github@gmail.com>
Sun, 4 May 2014 13:17:37 +0000 (23:17 +1000)
committerHuon Wilson <dbau.pp+github@gmail.com>
Mon, 5 May 2014 08:20:41 +0000 (18:20 +1000)
Turning a `&T` into an `&mut T` carries a large risk of 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`.

For memory safety, Rust has that guarantee that `&mut` pointers do not
alias with any other pointer, that is, if you have a `&mut T` then that
is the only usable pointer to that `T`. This allows Rust to assume that
writes through a `&mut T` do not affect the values of any other `&` or
`&mut` references. `&` pointers have no guarantees about aliasing or
not, so it's entirely possible for the same pointer to be passed into
both arguments of a function like

    fn foo(x: &int, y: &int) { ... }

Converting either of `x` or `y` to a `&mut` pointer and modifying it
would affect the other value: invalid behaviour.

(Similarly, it's undefined behaviour to modify the value of an immutable
local, like `let x = 1;`.)

At a low-level, the *only* safe way to obtain an `&mut` out of a `&` is
using the `Unsafe` type (there are higher level wrappers around it, like
`Cell`, `RefCell`, `Mutex` etc.). The `Unsafe` type is registered with
the compiler so that it can reason a little about these `&` to `&mut`
casts, but it is still up to the user to ensure that the `&mut`s
obtained out of an `Unsafe` never alias.

(Note that *any* conversion from `&` to `&mut` can be invalid, including
a plain `transmute`, or casting `&T` -> `*T` -> `*mut T` -> `&mut T`.)

[breaking-change]

src/libarena/lib.rs
src/libnative/io/file_win32.rs
src/librustuv/file.rs
src/libstd/cast.rs
src/libstd/comm/mod.rs
src/libstd/local_data.rs
src/libstd/slice.rs
src/libsync/arc.rs

index 95062d84e09859227a48254e89a495c994c27d5d..d80385587dad5f66998a75dc743115a06ef0bee4 100644 (file)
@@ -26,7 +26,7 @@
 
 extern crate collections;
 
-use std::cast::{transmute, transmute_mut, transmute_mut_lifetime};
+use std::cast::{transmute, transmute_mut_lifetime};
 use std::cast;
 use std::cell::{Cell, RefCell};
 use std::mem;
@@ -281,8 +281,8 @@ fn alloc_noncopy<'a, T>(&'a mut self, op: || -> T) -> &'a T {
     #[inline]
     pub fn alloc<'a, T>(&'a self, op: || -> T) -> &'a T {
         unsafe {
-            // FIXME: Borrow check
-            let this = transmute_mut(self);
+            // FIXME #13933: Remove/justify all `&T` to `&mut T` transmutes
+            let this: &mut Arena = transmute::<&_, &mut _>(self);
             if intrinsics::needs_drop::<T>() {
                 this.alloc_noncopy(op)
             } else {
@@ -438,7 +438,8 @@ pub fn with_capacity(capacity: uint) -> TypedArena<T> {
     #[inline]
     pub fn alloc<'a>(&'a self, object: T) -> &'a T {
         unsafe {
-            let this = cast::transmute_mut(self);
+            // FIXME #13933: Remove/justify all `&T` to `&mut T` transmutes
+            let this: &mut TypedArena<T> = cast::transmute::<&_, &mut _>(self);
             if this.ptr == this.end {
                 this.grow()
             }
index 88ba6dcbc7e58a5513df022028eb9cfe826f40f2..aae15a86614420acb4824132d4d8318c56e4b309 100644 (file)
@@ -174,7 +174,8 @@ fn seek(&mut self, pos: i64, style: io::SeekStyle) -> Result<u64, IoError> {
     fn tell(&self) -> Result<u64, IoError> {
         // This transmute is fine because our seek implementation doesn't
         // actually use the mutable self at all.
-        unsafe { cast::transmute_mut(self).seek(0, io::SeekCur) }
+        // FIXME #13933: Remove/justify all `&T` to `&mut T` transmutes
+        unsafe { cast::transmute::<&_, &mut FileDesc>(self).seek(0, io::SeekCur) }
     }
 
     fn fsync(&mut self) -> Result<(), IoError> {
index 665d418ab2ab6e0fd1c718f7d990afbc1fb971d6..9037363f9a00eb46c7338b0f41efbcca1b3a10b5 100644 (file)
@@ -445,7 +445,8 @@ fn seek(&mut self, pos: i64, whence: io::SeekStyle) -> Result<u64, IoError> {
     fn tell(&self) -> Result<u64, IoError> {
         use libc::SEEK_CUR;
         // this is temporary
-        let self_ = unsafe { cast::transmute_mut(self) };
+        // FIXME #13933: Remove/justify all `&T` to `&mut T` transmutes
+        let self_ = unsafe { cast::transmute::<&_, &mut FileWatcher>(self) };
         self_.seek_common(0, SEEK_CUR)
     }
     fn fsync(&mut self) -> Result<(), IoError> {
index bcbf804d7b3084704c1045ab5eaea72410ba281f..7a8f517bbf92c0af6d6014f554b68b550c353b4d 100644 (file)
@@ -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.
index 92e3e82c1c5a19f7cad10ab19b344a4265e757a9..bbe34d20f6a2e2f0ea615a630a1e12215ec94315 100644 (file)
@@ -318,6 +318,11 @@ fn f() $b
 mod shared;
 mod sync;
 
+// FIXME #13933: Remove/justify all `&T` to `&mut T` transmutes
+unsafe fn transmute_mut<'a,T>(x: &'a T) -> &'a mut T {
+    cast::transmute::<&_, &mut _>(x)
+}
+
 // Use a power of 2 to allow LLVM to optimize to something that's not a
 // division, this is hit pretty regularly.
 static RESCHED_FREQ: int = 256;
@@ -565,7 +570,7 @@ pub fn send_opt(&self, t: T) -> Result<(), T> {
 
         unsafe {
             let mut tmp = Sender::new(Stream(new_inner));
-            mem::swap(&mut cast::transmute_mut(self).inner, &mut tmp.inner);
+            mem::swap(&mut transmute_mut(self).inner, &mut tmp.inner);
         }
         return ret;
     }
@@ -599,7 +604,7 @@ fn clone(&self) -> Sender<T> {
             (*packet.get()).inherit_blocker(sleeper);
 
             let mut tmp = Sender::new(Shared(packet.clone()));
-            mem::swap(&mut cast::transmute_mut(self).inner, &mut tmp.inner);
+            mem::swap(&mut transmute_mut(self).inner, &mut tmp.inner);
         }
         Sender::new(Shared(packet))
     }
@@ -790,7 +795,7 @@ pub fn try_recv(&self) -> Result<T, TryRecvError> {
                 }
             };
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
+                mem::swap(&mut transmute_mut(self).inner,
                           &mut new_port.inner);
             }
         }
@@ -837,7 +842,7 @@ pub fn recv_opt(&self) -> Result<T, ()> {
                 Sync(ref p) => return unsafe { (*p.get()).recv() }
             };
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
+                mem::swap(&mut transmute_mut(self).inner,
                           &mut new_port.inner);
             }
         }
@@ -874,7 +879,7 @@ fn can_recv(&self) -> bool {
                 }
             };
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
+                mem::swap(&mut transmute_mut(self).inner,
                           &mut new_port.inner);
             }
         }
@@ -906,7 +911,7 @@ fn start_selection(&self, mut task: BlockedTask) -> Result<(), BlockedTask>{
             };
             task = t;
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
+                mem::swap(&mut transmute_mut(self).inner,
                           &mut new_port.inner);
             }
         }
@@ -930,7 +935,7 @@ fn abort_selection(&self) -> bool {
             let mut new_port = match result { Ok(b) => return b, Err(p) => p };
             was_upgrade = true;
             unsafe {
-                mem::swap(&mut cast::transmute_mut(self).inner,
+                mem::swap(&mut transmute_mut(self).inner,
                           &mut new_port.inner);
             }
         }
index bc6e324a5f7b06aa492c12727605b9c6cd3ffb57..e5c7ba4aa54bcbf9b0166c5eecda9086d8300d9c 100644 (file)
@@ -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) })) }
         }
     })
 }
index cb8550d248eabf17fbe9b784c8a44e0be7969139..d8c866ef44a39193e34996efaef6f649741cf248 100644 (file)
@@ -1739,7 +1739,9 @@ fn mut_shift_ref(&mut self) -> Option<&'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 @@ fn mut_pop_ref(&mut self) -> Option<&'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 @@ fn test_from_fn_fail() {
     #[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);
     }
 
index b5c660759522eab96d0407fa8c5a953ca0a73f3f..f5369ec862f8a0bf8bb004805ab354da1c0adf8c 100644 (file)
@@ -148,7 +148,7 @@ pub fn make_unique<'a>(&'a mut self) -> &'a mut T {
         // reference count is guaranteed to be 1 at this point, and we required
         // the Arc itself to be `mut`, so we're returning the only possible
         // reference to the inner data.
-        unsafe { cast::transmute_mut(self.deref()) }
+        unsafe { cast::transmute::<&_, &mut _>(self.deref()) }
     }
 }