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

num::NonZero::get can be 1 transmute instead of 2 #123139

Merged
merged 1 commit into from
Mar 28, 2024
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
26 changes: 14 additions & 12 deletions library/core/src/num/nonzero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,15 @@ use super::{IntErrorKind, ParseIntError};
///
/// # Safety
///
/// Types implementing this trait must be primitves that are valid when zeroed.
/// Types implementing this trait must be primitives that are valid when zeroed.
///
/// The associated `Self::NonZeroInner` type must have the same size+align as `Self`,
/// but with a niche and bit validity making it so the following `transmutes` are sound:
Copy link
Member Author

@scottmcm scottmcm Mar 27, 2024

Choose a reason for hiding this comment

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

I think these were already implied -- they certainly already hold for all the implementations that exist -- but figured I might as well write them out for anyone reading this stuff later.

(And it's a perma-unstable trait, so we can do whatever.)

///
/// - `Self::NonZeroInner` to `Option<Self::NonZeroInner>`
/// - `Option<Self::NonZeroInner>` to `Self`
///
/// (And, consequently, `Self::NonZeroInner` to `Self`.)
#[unstable(
feature = "nonzero_internals",
reason = "implementation detail which may disappear or be replaced at any time",
Expand Down Expand Up @@ -434,17 +442,11 @@ where
// of some not-inlined function, LLVM don't have range metadata
// to understand that the value cannot be zero.
//
// SAFETY: `Self` is guaranteed to have the same layout as `Option<Self>`.
match unsafe { intrinsics::transmute_unchecked(self) } {
None => {
// SAFETY: `NonZero` is guaranteed to only contain non-zero values, so this is unreachable.
unsafe { intrinsics::unreachable() }
}
Some(Self(inner)) => {
// SAFETY: `T::NonZeroInner` is guaranteed to have the same layout as `T`.
unsafe { intrinsics::transmute_unchecked(inner) }
}
}
// For now, using the transmute `assume`s the range at runtime.
//
// SAFETY: `ZeroablePrimitive` guarantees that the size and bit validity
// of `.0` is such that this transmute is sound.
unsafe { intrinsics::transmute_unchecked(self) }
}
}

Expand Down
Loading