Skip to content

What to do with transmute_copy #1703

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

Closed
strega-nil opened this issue Aug 4, 2016 · 9 comments
Closed

What to do with transmute_copy #1703

strega-nil opened this issue Aug 4, 2016 · 9 comments
Labels
T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@strega-nil
Copy link

strega-nil commented Aug 4, 2016

This function is a terror. It is full of undefined behavior, even more than it's sister, transmute. The only uses of this function are either Undefined Behavior, are transmutes, except that they didn't try, or are just transmutes in a generic context.

This is unfortunate. I think we should make this function either much more useful, or deprecate it and get rid of the undefined behavior.

Changing the definition to:

unsafe fn transmute_copy<T: ?Sized, U>(t: &T) -> U { 
    let u: U = std::mem::uninitialized();
    std::ptr::copy_nonoverlapping(t as *const t as *const u8, &mut u as *mut u as *mut u8
                                  std::mem::size_of::<U>());
    u
}

would allow us to pull types from byte arrays, for example, without having to do this all by hand.

Otherwise, it should be deprecated, because it isn't really useful enough, and leads to far too many issues; it's only stable because transmute was unstable at the time.

@Amanieu
Copy link
Member

Amanieu commented Aug 4, 2016

Why can't we just add ?Sized to the existing definition? The change you are proposing is to effectively remove the requirement that the address of t is suitably aligned for reading a U. If your intent is to support unaligned reads and writes then I think we would be better off adding specialized ptr::read_unaligned and ptr::write_unaligned functions rather than messing with the definition of transmute_copy.

Regarding your point that transmute_copy isn't useful, I have to disagree. I make extensive use of it in atomic-rs since it is used in a generic context, which makes plain transmute unusable.

@strega-nil
Copy link
Author

strega-nil commented Aug 4, 2016

@Amanieu That is my first suggestion. Change the definition to <T: ?Sized...

If it is used in the community extensively, I'd like that we change it to <T: ?Sized, U>. I don't like that transmute allows unaligned reads, while transmute_copy does not. It's unexpected, and leads to undefined behavior.

@strega-nil
Copy link
Author

I'd argue you should use ptr::{read, write} if you need the unsafe, assumes aligned behavior.

@arielb1
Copy link
Contributor

arielb1 commented Aug 11, 2016

transmute allows unaligned reads

transmute takes a copy (or move) of its source. Any alignment issues occur outside of it.

I'm however +1 for transmute_copy using copy_nonoverlapping rather than ptr::read.

@Amanieu
Copy link
Member

Amanieu commented Aug 11, 2016

I would rather avoid having transmute_copy abused for unaligned access since that is not its main purpose. We should instead add ptr::read_unaligned and ptr::write_unaligned specifically to support unaligned access.

I think changing the signature of transmute_copy to allow ?Sized for the source is pretty uncontroversial.

The main issue that needs to be discussed is what the alignment requirements for the source argument of transmute_copy should be. I see 3 options:

  • Must be suitably aligned for the source type
  • No alignment requirement (what @ubsan is proposing)
  • Must be suitably aligned for the target type (what is currently implemented)

@strega-nil
Copy link
Author

@Amanieu ptr::copy_nonoverlapping is a fine way to do unaligned access. It's what ptr::{read,write}_unaligned would desugar to.

@nrc nrc added T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC. labels Aug 17, 2016
@RalfJung
Copy link
Member

I don't know why this issue got closed in 2017 (@ubsan do you remember?), but 1.5 years later with rust-lang/rust#55052 the implementation got changed to use read_unaligned.

@strega-nil
Copy link
Author

@RalfJung I believe this was closed when I left the Rust community, and didn't want to deal with it.

@RalfJung
Copy link
Member

RalfJung commented Mar 18, 2019

Then I guess I can just hope that there's nothing else we lost track of because of issues being closed like that...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

5 participants