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
This commit is contained in:
bors 2014-05-05 16:51:30 -07:00
commit bd3fb81e5e
8 changed files with 74 additions and 46 deletions

View 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 @@ impl Arena {
#[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 @@ impl<T> 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()
}

View File

@ -174,7 +174,8 @@ impl rtio::RtioFileStream for FileDesc {
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> {

View File

@ -445,7 +445,8 @@ impl rtio::RtioFileStream for FileWatcher {
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> {

View 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.

View File

@ -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(); },

View 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) })) }
}
})
}

View File

@ -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);
}

View File

@ -148,7 +148,7 @@ impl<T: Send + Share + Clone> Arc<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()) }
}
}