Rollup merge of rust-lang#133651 - scottmcm:nonnull-nonzero-no-field-…

…projection, r=oli-obk

Update `NonZero` and `NonNull` to not field-project (per MCP#807)

rust-lang/compiler-team#807 (comment) was accepted, so this is the first PR towards moving the library to not using field projections into `[rustc_layout_scalar_valid_range_*]` types.

`NonZero` was already using `transmute` nearly everywhere, so there are very few changes to it.

`NonNull` needed more changes, but they're mostly simple, changing `.pointer` to `.as_ptr()`.

r? libs

cc rust-lang#133324, which will tidy up some of the MIR from this a bit more, but isn't a blocker.

library/core/src/num/nonzero.rs

@@ -37,6 +37,8 @@ pub unsafe trait ZeroablePrimitive: Sized + Copy + private::Sealed {
 macro_rules! impl_zeroable_primitive {
     ($($NonZeroInner:ident ( $primitive:ty )),+ $(,)?) => {
         mod private {
+            use super::*;
+
             #[unstable(
                 feature = "nonzero_internals",
                 reason = "implementation detail which may disappear or be replaced at any time",
@@ -45,7 +47,11 @@ macro_rules! impl_zeroable_primitive {
             pub trait Sealed {}
 
             $(
-                #[derive(Debug, Clone, Copy, PartialEq)]
+                // This inner type is never shown directly, so intentionally does not have Debug
+                #[expect(missing_debug_implementations)]
+                // Since this struct is non-generic and derives Copy,
+                // the derived Clone is `*self` and thus doesn't field-project.
+                #[derive(Clone, Copy)]
                 #[repr(transparent)]
                 #[rustc_layout_scalar_valid_range_start(1)]
                 #[rustc_nonnull_optimization_guaranteed]
@@ -55,6 +61,16 @@ macro_rules! impl_zeroable_primitive {
                     issue = "none"
                 )]
                 pub struct $NonZeroInner($primitive);
+
+                // This is required to allow matching a constant.  We don't get it from a derive
+                // because the derived `PartialEq` would do a field projection, which is banned
+                // by <https://github.com/rust-lang/compiler-team/issues/807>.
+                #[unstable(
+                    feature = "nonzero_internals",
+                    reason = "implementation detail which may disappear or be replaced at any time",
+                    issue = "none"
+                )]
+                impl StructuralPartialEq for $NonZeroInner {}
             )+
         }
 
@@ -172,7 +188,7 @@ where
 {
     #[inline]
     fn clone(&self) -> Self {
-        Self(self.0)
+        *self
     }
 }
 
@@ -440,15 +456,21 @@ where
     #[rustc_const_stable(feature = "const_nonzero_get", since = "1.34.0")]
     #[inline]
     pub const fn get(self) -> T {
-        // FIXME: This can be changed to simply `self.0` once LLVM supports `!range` metadata
-        // for function arguments: https://github.com/llvm/llvm-project/issues/76628
-        //
         // Rustc can set range metadata only if it loads `self` from
         // memory somewhere. If the value of `self` was from by-value argument
         // of some not-inlined function, LLVM don't have range metadata
         // to understand that the value cannot be zero.
         //
-        // For now, using the transmute `assume`s the range at runtime.
+        // Using the transmute `assume`s the range at runtime.
+        //
+        // Even once LLVM supports `!range` metadata for function arguments
+        // (see <https://github.com/llvm/llvm-project/issues/76628>), this can't
+        // be `.0` because MCP#807 bans field-projecting into `scalar_valid_range`
+        // types, and it arguably wouldn't want to be anyway because if this is
+        // MIR-inlined, there's no opportunity to put that argument metadata anywhere.
+        //
+        // The good answer here will eventually be pattern types, which will hopefully
+        // allow it to go back to `.0`, maybe with a cast of some sort.
         //
         // SAFETY: `ZeroablePrimitive` guarantees that the size and bit validity
         // of `.0` is such that this transmute is sound.

library/core/src/ptr/non_null.rs

@@ -7,7 +7,7 @@ use crate::pin::PinCoerceUnsized;
 use crate::ptr::Unique;
 use crate::slice::{self, SliceIndex};
 use crate::ub_checks::assert_unsafe_precondition;
-use crate::{fmt, hash, intrinsics, ptr};
+use crate::{fmt, hash, intrinsics, mem, ptr};
 
 /// `*mut T` but non-zero and [covariant].
 ///
@@ -69,6 +69,8 @@ use crate::{fmt, hash, intrinsics, ptr};
 #[rustc_nonnull_optimization_guaranteed]
 #[rustc_diagnostic_item = "NonNull"]
 pub struct NonNull<T: ?Sized> {
+    // Remember to use `.as_ptr()` instead of `.pointer`, as field projecting to
+    // this is banned by <https://github.com/rust-lang/compiler-team/issues/807>.
     pointer: *const T,
 }
 
@@ -282,7 +284,7 @@ impl<T: ?Sized> NonNull<T> {
     pub fn addr(self) -> NonZero<usize> {
         // SAFETY: The pointer is guaranteed by the type to be non-null,
         // meaning that the address will be non-zero.
-        unsafe { NonZero::new_unchecked(self.pointer.addr()) }
+        unsafe { NonZero::new_unchecked(self.as_ptr().addr()) }
     }
 
     /// Creates a new pointer with the given address and the [provenance][crate::ptr#provenance] of
@@ -296,7 +298,7 @@ impl<T: ?Sized> NonNull<T> {
     #[stable(feature = "strict_provenance", since = "1.84.0")]
     pub fn with_addr(self, addr: NonZero<usize>) -> Self {
         // SAFETY: The result of `ptr::from::with_addr` is non-null because `addr` is guaranteed to be non-zero.
-        unsafe { NonNull::new_unchecked(self.pointer.with_addr(addr.get()) as *mut _) }
+        unsafe { NonNull::new_unchecked(self.as_ptr().with_addr(addr.get()) as *mut _) }
     }
 
     /// Creates a new pointer by mapping `self`'s address to a new one, preserving the
@@ -335,7 +337,12 @@ impl<T: ?Sized> NonNull<T> {
     #[must_use]
     #[inline(always)]
     pub const fn as_ptr(self) -> *mut T {
-        self.pointer as *mut T
+        // This is a transmute for the same reasons as `NonZero::get`.
+
+        // SAFETY: `NonNull` is `transparent` over a `*const T`, and `*const T`
+        // and `*mut T` have the same layout, so transitively we can transmute
+        // our `NonNull` to a `*mut T` directly.
+        unsafe { mem::transmute::<Self, *mut T>(self) }
     }
 
     /// Returns a shared reference to the value. If the value may be uninitialized, [`as_uninit_ref`]
@@ -484,7 +491,7 @@ impl<T: ?Sized> NonNull<T> {
         // Additionally safety contract of `offset` guarantees that the resulting pointer is
         // pointing to an allocation, there can't be an allocation at null, thus it's safe to
         // construct `NonNull`.
-        unsafe { NonNull { pointer: intrinsics::offset(self.pointer, count) } }
+        unsafe { NonNull { pointer: intrinsics::offset(self.as_ptr(), count) } }
     }
 
     /// Calculates the offset from a pointer in bytes.
@@ -508,7 +515,7 @@ impl<T: ?Sized> NonNull<T> {
         // Additionally safety contract of `offset` guarantees that the resulting pointer is
         // pointing to an allocation, there can't be an allocation at null, thus it's safe to
         // construct `NonNull`.
-        unsafe { NonNull { pointer: self.pointer.byte_offset(count) } }
+        unsafe { NonNull { pointer: self.as_ptr().byte_offset(count) } }
     }
 
     /// Adds an offset to a pointer (convenience for `.offset(count as isize)`).
@@ -560,7 +567,7 @@ impl<T: ?Sized> NonNull<T> {
         // Additionally safety contract of `offset` guarantees that the resulting pointer is
         // pointing to an allocation, there can't be an allocation at null, thus it's safe to
         // construct `NonNull`.
-        unsafe { NonNull { pointer: intrinsics::offset(self.pointer, count) } }
+        unsafe { NonNull { pointer: intrinsics::offset(self.as_ptr(), count) } }
     }
 
     /// Calculates the offset from a pointer in bytes (convenience for `.byte_offset(count as isize)`).
@@ -584,7 +591,7 @@ impl<T: ?Sized> NonNull<T> {
         // Additionally safety contract of `add` guarantees that the resulting pointer is pointing
         // to an allocation, there can't be an allocation at null, thus it's safe to construct
         // `NonNull`.
-        unsafe { NonNull { pointer: self.pointer.byte_add(count) } }
+        unsafe { NonNull { pointer: self.as_ptr().byte_add(count) } }
     }
 
     /// Subtracts an offset from a pointer (convenience for
@@ -666,7 +673,7 @@ impl<T: ?Sized> NonNull<T> {
         // Additionally safety contract of `sub` guarantees that the resulting pointer is pointing
         // to an allocation, there can't be an allocation at null, thus it's safe to construct
         // `NonNull`.
-        unsafe { NonNull { pointer: self.pointer.byte_sub(count) } }
+        unsafe { NonNull { pointer: self.as_ptr().byte_sub(count) } }
     }
 
     /// Calculates the distance between two pointers within the same allocation. The returned value is in
@@ -763,7 +770,7 @@ impl<T: ?Sized> NonNull<T> {
         T: Sized,
     {
         // SAFETY: the caller must uphold the safety contract for `offset_from`.
-        unsafe { self.pointer.offset_from(origin.pointer) }
+        unsafe { self.as_ptr().offset_from(origin.as_ptr()) }
     }
 
     /// Calculates the distance between two pointers within the same allocation. The returned value is in
@@ -781,7 +788,7 @@ impl<T: ?Sized> NonNull<T> {
     #[rustc_const_stable(feature = "non_null_convenience", since = "1.80.0")]
     pub const unsafe fn byte_offset_from<U: ?Sized>(self, origin: NonNull<U>) -> isize {
         // SAFETY: the caller must uphold the safety contract for `byte_offset_from`.
-        unsafe { self.pointer.byte_offset_from(origin.pointer) }
+        unsafe { self.as_ptr().byte_offset_from(origin.as_ptr()) }
     }
 
     // N.B. `wrapping_offset``, `wrapping_add`, etc are not implemented because they can wrap to null
@@ -856,7 +863,7 @@ impl<T: ?Sized> NonNull<T> {
         T: Sized,
     {
         // SAFETY: the caller must uphold the safety contract for `sub_ptr`.
-        unsafe { self.pointer.sub_ptr(subtracted.pointer) }
+        unsafe { self.as_ptr().sub_ptr(subtracted.as_ptr()) }
     }
 
     /// Calculates the distance between two pointers within the same allocation, *where it's known that
@@ -875,7 +882,7 @@ impl<T: ?Sized> NonNull<T> {
     #[rustc_const_unstable(feature = "const_ptr_sub_ptr", issue = "95892")]
     pub const unsafe fn byte_sub_ptr<U: ?Sized>(self, origin: NonNull<U>) -> usize {
         // SAFETY: the caller must uphold the safety contract for `byte_sub_ptr`.
-        unsafe { self.pointer.byte_sub_ptr(origin.pointer) }
+        unsafe { self.as_ptr().byte_sub_ptr(origin.as_ptr()) }
     }
 
     /// Reads the value from `self` without moving it. This leaves the
@@ -893,7 +900,7 @@ impl<T: ?Sized> NonNull<T> {
         T: Sized,
     {
         // SAFETY: the caller must uphold the safety contract for `read`.
-        unsafe { ptr::read(self.pointer) }
+        unsafe { ptr::read(self.as_ptr()) }
     }
 
     /// Performs a volatile read of the value from `self` without moving it. This
@@ -914,7 +921,7 @@ impl<T: ?Sized> NonNull<T> {
         T: Sized,
     {
         // SAFETY: the caller must uphold the safety contract for `read_volatile`.
-        unsafe { ptr::read_volatile(self.pointer) }
+        unsafe { ptr::read_volatile(self.as_ptr()) }
     }
 
     /// Reads the value from `self` without moving it. This leaves the
@@ -934,7 +941,7 @@ impl<T: ?Sized> NonNull<T> {
         T: Sized,
     {
         // SAFETY: the caller must uphold the safety contract for `read_unaligned`.
-        unsafe { ptr::read_unaligned(self.pointer) }
+        unsafe { ptr::read_unaligned(self.as_ptr()) }
     }
 
     /// Copies `count * size_of<T>` bytes from `self` to `dest`. The source
@@ -954,7 +961,7 @@ impl<T: ?Sized> NonNull<T> {
         T: Sized,
     {
         // SAFETY: the caller must uphold the safety contract for `copy`.
-        unsafe { ptr::copy(self.pointer, dest.as_ptr(), count) }
+        unsafe { ptr::copy(self.as_ptr(), dest.as_ptr(), count) }
     }
 
     /// Copies `count * size_of<T>` bytes from `self` to `dest`. The source
@@ -974,7 +981,7 @@ impl<T: ?Sized> NonNull<T> {
         T: Sized,
     {
         // SAFETY: the caller must uphold the safety contract for `copy_nonoverlapping`.
-        unsafe { ptr::copy_nonoverlapping(self.pointer, dest.as_ptr(), count) }
+        unsafe { ptr::copy_nonoverlapping(self.as_ptr(), dest.as_ptr(), count) }
     }
 
     /// Copies `count * size_of<T>` bytes from `src` to `self`. The source
@@ -994,7 +1001,7 @@ impl<T: ?Sized> NonNull<T> {
         T: Sized,
     {
         // SAFETY: the caller must uphold the safety contract for `copy`.
-        unsafe { ptr::copy(src.pointer, self.as_ptr(), count) }
+        unsafe { ptr::copy(src.as_ptr(), self.as_ptr(), count) }
     }
 
     /// Copies `count * size_of<T>` bytes from `src` to `self`. The source
@@ -1014,7 +1021,7 @@ impl<T: ?Sized> NonNull<T> {
         T: Sized,
     {
         // SAFETY: the caller must uphold the safety contract for `copy_nonoverlapping`.
-        unsafe { ptr::copy_nonoverlapping(src.pointer, self.as_ptr(), count) }
+        unsafe { ptr::copy_nonoverlapping(src.as_ptr(), self.as_ptr(), count) }
     }
 
     /// Executes the destructor (if any) of the pointed-to value.
@@ -1201,7 +1208,7 @@ impl<T: ?Sized> NonNull<T> {
 
         {
             // SAFETY: `align` has been checked to be a power of 2 above.
-            unsafe { ptr::align_offset(self.pointer, align) }
+            unsafe { ptr::align_offset(self.as_ptr(), align) }
         }
     }
 
@@ -1229,7 +1236,7 @@ impl<T: ?Sized> NonNull<T> {
     where
         T: Sized,
     {
-        self.pointer.is_aligned()
+        self.as_ptr().is_aligned()
     }
 
     /// Returns whether the pointer is aligned to `align`.
@@ -1266,7 +1273,7 @@ impl<T: ?Sized> NonNull<T> {
     #[must_use]
     #[unstable(feature = "pointer_is_aligned_to", issue = "96284")]
     pub fn is_aligned_to(self, align: usize) -> bool {
-        self.pointer.is_aligned_to(align)
+        self.as_ptr().is_aligned_to(align)
     }
 }
 

tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-abort.diff

@@ -31,7 +31,6 @@
               }
           }
           scope 9 (inlined NonNull::<[u8]>::as_ptr) {
-              let mut _17: *const [u8];
           }
       }
       scope 3 (inlined #[track_caller] Option::<Layout>::unwrap) {
@@ -102,16 +101,9 @@
           StorageDead(_16);
           StorageDead(_12);
           StorageDead(_6);
--         StorageLive(_17);
-+         nop;
-          _17 = copy (_5.0: *const [u8]);
--         _4 = move _17 as *mut [u8] (PtrToPtr);
--         StorageDead(_17);
-+         _4 = copy _17 as *mut [u8] (PtrToPtr);
-+         nop;
+          _4 = copy _5 as *mut [u8] (Transmute);
           StorageDead(_5);
--         _3 = move _4 as *mut u8 (PtrToPtr);
-+         _3 = copy _17 as *mut u8 (PtrToPtr);
+          _3 = move _4 as *mut u8 (PtrToPtr);
           StorageDead(_4);
           StorageDead(_3);
 -         StorageDead(_1);

tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-unwind.diff

@@ -20,7 +20,6 @@
           scope 5 (inlined <std::alloc::Global as Allocator>::allocate) {
           }
           scope 6 (inlined NonNull::<[u8]>::as_ptr) {
-              let mut _12: *const [u8];
           }
       }
       scope 3 (inlined #[track_caller] Option::<Layout>::unwrap) {
@@ -45,16 +44,9 @@
 
       bb1: {
           StorageDead(_6);
--         StorageLive(_12);
-+         nop;
-          _12 = copy (_5.0: *const [u8]);
--         _4 = move _12 as *mut [u8] (PtrToPtr);
--         StorageDead(_12);
-+         _4 = copy _12 as *mut [u8] (PtrToPtr);
-+         nop;
+          _4 = copy _5 as *mut [u8] (Transmute);
           StorageDead(_5);
--         _3 = move _4 as *mut u8 (PtrToPtr);
-+         _3 = copy _12 as *mut u8 (PtrToPtr);
+          _3 = move _4 as *mut u8 (PtrToPtr);
           StorageDead(_4);
           StorageDead(_3);
 -         StorageDead(_1);

tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-abort.diff

@@ -31,7 +31,6 @@
               }
           }
           scope 9 (inlined NonNull::<[u8]>::as_ptr) {
-              let mut _17: *const [u8];
           }
       }
       scope 3 (inlined #[track_caller] Option::<Layout>::unwrap) {
@@ -102,16 +101,9 @@
           StorageDead(_16);
           StorageDead(_12);
           StorageDead(_6);
--         StorageLive(_17);
-+         nop;
-          _17 = copy (_5.0: *const [u8]);
--         _4 = move _17 as *mut [u8] (PtrToPtr);
--         StorageDead(_17);
-+         _4 = copy _17 as *mut [u8] (PtrToPtr);
-+         nop;
+          _4 = copy _5 as *mut [u8] (Transmute);
           StorageDead(_5);
--         _3 = move _4 as *mut u8 (PtrToPtr);
-+         _3 = copy _17 as *mut u8 (PtrToPtr);
+          _3 = move _4 as *mut u8 (PtrToPtr);
           StorageDead(_4);
           StorageDead(_3);
 -         StorageDead(_1);