Skip to content

Add more docs - mostly warnings - to std::mem::transmute #34609

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

Merged
merged 15 commits into from
Jul 27, 2016
150 changes: 145 additions & 5 deletions src/libcore/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,17 +278,157 @@ extern "rust-intrinsic" {
/// Moves a value out of scope without running drop glue.
pub fn forget<T>(_: T) -> ();

/// Unsafely transforms a value of one type into a value of another type.
/// Reinterprets the bits of a value of one type as another type. Both types
Copy link
Member

Choose a reason for hiding this comment

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

The first paragraph needs to be a single sentence as it's used as a short summary on the module page and search results.

/// must have the same size. Neither the original, nor the result, may be an
/// [invalid value]
/// (https://doc.rust-lang.org/nomicon/meet-safe-and-unsafe.html).
Copy link
Member

Choose a reason for hiding this comment

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

This link should be ../../nomicon/meet-safe-and-unsafe.html so it works in the offline docs.

///
/// Both types must have the same size.
/// `transmute::<T, U>(t)` is semantically equivalent to the following:
///
/// # Examples
/// ```
/// // assuming that T and U are the same size
/// unsafe fn transmute<T, U>(t: T) -> U {
/// let u: U = mem::uninitialized();
/// std::ptr::copy_nonoverlapping(&t as *const T as *const u8,
/// &mut u as *mut U as *mut u8,
/// mem::size_of::<T>());
/// mem::forget(t);
/// u
/// }
/// ```
Copy link
Member

Choose a reason for hiding this comment

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

I feel that this would be more useful to most users if explained intuitively, rather than by giving code.

Copy link
Contributor Author

@strega-nil strega-nil Jul 5, 2016

Choose a reason for hiding this comment

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

How does one "explain intuitively" this concept. For comparison, "Unsafely transforms" is what the previous docs had. Many people think it does a reinterpret_cast. I think it's necessary to have a real code example, in order to tell one what's actually happening. (I'm not just being thick, if you have a suggestion, please make it. I'm actually not sure how to explain it intuitively).

Copy link
Member

Choose a reason for hiding this comment

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

Something like "transmute returns a bitwise copy of the input. Before returning, the input's memory is freed without calling any destructor or otherwise tidying up"

///
/// `transmute` is incredibly unsafe. There are a vast number of ways to
/// cause undefined behavior with this function. `transmute` should be
/// the absolute last resort.
Copy link
Member

Choose a reason for hiding this comment

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

I object to the tone here, I think it will serve only to annoy readers, rather than warn them off. Rather than an implicit argument from authority, it would be better to say why transmute is unsafe and how it can cause UB and why that is bad.

Copy link
Contributor Author

@strega-nil strega-nil Jul 5, 2016

Choose a reason for hiding this comment

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

Perhaps. However, we already have the nomicon, and that is the point of it. We want to keep away people however possible. And, if you know what you're doing, you're not reading this blurb.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to have a sentence here which briefly describes why transmute is unsafe, and hopefully inspires the reader to go to the nomicon to find out more. We can't expect that every reader will also read the nomicon.

We want to keep away people however possible

I don't agree. We want to give users enough information to make an informed choice and to properly understand the trade-offs they are making with that choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess... I've just seen so many people use it poorly, making their Rust code less safe.

///
/// The [nomicon](https://doc.rust-lang.org/nomicon/transmutes.html) has
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ollie27 thank you for these :)

/// more complete documentation. Read it before using `transmute`.
Copy link
Member

Choose a reason for hiding this comment

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

"Read it before using transmute" is unnecessary and patronising

Copy link
Contributor Author

@strega-nil strega-nil Jul 5, 2016

Choose a reason for hiding this comment

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

I don't think it's unnecessary. You should read these docs before using transmute, transmute is one of the most Undefined Behavior-causing, misused parts of Rust. It's an advanced part of Rust, which should almost never be (mis)used by beginners.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, but the fact that there is a reference to it here in the docs implies that you should read it, you don't need to spell it out too. I think it will make readers less likely to actually read the reference because they will be irritated.

///
/// # Alternatives
///
/// There are very few good cases for `transmute`. Most can be achieved
/// through other means. Some more or less common uses, and a better way,
/// are as follows:
///
/// ```
/// use std::mem;
///
/// let array: &[u8] = unsafe { mem::transmute("Rust") };
/// assert_eq!(array, [82, 117, 115, 116]);
/// // turning a pointer into a usize
/// {
/// let ptr = &0;
/// let ptr_num_transmute = mem::transmute::<&i32, usize>(ptr);
/// // Use `as` casts instead
/// let ptr_num_cast = ptr as *const i32 as usize;
/// }
///
/// // Turning a *mut T into an &mut T
/// {
/// let ptr: *mut i32 = &mut 0;
/// let ref_transmuted = mem::transmute::<*mut i32, &mut i32>(ptr);
/// // Use reborrows
/// let ref_casted = &mut *ptr;
/// }
///
/// // Turning an &mut T into an &mut U
/// {
/// let ptr = &mut 0;
/// let val_transmuted = mem::transmute::<&mut i32, &mut u32>(ptr);
/// // Now let's put together `as` and reborrowing
/// let val_casts = &mut *(ptr as *mut i32 as *mut u32);
/// }
///
/// // Turning an `&str` into an `&[u8]`
/// {
/// // this is not a good way to do this.
/// let slice = unsafe { mem::transmute::<&str, &[u8]>("Rust") };
/// assert_eq!(slice, [82, 117, 115, 116]);
/// // You could use `str::as_bytes`
/// let slice = "Rust".as_bytes();
/// assert_eq!(slice, [82, 117, 115, 116]);
/// // Or, just use a byte string, if you have control over the string
/// // literal
/// assert_eq!(b"Rust", [82, 117, 116, 116]);
/// }
///
/// // Turning a Vec<&T> into a Vec<Option<&T>>
/// {
/// let store = [0, 1, 2, 3];
/// let v_orig = store.iter().collect::<Vec<&i32>>();
/// // Using transmute; Undefined Behavior
/// let v_transmuted = mem::transmute::<Vec<&i32>, Vec<Option<&i32>>>(
/// v_orig.clone());
/// // The suggested, safe way
/// let v_collected = v_orig.clone()
/// .into_iter()
/// .map(|r| Some(r))
/// .collect::<Vec<Option<&i32>>>();
/// // The no-copy, unsafe way, still using transmute, but not UB
/// // This is equivalent to the original, but safer, and reuses the
/// // same Vec internals. Therefore the new inner type must have the
/// // exact same size, and the same or lesser alignment, as the old
/// // type. The same caveats exist for this method as transmute, for
/// // the original inner type (`&i32`) to the converted inner type
/// // (`Option<&i32>`), so read the nomicon page linked above.
/// let v_no_copy = Vec::from_raw_parts(v_orig.as_mut_ptr(),
/// v_orig.len(),
/// v_orig.capacity());
/// mem::forget(v_orig);
/// }
///
Copy link
Contributor

Choose a reason for hiding this comment

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

Note the chaining of as to "hop" between types. as is not transitive; it requires explicit conversions between each step. (or something, to give intuition as what in the world is going on here :-D )

///
/// // Copying an `&mut T` to reslice:
/// {
/// fn split_at_mut_transmute<T>(slice: &mut [T], index: usize)
/// -> (&mut [T], &mut [T]) {
/// let len = slice.len();
/// assert!(index < len);
/// let slice2 = mem::transmute::<&mut [T], &mut [T]>(slice);
/// (slice[0..index], slice2[index..len])
/// }
/// // Again, use `as` and reborrowing
/// fn split_at_mut_casts<T>(slice: &mut [T], index: usize)
/// -> (&mut [T], &mut [T]) {
/// let len = slice.len();
/// assert!(index < len);
/// let slice2 = &mut *(slice as *mut [T]); // actually typesafe!
Copy link
Member

Choose a reason for hiding this comment

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

slice and slice2 are aliased here so is this really safe?

This isn't how split_at_mut is actually implemented so I'm not sure this is a good example and the advice should be just to use the method in std.

/// (slice[0..index], slice2[index..len])
/// }
/// }
/// ```
///
/// # Examples
///
/// There are valid uses of transmute, though they are few and far between.
///
/// ```
/// // getting the bitpattern of a floating point type
/// {
/// let x = mem::transmute::<f32, u32>(0.0/0.0)
/// }
///
///
/// // turning a pointer into a function pointer
/// {
/// // in file.c: `int foo(void) { ... }`
/// let handle: *mut libc::c_void = libc::dlopen(
/// b"file.so\0".as_ptr() as *const libc::c_char, libc::RTLD_LAZY);
/// let foo: *mut libc::c_void = libc::dlsym(
/// handle,
/// b"foo\0".as_ptr() as *const libc::c_char);
/// let foo = mem::transmute::<*mut libc::c_void,
/// extern fn() -> libc::c_int>(foo);
/// println!("{}", foo());
/// }
///
///
/// // extending an invariant lifetime; this is advanced, very unsafe rust
/// {
/// struct T<'a>(&'a i32);
/// let value = 0;
/// let t = T::new(&value);
/// let ptr = &mut t;
/// let ptr_extended = mem::transmute::<&mut T, &mut T<'static>>(ptr);
/// }
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn transmute<T, U>(e: T) -> U;
Expand Down