Implement `try_transmute!` by jswrenn · Pull Request #1018 · google/zerocopy

jswrenn · 2024-03-04T22:22:34Z

Closes #1013.

Makes progress towards #5.

joshlf · 2024-03-05T21:08:15Z

src/lib.rs

+    ///
+    /// # Safety
+    ///
+    /// Unsafe code may assume that, if `is_bit_valid(src)` returns true, `*src`


Suggested change

/// Unsafe code may assume that, if `is_bit_valid(src)` returns true, `*src`

/// Unsafe code may assume that, if `is_src_valid(src)` returns true, `*src`

joshlf · 2024-03-05T21:09:30Z

src/lib.rs

+    /// # Panics
+    ///
+    /// `is_src_valid` panics under exactly the same circumstances as
+    /// [`TryFromBytes::is_bit_valid`].


Suggested change

/// [`TryFromBytes::is_bit_valid`].

/// [`is_bit_valid`].

///

/// [`is_bit_valid`]: TryFromBytes::is_bit_valid

joshlf · 2024-03-05T21:14:14Z

src/lib.rs

+        }
+
+        // SAFETY:
+        // - `size_of::<Src>()` <= `size_of::<Self>()`


Suggested change

// - `size_of::<Src>()` <= `size_of::<Self>()`

// - We just validated that `size_of::<Src>()` <= `size_of::<Self>()`

src/lib.rs

joshlf · 2024-03-05T21:19:18Z

src/macro_util.rs

+    Src: core::fmt::Debug + crate::IntoBytes,
+    Dst: core::fmt::Debug + crate::TryFromBytes,


Suggested change

Src: core::fmt::Debug + crate::IntoBytes,

Dst: core::fmt::Debug + crate::TryFromBytes,

Src: crate::IntoBytes,

Dst: crate::TryFromBytes,

joshlf · 2024-03-05T21:19:52Z

src/macro_util.rs

+    let src = ManuallyDrop::new(src);
+
+    // SAFETY:
+    // - `src` is a bit-valid instance of `Dst`


Suggested change

// - `src` is a bit-valid instance of `Dst`

// - `is_src_valid` validates that `src` is a bit-valid instance of `Dst`

joshlf

Also: Tests.

joshlf · 2024-03-05T21:34:16Z

src/lib.rs

+/// Conditionally transmutes a value of one type to a value of another type of
+/// the same size.
+///
+/// This macro an expression of type  `Src` and produces a `Result<Dst, Src>`.


Suggested change

/// This macro an expression of type `Src` and produces a `Result<Dst, Src>`.

/// This macro consumes an expression of type `Src` and produces a `Result<Dst, Src>`.

joshlf · 2024-03-05T21:35:45Z

src/lib.rs

+/// Conditionally transmutes a value of one type to a value of another type of
+/// the same size.
+///
+/// This macro an expression of type  `Src` and produces a `Result<Dst, Src>`.
+///
+/// The expression `$e` must have a concrete type, `T`, which implements
+/// [`IntoBytes`]. The `try_transmute!` expression must also have a concrete
+/// type, `U` (`U` is inferred from the calling context), and `U` must implement
+/// [`TryFromBytes`].
+///
+/// Note that the `T` produced by the expression `$e` will *not* be dropped.
+/// Semantically, its bits will be copied into a new value of type `U`, the
+/// original `T` will be forgotten, and the value of type `U` will be returned.
+///
+/// # Examples
+///
+/// ```
+/// # use zerocopy::try_transmute;
+/// assert_eq!(try_transmute!(0u8), Ok(false));
+/// assert_eq!(try_transmute!(1u8), Ok(true));
+/// assert_eq!(try_transmute!(255u8), Result::<bool, _>::Err(255));
+/// ```


We should update or add language to discuss what validation is performed, and to discuss what happens on failure.

src/lib.rs

joshlf · 2024-05-13T19:06:10Z

src/lib.rs

+    /// `is_src_valid` panics under exactly the same circumstances as
+    /// [`is_bit_valid`].


Maybe also mention that size mismatch can cause a panic or a compilation failure? (just like we do for is_bit_valid)

joshlf · 2024-05-13T19:10:42Z

src/macro_util.rs

