Skip to content

Prepare NonNull for pattern types #152702

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.

Sign up for GitHub

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

Open
oli-obk wants to merge 1 commit into
base: main
Choose a base branch
from
+913 −928
Open

Prepare NonNull for pattern types #152702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 12 additions & 13 deletions library/core/src/ptr/non_null.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::clone::TrivialClone;
use crate::cmp::Ordering;
use crate::marker::{Destruct, PointeeSized, Unsize};
use crate::mem::{MaybeUninit, SizedTypeProperties};
use crate::mem::{MaybeUninit, SizedTypeProperties, transmute};
use crate::num::NonZero;
use crate::ops::{CoerceUnsized, DispatchFromDyn};
use crate::pin::PinCoerceUnsized;
Expand Down Expand Up @@ -100,9 +100,8 @@ impl<T: Sized> NonNull<T> {
#[must_use]
#[inline]
pub const fn without_provenance(addr: NonZero<usize>) -> Self {
let pointer = crate::ptr::without_provenance(addr.get());
// SAFETY: we know `addr` is non-zero.
unsafe { NonNull { pointer } }
// SAFETY: we know `addr` is non-zero and all nonzero integers are valid raw pointers.
unsafe { transmute(addr) }
}

/// Creates a new `NonNull` that is dangling, but well-aligned.
Expand Down Expand Up @@ -239,7 +238,7 @@ impl<T: PointeeSized> NonNull<T> {
"NonNull::new_unchecked requires that the pointer is non-null",
(ptr: *mut () = ptr as *mut ()) => !ptr.is_null()
);
NonNull { pointer: ptr as _ }
transmute(ptr)
}
}

Expand Down Expand Up @@ -282,7 +281,7 @@ impl<T: PointeeSized> NonNull<T> {
#[inline]
pub const fn from_ref(r: &T) -> Self {
// SAFETY: A reference cannot be null.
unsafe { NonNull { pointer: r as *const T } }
unsafe { transmute(r as *const T) }
}

/// Converts a mutable reference to a `NonNull` pointer.
Expand All @@ -291,7 +290,7 @@ impl<T: PointeeSized> NonNull<T> {
#[inline]
pub const fn from_mut(r: &mut T) -> Self {
// SAFETY: A mutable reference cannot be null.
unsafe { NonNull { pointer: r as *mut T } }
unsafe { transmute(r as *mut T) }
}

/// Performs the same functionality as [`std::ptr::from_raw_parts`], except that a
Expand Down Expand Up @@ -502,7 +501,7 @@ impl<T: PointeeSized> NonNull<T> {
#[inline]
pub const fn cast<U>(self) -> NonNull<U> {
// SAFETY: `self` is a `NonNull` pointer which is necessarily non-null
unsafe { NonNull { pointer: self.as_ptr() as *mut U } }
unsafe { transmute(self.as_ptr() as *mut U) }
}

/// Try to cast to a pointer of another type by checking alignment.
Expand Down Expand Up @@ -581,7 +580,7 @@ impl<T: PointeeSized> 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.as_ptr(), count) } }
unsafe { transmute(intrinsics::offset(self.as_ptr(), count)) }
}

/// Calculates the offset from a pointer in bytes.
Expand All @@ -605,7 +604,7 @@ impl<T: PointeeSized> 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.as_ptr().byte_offset(count) } }
unsafe { transmute(self.as_ptr().byte_offset(count)) }
}

/// Adds an offset to a pointer (convenience for `.offset(count as isize)`).
Expand Down Expand Up @@ -657,7 +656,7 @@ impl<T: PointeeSized> 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.as_ptr(), count) } }
unsafe { transmute(intrinsics::offset(self.as_ptr(), count)) }
}

/// Calculates the offset from a pointer in bytes (convenience for `.byte_offset(count as isize)`).
Expand All @@ -681,7 +680,7 @@ impl<T: PointeeSized> 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.as_ptr().byte_add(count) } }
unsafe { transmute(self.as_ptr().byte_add(count)) }
}

