Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std: deprecate cast::transmute_mut. #13934

Merged
merged 2 commits into from
May 6, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/libarena/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
Expand Down
3 changes: 2 additions & 1 deletion src/libnative/io/file_win32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
3 changes: 2 additions & 1 deletion src/librustuv/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
1 change: 1 addition & 0 deletions src/libstd/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know / have thought about how long deprecated features should stay around? I don't want to start the discussion here but I think, given the fact that some things may be deprecated in the near future, we should start discussing this and make the removal date/time part of the deprecation message.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a reasonable amount of time to allow people to get help updating, a relatively obscure function like this is probably ok to remove quicker than normal, a more common function might be left longer than normal. In any case it was mentioned briefly in the original email, specifically

we'll likely leave in #[deprecated] functions for about a month or so.

In terms of actual implementation details of removal, I personally think just using occasionally checking for things to remove & consulting the git history for timing is good enough (since we don't have so much code/#[deprecated]-churn that doing it manually with grep will be particularly hard); others may disagree with this though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@huonw all this sounds good to me. I think a month or so is fine for now. We'll likely need a better rule when we get nearer a 1.0 release.

pub unsafe fn transmute_mut<'a,T>(ptr: &'a T) -> &'a mut T { transmute(ptr) }

/// Coerce a reference to have an arbitrary associated lifetime.
Expand Down
78 changes: 49 additions & 29 deletions src/libstd/comm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -564,16 +585,16 @@ 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;
}
}

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))) } {
Expand All @@ -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))
}
Expand All @@ -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(); },
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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());
}
}
}
Expand All @@ -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),
Expand All @@ -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());
}
}
}
Expand All @@ -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,
Expand All @@ -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(()),
Expand All @@ -906,16 +926,16 @@ 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());
}
}
}

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)
Expand All @@ -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());
}
}
}
Expand All @@ -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(); },
Expand Down
4 changes: 2 additions & 2 deletions src/libstd/local_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) })) }
}
})
}
Expand Down
20 changes: 12 additions & 8 deletions src/libstd/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1739,15 +1739,19 @@ 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 _))
}
}

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

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

Expand Down
2 changes: 1 addition & 1 deletion src/libsync/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()) }
}
}

Expand Down