Auto merge of rust-lang#96010 - eduardosm:Unique-on-top-of-NonNull, r…

…=m-ou-se,tmiasko Implement `core::ptr::Unique` on top of `NonNull` Removes the use `rustc_layout_scalar_valid_range_start` and some `unsafe` blocks.
Dylan-DPC · Apr 17, 2022 · ac8b118 · ac8b118
2 parents 43a71dc + 7ba0292
commit ac8b118
Show file tree

Hide file tree

Showing 13 changed files with 56 additions and 51 deletions.
diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs
@@ -448,7 +448,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 
                     // a box with a non-zst allocator should not be directly dereferenced
                     if cg_base.layout.ty.is_box() && !cg_base.layout.field(cx, 1).is_zst() {
-                        let ptr = cg_base.extract_field(bx, 0).extract_field(bx, 0);
+                        // Extract `Box<T>` -> `Unique<T>` -> `NonNull<T>` -> `*const T`
+                        let ptr =
+                            cg_base.extract_field(bx, 0).extract_field(bx, 0).extract_field(bx, 0);
 
                         ptr.deref(bx.cx())
                     } else {
@@ -464,7 +466,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                 mir::ProjectionElem::Deref => {
                     // a box with a non-zst allocator should not be directly dereferenced
                     if cg_base.layout.ty.is_box() && !cg_base.layout.field(cx, 1).is_zst() {
-                        let ptr = cg_base.project_field(bx, 0).project_field(bx, 0);
+                        // Project `Box<T>` -> `Unique<T>` -> `NonNull<T>` -> `*const T`
+                        let ptr =
+                            cg_base.project_field(bx, 0).project_field(bx, 0).project_field(bx, 0);
 
                         bx.load_operand(ptr).deref(bx.cx())
                     } else {

diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs
@@ -120,12 +120,14 @@
 #![feature(const_maybe_uninit_uninit_array)]
 #![feature(const_maybe_uninit_as_mut_ptr)]
 #![feature(const_maybe_uninit_assume_init)]
+#![feature(const_nonnull_new)]
 #![feature(const_num_from_num)]
 #![feature(const_ops)]
 #![feature(const_option)]
 #![feature(const_option_ext)]
 #![feature(const_pin)]
 #![feature(const_replace)]
+#![feature(const_ptr_as_ref)]
 #![feature(const_ptr_is_null)]
 #![feature(const_ptr_offset_from)]
 #![feature(const_ptr_read)]

diff --git a/library/core/src/ptr/unique.rs b/library/core/src/ptr/unique.rs
@@ -1,8 +1,8 @@
 use crate::convert::From;
 use crate::fmt;
 use crate::marker::{PhantomData, Unsize};
-use crate::mem;
 use crate::ops::{CoerceUnsized, DispatchFromDyn};
+use crate::ptr::NonNull;
 
 /// A wrapper around a raw non-null `*mut T` that indicates that the possessor
 /// of this wrapper owns the referent. Useful for building abstractions like
@@ -32,9 +32,8 @@ use crate::ops::{CoerceUnsized, DispatchFromDyn};
 )]
 #[doc(hidden)]
 #[repr(transparent)]
-#[rustc_layout_scalar_valid_range_start(1)]
 pub struct Unique<T: ?Sized> {
-    pointer: *const T,
+    pointer: NonNull<T>,
     // NOTE: this marker has no consequences for variance, but is necessary
     // for dropck to understand that we logically own a `T`.
     //
@@ -71,9 +70,7 @@ impl<T: Sized> Unique<T> {
     #[must_use]
     #[inline]
     pub const fn dangling() -> Self {
-        // SAFETY: mem::align_of() returns a valid, non-null pointer. The
-        // conditions to call new_unchecked() are thus respected.
-        unsafe { Unique::new_unchecked(crate::ptr::invalid_mut::<T>(mem::align_of::<T>())) }
+        Self::from(NonNull::dangling())
     }
 }
 
@@ -87,15 +84,14 @@ impl<T: ?Sized> Unique<T> {
     #[inline]
     pub const unsafe fn new_unchecked(ptr: *mut T) -> Self {
         // SAFETY: the caller must guarantee that `ptr` is non-null.
-        unsafe { Unique { pointer: ptr as _, _marker: PhantomData } }
+        unsafe { Unique { pointer: NonNull::new_unchecked(ptr), _marker: PhantomData } }
     }
 
     /// Creates a new `Unique` if `ptr` is non-null.
     #[inline]
     pub const fn new(ptr: *mut T) -> Option<Self> {
-        if !ptr.is_null() {
-            // SAFETY: The pointer has already been checked and is not null.
-            Some(unsafe { Unique { pointer: ptr as _, _marker: PhantomData } })
+        if let Some(pointer) = NonNull::new(ptr) {
+            Some(Unique { pointer, _marker: PhantomData })
         } else {
             None
         }
@@ -105,7 +101,7 @@ impl<T: ?Sized> Unique<T> {
     #[must_use = "`self` will be dropped if the result is not used"]
     #[inline]
     pub const fn as_ptr(self) -> *mut T {
-        self.pointer as *mut T
+        self.pointer.as_ptr()
     }
 
     /// Dereferences the content.
@@ -118,7 +114,7 @@ impl<T: ?Sized> Unique<T> {
     pub const unsafe fn as_ref(&self) -> &T {
         // SAFETY: the caller must guarantee that `self` meets all the
         // requirements for a reference.
-        unsafe { &*self.as_ptr() }
+        unsafe { self.pointer.as_ref() }
     }
 
     /// Mutably dereferences the content.
@@ -131,17 +127,14 @@ impl<T: ?Sized> Unique<T> {
     pub const unsafe fn as_mut(&mut self) -> &mut T {
         // SAFETY: the caller must guarantee that `self` meets all the
         // requirements for a mutable reference.
-        unsafe { &mut *self.as_ptr() }
+        unsafe { self.pointer.as_mut() }
     }
 
     /// Casts to a pointer of another type.
     #[must_use = "`self` will be dropped if the result is not used"]
     #[inline]
     pub const fn cast<U>(self) -> Unique<U> {
-        // SAFETY: Unique::new_unchecked() creates a new unique and needs
-        // the given pointer to not be null.
-        // Since we are passing self as a pointer, it cannot be null.
-        unsafe { Unique::new_unchecked(self.as_ptr() as *mut U) }
+        Unique::from(self.pointer.cast())
     }
 }
 
@@ -184,7 +177,17 @@ impl<T: ?Sized> const From<&mut T> for Unique<T> {
     /// This conversion is infallible since references cannot be null.
     #[inline]
     fn from(reference: &mut T) -> Self {
-        // SAFETY: A mutable reference cannot be null
-        unsafe { Unique { pointer: reference as *mut T, _marker: PhantomData } }
+        Self::from(NonNull::from(reference))
+    }
+}
+
+#[unstable(feature = "ptr_internals", issue = "none")]
+impl<T: ?Sized> const From<NonNull<T>> for Unique<T> {
+    /// Converts a `NonNull<T>` to a `Unique<T>`.
+    ///
+    /// This conversion is infallible since `NonNull` cannot be null.
+    #[inline]
+    fn from(pointer: NonNull<T>) -> Self {
+        Unique { pointer, _marker: PhantomData }
     }
 }
diff --git a/src/etc/gdb_providers.py b/src/etc/gdb_providers.py
@@ -12,8 +12,10 @@
 def unwrap_unique_or_non_null(unique_or_nonnull):
     # BACKCOMPAT: rust 1.32
     # https://github.com/rust-lang/rust/commit/7a0911528058e87d22ea305695f4047572c5e067
+    # BACKCOMPAT: rust 1.60
+    # https://github.com/rust-lang/rust/commit/2a91eeac1a2d27dd3de1bf55515d765da20fd86f
     ptr = unique_or_nonnull["pointer"]
-    return ptr if ptr.type.code == gdb.TYPE_CODE_PTR else ptr[ZERO_FIELD]
+    return ptr if ptr.type.code == gdb.TYPE_CODE_PTR else ptr[ptr.type.fields()[0]]
 
 
 class EnumProvider:

diff --git a/src/etc/lldb_providers.py b/src/etc/lldb_providers.py
@@ -63,6 +63,8 @@ def from_uint(self, name, value):
 def unwrap_unique_or_non_null(unique_or_nonnull):
     # BACKCOMPAT: rust 1.32
     # https://github.com/rust-lang/rust/commit/7a0911528058e87d22ea305695f4047572c5e067
+    # BACKCOMPAT: rust 1.60
+    # https://github.com/rust-lang/rust/commit/2a91eeac1a2d27dd3de1bf55515d765da20fd86f
     ptr = unique_or_nonnull.GetChildMemberWithName("pointer")
     return ptr if ptr.TypeIsPointerType() else ptr.GetChildAtIndex(0)
 
@@ -268,7 +270,9 @@ class StdVecSyntheticProvider:
     struct RawVec<T> { ptr: Unique<T>, cap: usize, ... }
     rust 1.31.1: struct Unique<T: ?Sized> { pointer: NonZero<*const T>, ... }
     rust 1.33.0: struct Unique<T: ?Sized> { pointer: *const T, ... }
+    rust 1.62.0: struct Unique<T: ?Sized> { pointer: NonNull<T>, ... }
     struct NonZero<T>(T)
+    struct NonNull<T> { pointer: *const T }
     """
 
     def __init__(self, valobj, dict):

diff --git a/src/etc/natvis/liballoc.natvis b/src/etc/natvis/liballoc.natvis
@@ -7,7 +7,7 @@
       <Item Name="[capacity]" ExcludeView="simple">buf.cap</Item>
       <ArrayItems>
         <Size>len</Size>
-        <ValuePointer>buf.ptr.pointer</ValuePointer>
+        <ValuePointer>buf.ptr.pointer.pointer</ValuePointer>
       </ArrayItems>
     </Expand>
   </Type>
@@ -24,7 +24,7 @@
           <If Condition="i == head">
             <Break/>
           </If>
-          <Item>buf.ptr.pointer[i]</Item>
+          <Item>buf.ptr.pointer.pointer[i]</Item>
           <Exec>i = (i + 1 == buf.cap ? 0 : i + 1)</Exec>
         </Loop>
       </CustomListItems>
@@ -42,17 +42,17 @@
     </Expand>
   </Type>
   <Type Name="alloc::string::String">
-    <DisplayString>{(char*)vec.buf.ptr.pointer,[vec.len]s8}</DisplayString>
-    <StringView>(char*)vec.buf.ptr.pointer,[vec.len]s8</StringView>
+    <DisplayString>{(char*)vec.buf.ptr.pointer.pointer,[vec.len]s8}</DisplayString>
+    <StringView>(char*)vec.buf.ptr.pointer.pointer,[vec.len]s8</StringView>
     <Expand>
       <Item Name="[len]" ExcludeView="simple">vec.len</Item>
       <Item Name="[capacity]" ExcludeView="simple">vec.buf.cap</Item>
       <Synthetic Name="[chars]">
-        <DisplayString>{(char*)vec.buf.ptr.pointer,[vec.len]s8}</DisplayString>
+        <DisplayString>{(char*)vec.buf.ptr.pointer.pointer,[vec.len]s8}</DisplayString>
         <Expand>
           <ArrayItems>
             <Size>vec.len</Size>
-            <ValuePointer>(char*)vec.buf.ptr.pointer</ValuePointer>
+            <ValuePointer>(char*)vec.buf.ptr.pointer.pointer</ValuePointer>
           </ArrayItems>
         </Expand>
       </Synthetic>

diff --git a/src/etc/natvis/libcore.natvis b/src/etc/natvis/libcore.natvis
@@ -113,7 +113,7 @@
   </Type>
 
   <Type Name="core::ptr::unique::Unique&lt;*&gt;">
-    <DisplayString>Unique({(void*)pointer}: {pointer})</DisplayString>
+    <DisplayString>Unique({(void*)pointer.pointer}: {pointer.pointer})</DisplayString>
     <Expand>
       <ExpandedItem>pointer</ExpandedItem>
     </Expand>

diff --git a/src/etc/natvis/libstd.natvis b/src/etc/natvis/libstd.natvis
@@ -104,14 +104,14 @@
   </Type>
 
   <Type Name="std::ffi::os_str::OsString">
-    <DisplayString>{(char*)inner.inner.bytes.buf.ptr.pointer,[inner.inner.bytes.len]}</DisplayString>
+    <DisplayString>{(char*)inner.inner.bytes.buf.ptr.pointer.pointer,[inner.inner.bytes.len]}</DisplayString>
     <Expand>
       <Synthetic Name="[chars]">
-        <DisplayString>{(char*)inner.inner.bytes.buf.ptr.pointer,[inner.inner.bytes.len]}</DisplayString>
+        <DisplayString>{(char*)inner.inner.bytes.buf.ptr.pointer.pointer,[inner.inner.bytes.len]}</DisplayString>
         <Expand>
           <ArrayItems>
             <Size>inner.inner.bytes.len</Size>
-            <ValuePointer>(char*)inner.inner.bytes.buf.ptr.pointer</ValuePointer>
+            <ValuePointer>(char*)inner.inner.bytes.buf.ptr.pointer.pointer</ValuePointer>
           </ArrayItems>
         </Expand>
       </Synthetic>

diff --git a/src/test/mir-opt/inline/inline_into_box_place.main.Inline.32bit.diff b/src/test/mir-opt/inline/inline_into_box_place.main.Inline.32bit.diff
@@ -35,7 +35,7 @@
 +         StorageLive(_7);                 // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
 +         _7 = &mut (*_5);                 // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
 +         Deinit((*_7));                   // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
-+         ((*_7).0: alloc::raw_vec::RawVec<u32>) = const alloc::raw_vec::RawVec::<u32> { ptr: Unique::<u32> { pointer: {0x4 as *const u32}, _marker: PhantomData::<u32> }, cap: 0_usize, alloc: std::alloc::Global }; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
++         ((*_7).0: alloc::raw_vec::RawVec<u32>) = const alloc::raw_vec::RawVec::<u32> { ptr: Unique::<u32> { pointer: NonNull::<u32> { pointer: {0x4 as *const u32} }, _marker: PhantomData::<u32> }, cap: 0_usize, alloc: std::alloc::Global }; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
                                            // mir::Constant
 -                                          // + span: $DIR/inline-into-box-place.rs:8:33: 8:41
 -                                          // + user_ty: UserType(1)

diff --git a/src/test/mir-opt/inline/inline_into_box_place.main.Inline.64bit.diff b/src/test/mir-opt/inline/inline_into_box_place.main.Inline.64bit.diff
@@ -35,7 +35,7 @@
 +         StorageLive(_7);                 // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
 +         _7 = &mut (*_5);                 // scope 0 at $DIR/inline-into-box-place.rs:8:33: 8:43
 +         Deinit((*_7));                   // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
-+         ((*_7).0: alloc::raw_vec::RawVec<u32>) = const alloc::raw_vec::RawVec::<u32> { ptr: Unique::<u32> { pointer: {0x4 as *const u32}, _marker: PhantomData::<u32> }, cap: 0_usize, alloc: std::alloc::Global }; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
++         ((*_7).0: alloc::raw_vec::RawVec<u32>) = const alloc::raw_vec::RawVec::<u32> { ptr: Unique::<u32> { pointer: NonNull::<u32> { pointer: {0x4 as *const u32} }, _marker: PhantomData::<u32> }, cap: 0_usize, alloc: std::alloc::Global }; // scope 3 at $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
                                            // mir::Constant
 -                                          // + span: $DIR/inline-into-box-place.rs:8:33: 8:41
 -                                          // + user_ty: UserType(1)

diff --git a/src/test/ui/lint/lint-ctypes-enum.rs b/src/test/ui/lint/lint-ctypes-enum.rs
@@ -67,7 +67,6 @@ extern "C" {
    fn option_fn(x: Option<extern "C" fn()>);
    fn nonnull(x: Option<std::ptr::NonNull<u8>>);
    fn unique(x: Option<std::ptr::Unique<u8>>);
-   //~^ ERROR `extern` block uses type `Option<Unique<u8>>`
    fn nonzero_u8(x: Option<num::NonZeroU8>);
    fn nonzero_u16(x: Option<num::NonZeroU16>);
    fn nonzero_u32(x: Option<num::NonZeroU32>);

diff --git a/src/test/ui/lint/lint-ctypes-enum.stderr b/src/test/ui/lint/lint-ctypes-enum.stderr
@@ -54,33 +54,24 @@ LL | |     G,
 LL | | }
    | |_^
 
-error: `extern` block uses type `Option<Unique<u8>>`, which is not FFI-safe
-  --> $DIR/lint-ctypes-enum.rs:69:17
-   |
-LL |    fn unique(x: Option<std::ptr::Unique<u8>>);
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
-   |
-   = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
-   = note: enum has no representation hint
-
 error: `extern` block uses type `u128`, which is not FFI-safe
-  --> $DIR/lint-ctypes-enum.rs:75:23
+  --> $DIR/lint-ctypes-enum.rs:74:23
    |
 LL |    fn nonzero_u128(x: Option<num::NonZeroU128>);
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
    |
    = note: 128-bit integers don't currently have a known stable ABI
 
 error: `extern` block uses type `i128`, which is not FFI-safe
-  --> $DIR/lint-ctypes-enum.rs:82:23
+  --> $DIR/lint-ctypes-enum.rs:81:23
    |
 LL |    fn nonzero_i128(x: Option<num::NonZeroI128>);
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
    |
    = note: 128-bit integers don't currently have a known stable ABI
 
 error: `extern` block uses type `Option<TransparentUnion<NonZeroU8>>`, which is not FFI-safe
-  --> $DIR/lint-ctypes-enum.rs:87:28
+  --> $DIR/lint-ctypes-enum.rs:86:28
    |
 LL |    fn transparent_union(x: Option<TransparentUnion<num::NonZeroU8>>);
    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
@@ -89,7 +80,7 @@ LL |    fn transparent_union(x: Option<TransparentUnion<num::NonZeroU8>>);
    = note: enum has no representation hint
 
 error: `extern` block uses type `Option<Rust<NonZeroU8>>`, which is not FFI-safe
-  --> $DIR/lint-ctypes-enum.rs:89:20
+  --> $DIR/lint-ctypes-enum.rs:88:20
    |
 LL |    fn repr_rust(x: Option<Rust<num::NonZeroU8>>);
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
@@ -98,13 +89,13 @@ LL |    fn repr_rust(x: Option<Rust<num::NonZeroU8>>);
    = note: enum has no representation hint
 
 error: `extern` block uses type `Result<(), NonZeroI32>`, which is not FFI-safe
-  --> $DIR/lint-ctypes-enum.rs:90:20
+  --> $DIR/lint-ctypes-enum.rs:89:20
    |
 LL |    fn no_result(x: Result<(), num::NonZeroI32>);
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
    |
    = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
    = note: enum has no representation hint
 
-error: aborting due to 9 previous errors
+error: aborting due to 8 previous errors
 
diff --git a/src/test/ui/traits/cycle-cache-err-60010.stderr b/src/test/ui/traits/cycle-cache-err-60010.stderr
@@ -4,7 +4,7 @@ error[E0275]: overflow evaluating the requirement `SalsaStorage: RefUnwindSafe`
 LL |     _parse: <ParseQuery as Query<RootDatabase>>::Data,
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: required because it appears within the type `*const SalsaStorage`
+   = note: required because it appears within the type `PhantomData<SalsaStorage>`
    = note: required because it appears within the type `Unique<SalsaStorage>`
    = note: required because it appears within the type `Box<SalsaStorage>`
 note: required because it appears within the type `Runtime<RootDatabase>`