/// Subtracts an offset from a pointer (convenience for
Expand Down Expand Up @@ -763,7 +762,7 @@ impl<T: PointeeSized> 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.as_ptr().byte_sub(count) } }
unsafe { transmute(self.as_ptr().byte_sub(count)) }
}

/// Calculates the distance between two pointers within the same allocation. The returned value is in
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen-llvm/loads.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub fn load_raw_pointer<'a>(x: &*const i32) -> *const i32 {
// CHECK-LABEL: @load_box
#[no_mangle]
pub fn load_box<'a>(x: Box<Box<i32>>) -> Box<i32> {
// CHECK: load ptr, ptr %{{.*}}, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_4_META]], !noundef !{{[0-9]+}}
// CHECK: load ptr, ptr %{{.*}}, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !noundef !{{[0-9]+}}
Copy link
Member

@scottmcm scottmcm Feb 17, 2026

Choose a reason for hiding this comment

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

Hmm, if this is showing up here with just the nonnull change it looks like whatever emits this !align needs an update to see through pattern types, maybe?

Copy link
Contributor Author

@oli-obk oli-obk Feb 17, 2026

Choose a reason for hiding this comment

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

This PR doesn't use pattern types yet 😅 so the issue is likely that we lose the information somewhere in the transmute. Lemme try your other suggestion, which maybe LLVM can preserve things if things are simpler

Copy link
Member

@scottmcm scottmcm Feb 17, 2026

Choose a reason for hiding this comment

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

Oh, doh, of course it doesn't use pattern types 🤦

Note that this is a -C no-prepopulate-passes test, though, so it's only testing things that we in cg_llvm actually emit for the loads. So this implies that it's loading as a pointer instead of as a Box, somehow, I guess?

(I was going to say "well if it's just llvm adding this maybe I'm not worried about it", but if it's us in cg_llvm adding it then presumably it's worth trying to keep it working.)

Copy link
Member

@scottmcm scottmcm Feb 17, 2026

Choose a reason for hiding this comment

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

Hmm, thinking more about what you said about the transmute, I wonder if this is something I regressed a long time ago with moving transmute to being primitive instead of always going through memory. Box<Box<i32>> and Box<i32> are both BackendAbi::Scalar, so maybe something is passed through a transmute now instead of being loaded from an alloca as the Box which would be where the !align on the load would be coming from...

I think that pushes me to just r=me for this stuff, as if it's an existing "oh this combination isn't great" that's not this PR's problem, and we can reoptimize it later as needed.

Copy link
Contributor Author

@oli-obk oli-obk Feb 19, 2026

Choose a reason for hiding this comment

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

Yea, seems like there is quite some space to expwriment with. I thought about creating an issue, but it seems too vague to be very useful

*x
}

Expand Down
1 change: 1 addition & 0 deletions tests/codegen-units/item-collection/opaque-return-impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,4 @@ pub fn foo3() -> Box<dyn Iterator<Item = usize>> {
//~ MONO_ITEM fn std::boxed::Box::<Counter>::new
//~ MONO_ITEM fn Counter::new
//~ MONO_ITEM fn std::fmt::Arguments::<'_>::from_str
//~ MONO_ITEM fn std::boxed::box_new_uninit
12 changes: 0 additions & 12 deletions ...opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined std::ptr::without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
Expand All @@ -43,10 +34,7 @@
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_6);
_4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
_3 = const std::ptr::Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }};
Expand Down
12 changes: 0 additions & 12 deletions ...pt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined std::ptr::without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
Expand All @@ -43,10 +34,7 @@
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_6);
_4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
_3 = const std::ptr::Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }};
Expand Down
12 changes: 0 additions & 12 deletions ...opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined std::ptr::without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
Expand All @@ -43,10 +34,7 @@
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_6);
_4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
_3 = const std::ptr::Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }};
Expand Down
12 changes: 0 additions & 12 deletions ...pt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined std::ptr::without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
Expand All @@ -43,10 +34,7 @@
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_6 = const {0x1 as *const [bool; 0]};
_5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_6);
_4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
_3 = const std::ptr::Unique::<[bool]> {{ pointer: NonNull::<[bool]> {{ pointer: Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: *const [bool] }}, _marker: PhantomData::<[bool]> }};
Expand Down
17 changes: 2 additions & 15 deletions tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined std::ptr::without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
Expand All @@ -43,13 +34,9 @@
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
- _6 = const <[bool; 0] as std::mem::SizedTypeProperties>::ALIGNMENT as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _6 };
+ _6 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_6);
- _5 = const <[bool; 0] as std::mem::SizedTypeProperties>::ALIGNMENT as std::ptr::NonNull<[bool; 0]> (Transmute);
- _4 = std::ptr::Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> };
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
+ _4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
- _3 = move _4 as std::ptr::Unique<[bool]> (PointerCoercion(Unsize, Implicit));
Expand Down
17 changes: 2 additions & 15 deletions tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined std::ptr::without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
Expand All @@ -43,13 +34,9 @@
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
- _6 = const <[bool; 0] as std::mem::SizedTypeProperties>::ALIGNMENT as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _6 };
+ _6 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_6);
- _5 = const <[bool; 0] as std::mem::SizedTypeProperties>::ALIGNMENT as std::ptr::NonNull<[bool; 0]> (Transmute);
- _4 = std::ptr::Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> };
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
+ _4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
- _3 = move _4 as std::ptr::Unique<[bool]> (PointerCoercion(Unsize, Implicit));
Expand Down
17 changes: 2 additions & 15 deletions tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined std::ptr::without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
Expand All @@ -43,13 +34,9 @@
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
- _6 = const <[bool; 0] as std::mem::SizedTypeProperties>::ALIGNMENT as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _6 };
+ _6 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_6);
- _5 = const <[bool; 0] as std::mem::SizedTypeProperties>::ALIGNMENT as std::ptr::NonNull<[bool; 0]> (Transmute);
- _4 = std::ptr::Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> };
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
+ _4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
- _3 = move _4 as std::ptr::Unique<[bool]> (PointerCoercion(Unsize, Implicit));
Expand Down
17 changes: 2 additions & 15 deletions tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,6 @@
scope 8 (inlined std::ptr::Alignment::as_nonzero) {
}
scope 9 (inlined NonNull::<[bool; 0]>::without_provenance) {
let _6: *const [bool; 0];
scope 10 {
}
scope 11 (inlined NonZero::<usize>::get) {
}
scope 12 (inlined std::ptr::without_provenance::<[bool; 0]>) {
scope 13 (inlined without_provenance_mut::<[bool; 0]>) {
}
}
}
}
scope 7 (inlined std::ptr::Alignment::of::<[bool; 0]>) {
Expand All @@ -43,13 +34,9 @@
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
- _6 = const <[bool; 0] as std::mem::SizedTypeProperties>::ALIGNMENT as *const [bool; 0] (Transmute);
- _5 = NonNull::<[bool; 0]> { pointer: copy _6 };
+ _6 = const {0x1 as *const [bool; 0]};
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
StorageDead(_6);
- _5 = const <[bool; 0] as std::mem::SizedTypeProperties>::ALIGNMENT as std::ptr::NonNull<[bool; 0]> (Transmute);
- _4 = std::ptr::Unique::<[bool; 0]> { pointer: move _5, _marker: const PhantomData::<[bool; 0]> };
+ _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
+ _4 = const std::ptr::Unique::<[bool; 0]> {{ pointer: NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }}, _marker: PhantomData::<[bool; 0]> }};
StorageDead(_5);
- _3 = move _4 as std::ptr::Unique<[bool]> (PointerCoercion(Unsize, Implicit));
Expand Down
Loading
Loading