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

Add to_bytes and from_bytes to primitive integers #49871

Merged
merged 1 commit into from
Apr 14, 2018
Merged
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
89 changes: 89 additions & 0 deletions src/libcore/num/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use convert::TryFrom;
use fmt;
use intrinsics;
use mem;
#[allow(deprecated)] use nonzero::NonZero;
use ops;
use str::FromStr;
Expand Down Expand Up @@ -1619,6 +1620,50 @@ $EndFeature, "
#[inline]
pub fn is_negative(self) -> bool { self < 0 }
}

/// Return the memory representation of this integer as a byte array.
///
/// The target platform’s native endianness is used.
/// Portable code likely wants to use this after [`to_be`] or [`to_le`].
///
/// [`to_be`]: #method.to_be
/// [`to_le`]: #method.to_le
///
/// # Examples
///
/// ```
/// #![feature(int_to_from_bytes)]
///
/// let bytes = i32::min_value().to_be().to_bytes();
/// assert_eq!(bytes, [0x80, 0, 0, 0]);
/// ```
#[unstable(feature = "int_to_from_bytes", issue = "49792")]
#[inline]
pub fn to_bytes(self) -> [u8; mem::size_of::<Self>()] {
unsafe { mem::transmute(self) }
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't need to be a transmute right? Can you do something like this instead?

unsafe { *(&self as *const Self as *const u8 as *[u8; mem::size_of::<Self>()]) }

Maybe we don't care about decreasing the use of transmute?

Also, neat, I didn't realize mem::size_of could be used at the type level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be that, but that seems harder to read to me. Why would it be preferable? Regarding transmute I often prefer to fully qualify it with a turbofish, but in this case that felt unnecessary since it forms the entire body of a method that has an explicitly-written signature.

size_of became a const fn in #42859

Copy link
Contributor

Choose a reason for hiding this comment

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

I think transmute is fine here, because we're transmuting non-reference types. For reference/pointer types I agree that we should not be using transmute (this is what we suggest in clippy at least)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oli-obk I’m not opposing this principle, but could you say more about why?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming the "why" refers to "transmute is fine for non-reference/pointer types":

let u = transmute::<T, U>(t); additionally checks whether the size of both sides of the conversion match. Going through raw pointer casts is essentially transmute_copy.

There's no increased source for errors when using transmute with all types specified (in this case specified in the function signature of the surrounding function). The raw pointer casts are very sigil heavy and need a few reads to figure out.

If you are transmuting references on the other hand, the size check doesn't gain you anything (because references to sized types are always one pointer large). In that case the raw pointer casts are also less sigil heavy because you don't need the all the dereference or reference expressions, but it's mostly a cast.

}

/// Create an integer value from its memory representation as a byte array.
///
/// The target platform’s native endianness is used.
/// Portable code likely wants to use [`from_be`] or [`from_le`] after this.
///
/// [`from_be`]: #method.from_be
/// [`from_le`]: #method.from_le
///
/// # Examples
///
/// ```
/// #![feature(int_to_from_bytes)]
///
/// let int = i32::from_be(i32::from_bytes([0x80, 0, 0, 0]));
/// assert_eq!(int, i32::min_value());
/// ```
#[unstable(feature = "int_to_from_bytes", issue = "49792")]
#[inline]
pub fn from_bytes(bytes: [u8; mem::size_of::<Self>()]) -> Self {
unsafe { mem::transmute(bytes) }
}
}
}

Expand Down Expand Up @@ -2933,6 +2978,50 @@ $EndFeature, "
self.one_less_than_next_power_of_two().checked_add(1)
}
}

/// Return the memory representation of this integer as a byte array.
///
/// The target platform’s native endianness is used.
/// Portable code likely wants to use this after [`to_be`] or [`to_le`].
///
/// [`to_be`]: #method.to_be
/// [`to_le`]: #method.to_le
///
/// # Examples
///
/// ```
/// #![feature(int_to_from_bytes)]
///
/// let bytes = 0x1234_5678_u32.to_be().to_bytes();
/// assert_eq!(bytes, [0x12, 0x34, 0x56, 0x78]);
/// ```
#[unstable(feature = "int_to_from_bytes", issue = "49792")]
#[inline]
pub fn to_bytes(self) -> [u8; mem::size_of::<Self>()] {
unsafe { mem::transmute(self) }
}

/// Create an integer value from its memory representation as a byte array.
///
/// The target platform’s native endianness is used.
/// Portable code likely wants to use [`to_be`] or [`to_le`] after this.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: These should probably mention from_be and from_le instead.

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 find it amusing that we have for example both to_be and from_be but their implementation is exactly the same. Anyway, fixed.

///
/// [`to_be`]: #method.to_be
/// [`to_le`]: #method.to_le
///
/// # Examples
///
/// ```
/// #![feature(int_to_from_bytes)]
///
/// let int = u32::from_be(u32::from_bytes([0x12, 0x34, 0x56, 0x78]));
/// assert_eq!(int, 0x1234_5678_u32);
/// ```
#[unstable(feature = "int_to_from_bytes", issue = "49792")]
#[inline]
pub fn from_bytes(bytes: [u8; mem::size_of::<Self>()]) -> Self {
unsafe { mem::transmute(bytes) }
}
}
}

Expand Down