+    Src: crate::IntoBytes,
+    Dst: crate::TryFromBytes,
+{
+    if !Dst::is_mut_src_valid(&mut src) {


If we can get Unalign<Dst>: KnownLayout without Dst: KnownLayout, then we could use <Unalign<Dst> as TryFromBytes>::try_mut_from here instead of needing to add is_mut_src_valid, right?

Using #1252, we might be able to use Unalign<SizedKnownLayout<Dst>>.

Maybe, though doing so would entail needless runtime checks and ultimately end with throwing away the return value of try_mut_from.

I think is_src_valid (which I've now generalized to take a Ptr<Src>) will also be useful for try_transmute_mut and try_transmute_ref.

joshlf

UI tests? There are a lot of transmute-*.rs UI tests that we could duplicate, and possible some we could add (e.g. for Dst: !TryFromBytes).

src/macro_util.rs

src/lib.rs

jswrenn · 2024-05-14T19:01:39Z

UI tests? There are a lot of transmute-*.rs UI tests that we could duplicate, and possible some we could add (e.g. for Dst: !TryFromBytes).

Done

Closes #1013. Makes progress towards #5.

jswrenn force-pushed the try_transmute branch 2 times, most recently from 41cf362 to 718acaa Compare March 4, 2024 22:41

jswrenn force-pushed the relax-cast_unsized branch from dac2194 to 5c9afdf Compare March 5, 2024 20:45

joshlf requested changes Mar 5, 2024

View reviewed changes

jswrenn force-pushed the try_transmute branch 2 times, most recently from 2050c25 to 24fa819 Compare March 5, 2024 21:31

joshlf requested changes Mar 5, 2024

View reviewed changes

jswrenn force-pushed the try_transmute branch 2 times, most recently from e6dc8ee to 1eba8aa Compare March 5, 2024 21:37

jswrenn force-pushed the try_transmute branch from 1eba8aa to f4227a1 Compare March 13, 2024 18:21

joshlf mentioned this pull request May 3, 2024

0.8 Release Roadmap #671

Closed

87 tasks

jswrenn force-pushed the try_transmute branch 2 times, most recently from 639ee24 to e8b31b6 Compare May 13, 2024 15:33

jswrenn changed the base branch from relax-cast_unsized to main May 13, 2024 15:40

jswrenn force-pushed the try_transmute branch 2 times, most recently from f2fd06c to 80bdd61 Compare May 13, 2024 17:24

jswrenn marked this pull request as ready for review May 13, 2024 17:55

jswrenn requested a review from joshlf May 13, 2024 17:55

joshlf requested changes May 13, 2024

View reviewed changes

jswrenn force-pushed the try_transmute branch 3 times, most recently from 0081904 to 46cfa04 Compare May 14, 2024 16:58

jswrenn requested a review from joshlf May 14, 2024 17:04

joshlf requested changes May 14, 2024

View reviewed changes

src/macro_util.rs Show resolved Hide resolved

src/macro_util.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

jswrenn force-pushed the try_transmute branch 2 times, most recently from d3b7f6e to 004b5b4 Compare May 14, 2024 18:59

jswrenn requested a review from joshlf May 14, 2024 19:01

Implement try_transmute!

967cf31

Closes #1013. Makes progress towards #5.

jswrenn force-pushed the try_transmute branch from 004b5b4 to 967cf31 Compare May 14, 2024 19:15

joshlf approved these changes May 14, 2024

View reviewed changes

joshlf enabled auto-merge May 14, 2024 19:18

joshlf added this pull request to the merge queue May 14, 2024

Merged via the queue into main with commit 5b095e0 May 14, 2024

joshlf deleted the try_transmute branch May 14, 2024 19:46

	/// Unsafe code may assume that, if `is_bit_valid(src)` returns true, `*src`
	/// Unsafe code may assume that, if `is_src_valid(src)` returns true, `*src`

	// - `size_of::<Src>()` <= `size_of::<Self>()`
	// - We just validated that `size_of::<Src>()` <= `size_of::<Self>()`

		Src: core::fmt::Debug + crate::IntoBytes,
		Dst: core::fmt::Debug + crate::TryFromBytes,

	// - `src` is a bit-valid instance of `Dst`
	// - `is_src_valid` validates that `src` is a bit-valid instance of `Dst`

	/// This macro an expression of type `Src` and produces a `Result<Dst, Src>`.
	/// This macro consumes an expression of type `Src` and produces a `Result<Dst, Src>`.

		/// `is_src_valid` panics under exactly the same circumstances as
		/// [`is_bit_valid`].

Conversation

jswrenn commented Mar 4, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshlf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshlf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jswrenn commented May 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants