Skip to content

Implement core::ptr::Unique on top of NonNull #96010

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

Merged
merged 7 commits into from
Apr 17, 2022
Merged
Show file tree
Hide file tree
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
8 changes: 6 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,14 @@
#![feature(const_likely)]
#![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)]
Expand Down
41 changes: 22 additions & 19 deletions library/core/src/ptr/unique.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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`.
//
Expand Down Expand Up @@ -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())
}
}

Expand All @@ -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
}
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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())
}
}

Expand Down Expand Up @@ -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 }
}
}
4 changes: 3 additions & 1 deletion src/etc/gdb_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 4 additions & 0 deletions src/etc/lldb_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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):
Expand Down
12 changes: 6 additions & 6 deletions src/etc/natvis/liballoc.natvis
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion src/etc/natvis/libcore.natvis
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
6 changes: 3 additions & 3 deletions src/etc/natvis/libstd.natvis
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/lint/lint-ctypes-enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>);
Expand Down
21 changes: 6 additions & 15 deletions src/test/ui/lint/lint-ctypes-enum.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

2 changes: 1 addition & 1 deletion src/test/ui/traits/cycle-cache-err-60010.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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>`
Expand Down