From c738dcc284c17addbc0a311c29b4ecaaf2fcfbaa Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 11 Apr 2023 19:18:59 +0000 Subject: [PATCH 01/22] Add `bits_for` helper for tagged pointers & fixup docs --- .../rustc_data_structures/src/tagged_ptr.rs | 55 ++++++++++++++----- compiler/rustc_middle/src/ty/list.rs | 3 +- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index 651bc556c9853..c96d7835b2bee 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -24,32 +24,45 @@ mod drop; pub use copy::CopyTaggedPtr; pub use drop::TaggedPtr; -/// This describes the pointer type encapsulated by TaggedPtr. +/// This describes the pointer type encapsulated by [`TaggedPtr`] and +/// [`CopyTaggedPtr`]. /// /// # Safety /// /// The usize returned from `into_usize` must be a valid, dereferenceable, -/// pointer to `::Target`. Note that pointers to `Pointee` must -/// be thin, even though `Pointee` may not be sized. +/// pointer to [`::Target`]. Note that pointers to +/// [`Self::Target`] must be thin, even though [`Self::Target`] may not be +/// `Sized`. /// /// Note that the returned pointer from `into_usize` should be castable to `&mut -/// ::Target` if `Pointer: DerefMut`. +/// ::Target` if `Self: DerefMut`. /// /// The BITS constant must be correct. At least `BITS` bits, least-significant, /// must be zero on all returned pointers from `into_usize`. /// -/// For example, if the alignment of `Pointee` is 2, then `BITS` should be 1. +/// For example, if the alignment of [`Self::Target`] is 2, then `BITS` should be 1. +/// +/// [`::Target`]: Deref::Target +/// [`Self::Target`]: Deref::Target pub unsafe trait Pointer: Deref { + /// Number of unused (always zero) **least significant bits** in this + /// pointer, usually related to the pointees alignment. + /// /// Most likely the value you want to use here is the following, unless - /// your Pointee type is unsized (e.g., `ty::List` in rustc) in which - /// case you'll need to manually figure out what the right type to pass to - /// align_of is. + /// your [`Self::Target`] type is unsized (e.g., `ty::List` in rustc) + /// or your pointer is over/under aligned, in which case you'll need to + /// manually figure out what the right type to pass to [`bits_for`] is, or + /// what the value to set here. /// - /// ```ignore UNSOLVED (what to do about the Self) + /// ```rust /// # use std::ops::Deref; - /// std::mem::align_of::<::Target>().trailing_zeros() as usize; + /// # type Self = &'static u64; + /// bits_for::() /// ``` + /// + /// [`Self::Target`]: Deref::Target const BITS: usize; + fn into_usize(self) -> usize; /// # Safety @@ -90,7 +103,7 @@ pub unsafe trait Tag: Copy { } unsafe impl Pointer for Box { - const BITS: usize = std::mem::align_of::().trailing_zeros() as usize; + const BITS: usize = bits_for::(); #[inline] fn into_usize(self) -> usize { Box::into_raw(self) as usize @@ -106,7 +119,7 @@ unsafe impl Pointer for Box { } unsafe impl Pointer for Rc { - const BITS: usize = std::mem::align_of::().trailing_zeros() as usize; + const BITS: usize = bits_for::(); #[inline] fn into_usize(self) -> usize { Rc::into_raw(self) as usize @@ -122,7 +135,7 @@ unsafe impl Pointer for Rc { } unsafe impl Pointer for Arc { - const BITS: usize = std::mem::align_of::().trailing_zeros() as usize; + const BITS: usize = bits_for::(); #[inline] fn into_usize(self) -> usize { Arc::into_raw(self) as usize @@ -138,7 +151,7 @@ unsafe impl Pointer for Arc { } unsafe impl<'a, T: 'a> Pointer for &'a T { - const BITS: usize = std::mem::align_of::().trailing_zeros() as usize; + const BITS: usize = bits_for::(); #[inline] fn into_usize(self) -> usize { self as *const T as usize @@ -153,7 +166,7 @@ unsafe impl<'a, T: 'a> Pointer for &'a T { } unsafe impl<'a, T: 'a> Pointer for &'a mut T { - const BITS: usize = std::mem::align_of::().trailing_zeros() as usize; + const BITS: usize = bits_for::(); #[inline] fn into_usize(self) -> usize { self as *mut T as usize @@ -166,3 +179,15 @@ unsafe impl<'a, T: 'a> Pointer for &'a mut T { f(&*(&ptr as *const usize as *const Self)) } } + +/// Returns the number of bits available for use for tags in a pointer to `T` +/// (this is based on `T`'s alignment). +pub const fn bits_for() -> usize { + let bits = std::mem::align_of::().trailing_zeros(); + + // This is a replacement for `.try_into().unwrap()` unavailable in `const` + // (it's fine to make an assert here, since this is only called in compile time) + assert!((bits as u128) < usize::MAX as u128); + + bits as usize +} diff --git a/compiler/rustc_middle/src/ty/list.rs b/compiler/rustc_middle/src/ty/list.rs index 79365ef281be7..4526487cf1d53 100644 --- a/compiler/rustc_middle/src/ty/list.rs +++ b/compiler/rustc_middle/src/ty/list.rs @@ -1,4 +1,5 @@ use crate::arena::Arena; +use rustc_data_structures::tagged_ptr::bits_for; use rustc_serialize::{Encodable, Encoder}; use std::alloc::Layout; use std::cmp::Ordering; @@ -199,7 +200,7 @@ impl<'a, T: Copy> IntoIterator for &'a List { unsafe impl Sync for List {} unsafe impl<'a, T: 'a> rustc_data_structures::tagged_ptr::Pointer for &'a List { - const BITS: usize = std::mem::align_of::().trailing_zeros() as usize; + const BITS: usize = bits_for::(); #[inline] fn into_usize(self) -> usize { From f028636b1aec09366973468e8c347c1cf8291561 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 11 Apr 2023 19:21:43 +0000 Subject: [PATCH 02/22] Sprinkle some whitespace & uses --- compiler/rustc_data_structures/src/tagged_ptr.rs | 16 ++++++++++++++-- .../rustc_data_structures/src/tagged_ptr/copy.rs | 15 +++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index c96d7835b2bee..e69a11dae0a9d 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -13,7 +13,7 @@ //! The tag must implement the `Tag` trait. We assert that the tag and `Pointer` //! are compatible at compile time. -use std::mem::ManuallyDrop; +use std::mem::{self, ManuallyDrop}; use std::ops::Deref; use std::rc::Rc; use std::sync::Arc; @@ -104,14 +104,17 @@ pub unsafe trait Tag: Copy { unsafe impl Pointer for Box { const BITS: usize = bits_for::(); + #[inline] fn into_usize(self) -> usize { Box::into_raw(self) as usize } + #[inline] unsafe fn from_usize(ptr: usize) -> Self { Box::from_raw(ptr as *mut T) } + unsafe fn with_ref R>(ptr: usize, f: F) -> R { let raw = ManuallyDrop::new(Self::from_usize(ptr)); f(&raw) @@ -120,14 +123,17 @@ unsafe impl Pointer for Box { unsafe impl Pointer for Rc { const BITS: usize = bits_for::(); + #[inline] fn into_usize(self) -> usize { Rc::into_raw(self) as usize } + #[inline] unsafe fn from_usize(ptr: usize) -> Self { Rc::from_raw(ptr as *const T) } + unsafe fn with_ref R>(ptr: usize, f: F) -> R { let raw = ManuallyDrop::new(Self::from_usize(ptr)); f(&raw) @@ -136,14 +142,17 @@ unsafe impl Pointer for Rc { unsafe impl Pointer for Arc { const BITS: usize = bits_for::(); + #[inline] fn into_usize(self) -> usize { Arc::into_raw(self) as usize } + #[inline] unsafe fn from_usize(ptr: usize) -> Self { Arc::from_raw(ptr as *const T) } + unsafe fn with_ref R>(ptr: usize, f: F) -> R { let raw = ManuallyDrop::new(Self::from_usize(ptr)); f(&raw) @@ -152,14 +161,17 @@ unsafe impl Pointer for Arc { unsafe impl<'a, T: 'a> Pointer for &'a T { const BITS: usize = bits_for::(); + #[inline] fn into_usize(self) -> usize { self as *const T as usize } + #[inline] unsafe fn from_usize(ptr: usize) -> Self { &*(ptr as *const T) } + unsafe fn with_ref R>(ptr: usize, f: F) -> R { f(&*(&ptr as *const usize as *const Self)) } @@ -183,7 +195,7 @@ unsafe impl<'a, T: 'a> Pointer for &'a mut T { /// Returns the number of bits available for use for tags in a pointer to `T` /// (this is based on `T`'s alignment). pub const fn bits_for() -> usize { - let bits = std::mem::align_of::().trailing_zeros(); + let bits = mem::align_of::().trailing_zeros(); // This is a replacement for `.try_into().unwrap()` unavailable in `const` // (it's fine to make an assert here, since this is only called in compile time) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index e1d3e0bd35a67..d0c2b91458493 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -3,6 +3,7 @@ use crate::stable_hasher::{HashStable, StableHasher}; use std::fmt; use std::marker::PhantomData; use std::num::NonZeroUsize; +use std::ops::{Deref, DerefMut}; /// A `Copy` TaggedPtr. /// @@ -73,6 +74,7 @@ where pub(super) fn pointer_raw(&self) -> usize { self.packed.get() << T::BITS } + pub fn pointer(self) -> P where P: Copy, @@ -83,21 +85,25 @@ where // P: Copy unsafe { P::from_usize(self.pointer_raw()) } } + pub fn pointer_ref(&self) -> &P::Target { // SAFETY: pointer_raw returns the original pointer unsafe { std::mem::transmute_copy(&self.pointer_raw()) } } + pub fn pointer_mut(&mut self) -> &mut P::Target where - P: std::ops::DerefMut, + P: DerefMut, { // SAFETY: pointer_raw returns the original pointer unsafe { std::mem::transmute_copy(&self.pointer_raw()) } } + #[inline] pub fn tag(&self) -> T { unsafe { T::from_usize(self.packed.get() >> Self::TAG_BIT_SHIFT) } } + #[inline] pub fn set_tag(&mut self, tag: T) { let mut packed = self.packed.get(); @@ -109,20 +115,21 @@ where } } -impl std::ops::Deref for CopyTaggedPtr +impl Deref for CopyTaggedPtr where P: Pointer, T: Tag, { type Target = P::Target; + fn deref(&self) -> &Self::Target { self.pointer_ref() } } -impl std::ops::DerefMut for CopyTaggedPtr +impl DerefMut for CopyTaggedPtr where - P: Pointer + std::ops::DerefMut, + P: Pointer + DerefMut, T: Tag, { fn deref_mut(&mut self) -> &mut Self::Target { From 3c6f4c126027aca4153ba3a6b48c6abc164ef94d Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 11 Apr 2023 21:31:23 +0000 Subject: [PATCH 03/22] Bless tagged pointers (comply to strict provenance) --- compiler/rustc_data_structures/src/lib.rs | 1 + .../rustc_data_structures/src/tagged_ptr.rs | 90 ++++++++++--------- .../src/tagged_ptr/copy.rs | 43 +++++---- .../src/tagged_ptr/drop.rs | 2 +- compiler/rustc_middle/src/ty/list.rs | 16 ++-- 5 files changed, 85 insertions(+), 67 deletions(-) diff --git a/compiler/rustc_data_structures/src/lib.rs b/compiler/rustc_data_structures/src/lib.rs index e373bd18402a8..0a3f5f32d16f7 100644 --- a/compiler/rustc_data_structures/src/lib.rs +++ b/compiler/rustc_data_structures/src/lib.rs @@ -29,6 +29,7 @@ #![feature(get_mut_unchecked)] #![feature(lint_reasons)] #![feature(unwrap_infallible)] +#![feature(strict_provenance)] #![allow(rustc::default_hash_types)] #![allow(rustc::potential_query_instability)] #![deny(rustc::untranslatable_diagnostic)] diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index e69a11dae0a9d..8ad2b2a41fdcd 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -15,6 +15,7 @@ use std::mem::{self, ManuallyDrop}; use std::ops::Deref; +use std::ptr::NonNull; use std::rc::Rc; use std::sync::Arc; @@ -29,21 +30,24 @@ pub use drop::TaggedPtr; /// /// # Safety /// -/// The usize returned from `into_usize` must be a valid, dereferenceable, -/// pointer to [`::Target`]. Note that pointers to -/// [`Self::Target`] must be thin, even though [`Self::Target`] may not be -/// `Sized`. +/// The pointer returned from [`into_ptr`] must be a [valid], pointer to +/// [`::Target`]. Note that pointers to [`Self::Target`] must be +/// thin, even though [`Self::Target`] may not be `Sized`. /// -/// Note that the returned pointer from `into_usize` should be castable to `&mut -/// ::Target` if `Self: DerefMut`. +/// Note that if `Self` implements [`DerefMut`] the pointer returned from +/// [`into_ptr`] must be valid for writes (and thus calling [`NonNull::as_mut`] +/// on it must be safe). /// -/// The BITS constant must be correct. At least `BITS` bits, least-significant, -/// must be zero on all returned pointers from `into_usize`. +/// The `BITS` constant must be correct. At least `BITS` bits, least-significant, +/// must be zero on all pointers returned from [`into_ptr`]. /// /// For example, if the alignment of [`Self::Target`] is 2, then `BITS` should be 1. /// +/// [`into_ptr`]: Pointer::into_ptr +/// [valid]: std::ptr#safety /// [`::Target`]: Deref::Target /// [`Self::Target`]: Deref::Target +/// [`DerefMut`]: std::ops::DerefMut pub unsafe trait Pointer: Deref { /// Number of unused (always zero) **least significant bits** in this /// pointer, usually related to the pointees alignment. @@ -63,7 +67,7 @@ pub unsafe trait Pointer: Deref { /// [`Self::Target`]: Deref::Target const BITS: usize; - fn into_usize(self) -> usize; + fn into_ptr(self) -> NonNull; /// # Safety /// @@ -71,7 +75,7 @@ pub unsafe trait Pointer: Deref { /// /// This acts as `ptr::read` semantically, it should not be called more than /// once on non-`Copy` `Pointer`s. - unsafe fn from_usize(ptr: usize) -> Self; + unsafe fn from_ptr(ptr: NonNull) -> Self; /// This provides a reference to the `Pointer` itself, rather than the /// `Deref::Target`. It is used for cases where we want to call methods that @@ -81,7 +85,7 @@ pub unsafe trait Pointer: Deref { /// # Safety /// /// The passed `ptr` must be returned from `into_usize`. - unsafe fn with_ref R>(ptr: usize, f: F) -> R; + unsafe fn with_ref R>(ptr: NonNull, f: F) -> R; } /// This describes tags that the `TaggedPtr` struct can hold. @@ -106,17 +110,18 @@ unsafe impl Pointer for Box { const BITS: usize = bits_for::(); #[inline] - fn into_usize(self) -> usize { - Box::into_raw(self) as usize + fn into_ptr(self) -> NonNull { + // Safety: pointers from `Box::into_raw` are valid & non-null + unsafe { NonNull::new_unchecked(Box::into_raw(self)) } } #[inline] - unsafe fn from_usize(ptr: usize) -> Self { - Box::from_raw(ptr as *mut T) + unsafe fn from_ptr(ptr: NonNull) -> Self { + Box::from_raw(ptr.as_ptr()) } - unsafe fn with_ref R>(ptr: usize, f: F) -> R { - let raw = ManuallyDrop::new(Self::from_usize(ptr)); + unsafe fn with_ref R>(ptr: NonNull, f: F) -> R { + let raw = ManuallyDrop::new(Self::from_ptr(ptr)); f(&raw) } } @@ -125,17 +130,17 @@ unsafe impl Pointer for Rc { const BITS: usize = bits_for::(); #[inline] - fn into_usize(self) -> usize { - Rc::into_raw(self) as usize + fn into_ptr(self) -> NonNull { + unsafe { NonNull::new_unchecked(Rc::into_raw(self).cast_mut()) } } #[inline] - unsafe fn from_usize(ptr: usize) -> Self { - Rc::from_raw(ptr as *const T) + unsafe fn from_ptr(ptr: NonNull) -> Self { + Rc::from_raw(ptr.as_ptr()) } - unsafe fn with_ref R>(ptr: usize, f: F) -> R { - let raw = ManuallyDrop::new(Self::from_usize(ptr)); + unsafe fn with_ref R>(ptr: NonNull, f: F) -> R { + let raw = ManuallyDrop::new(Self::from_ptr(ptr)); f(&raw) } } @@ -144,17 +149,17 @@ unsafe impl Pointer for Arc { const BITS: usize = bits_for::(); #[inline] - fn into_usize(self) -> usize { - Arc::into_raw(self) as usize + fn into_ptr(self) -> NonNull { + unsafe { NonNull::new_unchecked(Arc::into_raw(self).cast_mut()) } } #[inline] - unsafe fn from_usize(ptr: usize) -> Self { - Arc::from_raw(ptr as *const T) + unsafe fn from_ptr(ptr: NonNull) -> Self { + Arc::from_raw(ptr.as_ptr()) } - unsafe fn with_ref R>(ptr: usize, f: F) -> R { - let raw = ManuallyDrop::new(Self::from_usize(ptr)); + unsafe fn with_ref R>(ptr: NonNull, f: F) -> R { + let raw = ManuallyDrop::new(Self::from_ptr(ptr)); f(&raw) } } @@ -163,32 +168,35 @@ unsafe impl<'a, T: 'a> Pointer for &'a T { const BITS: usize = bits_for::(); #[inline] - fn into_usize(self) -> usize { - self as *const T as usize + fn into_ptr(self) -> NonNull { + NonNull::from(self) } #[inline] - unsafe fn from_usize(ptr: usize) -> Self { - &*(ptr as *const T) + unsafe fn from_ptr(ptr: NonNull) -> Self { + ptr.as_ref() } - unsafe fn with_ref R>(ptr: usize, f: F) -> R { - f(&*(&ptr as *const usize as *const Self)) + unsafe fn with_ref R>(ptr: NonNull, f: F) -> R { + f(&ptr.as_ref()) } } unsafe impl<'a, T: 'a> Pointer for &'a mut T { const BITS: usize = bits_for::(); + #[inline] - fn into_usize(self) -> usize { - self as *mut T as usize + fn into_ptr(self) -> NonNull { + NonNull::from(self) } + #[inline] - unsafe fn from_usize(ptr: usize) -> Self { - &mut *(ptr as *mut T) + unsafe fn from_ptr(mut ptr: NonNull) -> Self { + ptr.as_mut() } - unsafe fn with_ref R>(ptr: usize, f: F) -> R { - f(&*(&ptr as *const usize as *const Self)) + + unsafe fn with_ref R>(mut ptr: NonNull, f: F) -> R { + f(&ptr.as_mut()) } } diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index d0c2b91458493..958656f9a02b8 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -4,6 +4,7 @@ use std::fmt; use std::marker::PhantomData; use std::num::NonZeroUsize; use std::ops::{Deref, DerefMut}; +use std::ptr::NonNull; /// A `Copy` TaggedPtr. /// @@ -18,7 +19,7 @@ where P: Pointer, T: Tag, { - packed: NonZeroUsize, + packed: NonNull, data: PhantomData<(P, T)>, } @@ -53,26 +54,36 @@ where const ASSERTION: () = { assert!(T::BITS <= P::BITS); // Used for the transmute_copy's below + // TODO(waffle): do we need this assert anymore? assert!(std::mem::size_of::<&P::Target>() == std::mem::size_of::()); }; pub fn new(pointer: P, tag: T) -> Self { // Trigger assert! let () = Self::ASSERTION; + let packed_tag = tag.into_usize() << Self::TAG_BIT_SHIFT; Self { - // SAFETY: We know that the pointer is non-null, as it must be - // dereferenceable per `Pointer` safety contract. - packed: unsafe { - NonZeroUsize::new_unchecked((P::into_usize(pointer) >> T::BITS) | packed_tag) - }, + packed: P::into_ptr(pointer).map_addr(|addr| { + // SAFETY: + // - The pointer is `NonNull` => it's address is `NonZeroUsize` + // - `P::BITS` least significant bits are always zero (`Pointer` contract) + // - `T::BITS <= P::BITS` (from `Self::ASSERTION`) + // + // Thus `addr >> T::BITS` is guaranteed to be non-zero. + // + // `{non_zero} | packed_tag` can't make the value zero. + + let packed = (addr.get() >> T::BITS) | packed_tag; + unsafe { NonZeroUsize::new_unchecked(packed) } + }), data: PhantomData, } } - pub(super) fn pointer_raw(&self) -> usize { - self.packed.get() << T::BITS + pub(super) fn pointer_raw(&self) -> NonNull { + self.packed.map_addr(|addr| unsafe { NonZeroUsize::new_unchecked(addr.get() << T::BITS) }) } pub fn pointer(self) -> P @@ -83,12 +94,12 @@ where // // Note that this isn't going to double-drop or anything because we have // P: Copy - unsafe { P::from_usize(self.pointer_raw()) } + unsafe { P::from_ptr(self.pointer_raw()) } } pub fn pointer_ref(&self) -> &P::Target { // SAFETY: pointer_raw returns the original pointer - unsafe { std::mem::transmute_copy(&self.pointer_raw()) } + unsafe { self.pointer_raw().as_ref() } } pub fn pointer_mut(&mut self) -> &mut P::Target @@ -96,22 +107,22 @@ where P: DerefMut, { // SAFETY: pointer_raw returns the original pointer - unsafe { std::mem::transmute_copy(&self.pointer_raw()) } + unsafe { self.pointer_raw().as_mut() } } #[inline] pub fn tag(&self) -> T { - unsafe { T::from_usize(self.packed.get() >> Self::TAG_BIT_SHIFT) } + unsafe { T::from_usize(self.packed.addr().get() >> Self::TAG_BIT_SHIFT) } } #[inline] pub fn set_tag(&mut self, tag: T) { - let mut packed = self.packed.get(); + // TODO: refactor packing into a function and reuse it here let new_tag = T::into_usize(tag) << Self::TAG_BIT_SHIFT; let tag_mask = (1 << T::BITS) - 1; - packed &= !(tag_mask << Self::TAG_BIT_SHIFT); - packed |= new_tag; - self.packed = unsafe { NonZeroUsize::new_unchecked(packed) }; + self.packed = self.packed.map_addr(|addr| unsafe { + NonZeroUsize::new_unchecked(addr.get() & !(tag_mask << Self::TAG_BIT_SHIFT) | new_tag) + }); } } diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index b0315c93d934d..c734dadefe9be 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -76,7 +76,7 @@ where fn drop(&mut self) { // No need to drop the tag, as it's Copy unsafe { - drop(P::from_usize(self.raw.pointer_raw())); + drop(P::from_ptr(self.raw.pointer_raw())); } } } diff --git a/compiler/rustc_middle/src/ty/list.rs b/compiler/rustc_middle/src/ty/list.rs index 4526487cf1d53..01b5e82cd2266 100644 --- a/compiler/rustc_middle/src/ty/list.rs +++ b/compiler/rustc_middle/src/ty/list.rs @@ -8,7 +8,7 @@ use std::hash::{Hash, Hasher}; use std::iter; use std::mem; use std::ops::Deref; -use std::ptr; +use std::ptr::{self, NonNull}; use std::slice; /// `List` is a bit like `&[T]`, but with some critical differences. @@ -203,18 +203,16 @@ unsafe impl<'a, T: 'a> rustc_data_structures::tagged_ptr::Pointer for &'a List(); #[inline] - fn into_usize(self) -> usize { - self as *const List as usize + fn into_ptr(self) -> NonNull> { + NonNull::from(self) } #[inline] - unsafe fn from_usize(ptr: usize) -> &'a List { - &*(ptr as *const List) + unsafe fn from_ptr(ptr: NonNull>) -> &'a List { + ptr.as_ref() } - unsafe fn with_ref R>(ptr: usize, f: F) -> R { - // `Self` is `&'a List` which impls `Copy`, so this is fine. - let ptr = Self::from_usize(ptr); - f(&ptr) + unsafe fn with_ref R>(ptr: NonNull>, f: F) -> R { + f(&ptr.as_ref()) } } From 12fd610e01bbab0c58170618caf03ac7d9ddb406 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 11 Apr 2023 21:40:39 +0000 Subject: [PATCH 04/22] Refactor tagged ptr packing into a function --- .../src/tagged_ptr/copy.rs | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 958656f9a02b8..065aaa40759a4 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -50,6 +50,10 @@ where P: Pointer, T: Tag, { + pub fn new(pointer: P, tag: T) -> Self { + Self { packed: Self::pack(P::into_ptr(pointer), tag), data: PhantomData } + } + const TAG_BIT_SHIFT: usize = usize::BITS as usize - T::BITS; const ASSERTION: () = { assert!(T::BITS <= P::BITS); @@ -58,28 +62,28 @@ where assert!(std::mem::size_of::<&P::Target>() == std::mem::size_of::()); }; - pub fn new(pointer: P, tag: T) -> Self { + /// Pack pointer `ptr` that comes from [`P::into_ptr`] with a `tag`. + /// + /// [`P::into_ptr`]: Pointer::into_ptr + fn pack(ptr: NonNull, tag: T) -> NonNull { // Trigger assert! let () = Self::ASSERTION; let packed_tag = tag.into_usize() << Self::TAG_BIT_SHIFT; - Self { - packed: P::into_ptr(pointer).map_addr(|addr| { - // SAFETY: - // - The pointer is `NonNull` => it's address is `NonZeroUsize` - // - `P::BITS` least significant bits are always zero (`Pointer` contract) - // - `T::BITS <= P::BITS` (from `Self::ASSERTION`) - // - // Thus `addr >> T::BITS` is guaranteed to be non-zero. - // - // `{non_zero} | packed_tag` can't make the value zero. - - let packed = (addr.get() >> T::BITS) | packed_tag; - unsafe { NonZeroUsize::new_unchecked(packed) } - }), - data: PhantomData, - } + ptr.map_addr(|addr| { + // SAFETY: + // - The pointer is `NonNull` => it's address is `NonZeroUsize` + // - `P::BITS` least significant bits are always zero (`Pointer` contract) + // - `T::BITS <= P::BITS` (from `Self::ASSERTION`) + // + // Thus `addr >> T::BITS` is guaranteed to be non-zero. + // + // `{non_zero} | packed_tag` can't make the value zero. + + let packed = (addr.get() >> T::BITS) | packed_tag; + unsafe { NonZeroUsize::new_unchecked(packed) } + }) } pub(super) fn pointer_raw(&self) -> NonNull { @@ -117,12 +121,7 @@ where #[inline] pub fn set_tag(&mut self, tag: T) { - // TODO: refactor packing into a function and reuse it here - let new_tag = T::into_usize(tag) << Self::TAG_BIT_SHIFT; - let tag_mask = (1 << T::BITS) - 1; - self.packed = self.packed.map_addr(|addr| unsafe { - NonZeroUsize::new_unchecked(addr.get() & !(tag_mask << Self::TAG_BIT_SHIFT) | new_tag) - }); + self.packed = Self::pack(self.pointer_raw(), tag); } } From ad92677008c23655744c92951c322dc5e453e6f9 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 11 Apr 2023 21:45:19 +0000 Subject: [PATCH 05/22] Fix doc test --- compiler/rustc_data_structures/src/tagged_ptr.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index 8ad2b2a41fdcd..511566bb575d0 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -60,8 +60,12 @@ pub unsafe trait Pointer: Deref { /// /// ```rust /// # use std::ops::Deref; - /// # type Self = &'static u64; - /// bits_for::() + /// # use rustc_data_structures::tagged_ptr::bits_for; + /// # struct T; + /// # impl Deref for T { type Target = u8; fn deref(&self) -> &u8 { &0 } } + /// # impl T { + /// const BITS: usize = bits_for::<::Target>(); + /// # } /// ``` /// /// [`Self::Target`]: Deref::Target From 26232f1ff5a28f1dadd159a9604e668c55e40585 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 10:21:12 +0000 Subject: [PATCH 06/22] Remove useless parameter from ghost --- compiler/rustc_data_structures/src/tagged_ptr/copy.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 065aaa40759a4..0a2d38c21f0c1 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -20,7 +20,7 @@ where T: Tag, { packed: NonNull, - data: PhantomData<(P, T)>, + tag_ghost: PhantomData, } impl Copy for CopyTaggedPtr @@ -51,7 +51,7 @@ where T: Tag, { pub fn new(pointer: P, tag: T) -> Self { - Self { packed: Self::pack(P::into_ptr(pointer), tag), data: PhantomData } + Self { packed: Self::pack(P::into_ptr(pointer), tag), tag_ghost: PhantomData } } const TAG_BIT_SHIFT: usize = usize::BITS as usize - T::BITS; From 9051331dd751042c49119701e745463921106c8b Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 11:00:35 +0000 Subject: [PATCH 07/22] Lift `Pointer`'s requirement for the pointer to be thin fat pointers rule! --- compiler/rustc_data_structures/src/aligned.rs | 31 +++++++++++++++++++ compiler/rustc_data_structures/src/lib.rs | 1 + .../rustc_data_structures/src/tagged_ptr.rs | 21 +++++++------ .../src/tagged_ptr/copy.rs | 7 +---- compiler/rustc_middle/src/ty/list.rs | 31 +++++++++---------- 5 files changed, 58 insertions(+), 33 deletions(-) create mode 100644 compiler/rustc_data_structures/src/aligned.rs diff --git a/compiler/rustc_data_structures/src/aligned.rs b/compiler/rustc_data_structures/src/aligned.rs new file mode 100644 index 0000000000000..2d0adfe2ae35b --- /dev/null +++ b/compiler/rustc_data_structures/src/aligned.rs @@ -0,0 +1,31 @@ +use std::mem; + +/// Returns the ABI-required minimum alignment of a type in bytes. +/// +/// This is equivalent to [`mem::align_of`], but also works for some unsized +/// types (e.g. slices or rustc's `List`s). +pub const fn align_of() -> usize { + T::ALIGN +} + +/// A type with a statically known alignment. +/// +/// # Safety +/// +/// `Self::ALIGN` must be equal to the alignment of `Self`. For sized types it +/// is [`mem::align_of()`], for unsized types it depends on the type, for +/// example `[T]` has alignment of `T`. +/// +/// [`mem::align_of()`]: mem::align_of +pub unsafe trait Aligned { + /// Alignment of `Self`. + const ALIGN: usize; +} + +unsafe impl Aligned for T { + const ALIGN: usize = mem::align_of::(); +} + +unsafe impl Aligned for [T] { + const ALIGN: usize = mem::align_of::(); +} diff --git a/compiler/rustc_data_structures/src/lib.rs b/compiler/rustc_data_structures/src/lib.rs index 0a3f5f32d16f7..ea1f71d7115ca 100644 --- a/compiler/rustc_data_structures/src/lib.rs +++ b/compiler/rustc_data_structures/src/lib.rs @@ -83,6 +83,7 @@ pub mod transitive_relation; pub mod vec_linked_list; pub mod work_queue; pub use atomic_ref::AtomicRef; +pub mod aligned; pub mod frozen; pub mod owned_slice; pub mod sso; diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index 511566bb575d0..f45f5a4215645 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -13,12 +13,14 @@ //! The tag must implement the `Tag` trait. We assert that the tag and `Pointer` //! are compatible at compile time. -use std::mem::{self, ManuallyDrop}; +use std::mem::ManuallyDrop; use std::ops::Deref; use std::ptr::NonNull; use std::rc::Rc; use std::sync::Arc; +use crate::aligned::Aligned; + mod copy; mod drop; @@ -31,8 +33,7 @@ pub use drop::TaggedPtr; /// # Safety /// /// The pointer returned from [`into_ptr`] must be a [valid], pointer to -/// [`::Target`]. Note that pointers to [`Self::Target`] must be -/// thin, even though [`Self::Target`] may not be `Sized`. +/// [`::Target`]. /// /// Note that if `Self` implements [`DerefMut`] the pointer returned from /// [`into_ptr`] must be valid for writes (and thus calling [`NonNull::as_mut`] @@ -110,7 +111,7 @@ pub unsafe trait Tag: Copy { unsafe fn from_usize(tag: usize) -> Self; } -unsafe impl Pointer for Box { +unsafe impl Pointer for Box { const BITS: usize = bits_for::(); #[inline] @@ -130,7 +131,7 @@ unsafe impl Pointer for Box { } } -unsafe impl Pointer for Rc { +unsafe impl Pointer for Rc { const BITS: usize = bits_for::(); #[inline] @@ -149,7 +150,7 @@ unsafe impl Pointer for Rc { } } -unsafe impl Pointer for Arc { +unsafe impl Pointer for Arc { const BITS: usize = bits_for::(); #[inline] @@ -168,7 +169,7 @@ unsafe impl Pointer for Arc { } } -unsafe impl<'a, T: 'a> Pointer for &'a T { +unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a T { const BITS: usize = bits_for::(); #[inline] @@ -186,7 +187,7 @@ unsafe impl<'a, T: 'a> Pointer for &'a T { } } -unsafe impl<'a, T: 'a> Pointer for &'a mut T { +unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a mut T { const BITS: usize = bits_for::(); #[inline] @@ -206,8 +207,8 @@ unsafe impl<'a, T: 'a> Pointer for &'a mut T { /// Returns the number of bits available for use for tags in a pointer to `T` /// (this is based on `T`'s alignment). -pub const fn bits_for() -> usize { - let bits = mem::align_of::().trailing_zeros(); +pub const fn bits_for() -> usize { + let bits = crate::aligned::align_of::().trailing_zeros(); // This is a replacement for `.try_into().unwrap()` unavailable in `const` // (it's fine to make an assert here, since this is only called in compile time) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 0a2d38c21f0c1..09d55b20ab4de 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -55,12 +55,7 @@ where } const TAG_BIT_SHIFT: usize = usize::BITS as usize - T::BITS; - const ASSERTION: () = { - assert!(T::BITS <= P::BITS); - // Used for the transmute_copy's below - // TODO(waffle): do we need this assert anymore? - assert!(std::mem::size_of::<&P::Target>() == std::mem::size_of::()); - }; + const ASSERTION: () = { assert!(T::BITS <= P::BITS) }; /// Pack pointer `ptr` that comes from [`P::into_ptr`] with a `tag`. /// diff --git a/compiler/rustc_middle/src/ty/list.rs b/compiler/rustc_middle/src/ty/list.rs index 01b5e82cd2266..590beef7f7d42 100644 --- a/compiler/rustc_middle/src/ty/list.rs +++ b/compiler/rustc_middle/src/ty/list.rs @@ -1,5 +1,5 @@ use crate::arena::Arena; -use rustc_data_structures::tagged_ptr::bits_for; +use rustc_data_structures::aligned::Aligned; use rustc_serialize::{Encodable, Encoder}; use std::alloc::Layout; use std::cmp::Ordering; @@ -8,7 +8,7 @@ use std::hash::{Hash, Hasher}; use std::iter; use std::mem; use std::ops::Deref; -use std::ptr::{self, NonNull}; +use std::ptr; use std::slice; /// `List` is a bit like `&[T]`, but with some critical differences. @@ -199,20 +199,17 @@ impl<'a, T: Copy> IntoIterator for &'a List { unsafe impl Sync for List {} -unsafe impl<'a, T: 'a> rustc_data_structures::tagged_ptr::Pointer for &'a List { - const BITS: usize = bits_for::(); - - #[inline] - fn into_ptr(self) -> NonNull> { - NonNull::from(self) - } - - #[inline] - unsafe fn from_ptr(ptr: NonNull>) -> &'a List { - ptr.as_ref() - } +// Safety: +// Layouts of `Equivalent` and `List` are the same, modulo opaque tail, +// thus aligns of `Equivalent` and `List` must be the same. +unsafe impl Aligned for List { + const ALIGN: usize = { + #[repr(C)] + struct Equivalent { + _len: usize, + _data: [T; 0], + } - unsafe fn with_ref R>(ptr: NonNull>, f: F) -> R { - f(&ptr.as_ref()) - } + mem::align_of::>() + }; } From c6acd5c92fc9c193eb5ac71c24c6214d6105597b Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 11:15:10 +0000 Subject: [PATCH 08/22] Remove `Pointer::with_ref` in favour implementing it on tagged pointers directly --- .../rustc_data_structures/src/tagged_ptr.rs | 34 ------------------- .../src/tagged_ptr/copy.rs | 23 +++++++++++-- .../src/tagged_ptr/drop.rs | 8 +++-- 3 files changed, 25 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index f45f5a4215645..9a3d76fd4d4da 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -13,7 +13,6 @@ //! The tag must implement the `Tag` trait. We assert that the tag and `Pointer` //! are compatible at compile time. -use std::mem::ManuallyDrop; use std::ops::Deref; use std::ptr::NonNull; use std::rc::Rc; @@ -81,16 +80,6 @@ pub unsafe trait Pointer: Deref { /// This acts as `ptr::read` semantically, it should not be called more than /// once on non-`Copy` `Pointer`s. unsafe fn from_ptr(ptr: NonNull) -> Self; - - /// This provides a reference to the `Pointer` itself, rather than the - /// `Deref::Target`. It is used for cases where we want to call methods that - /// may be implement differently for the Pointer than the Pointee (e.g., - /// `Rc::clone` vs cloning the inner value). - /// - /// # Safety - /// - /// The passed `ptr` must be returned from `into_usize`. - unsafe fn with_ref R>(ptr: NonNull, f: F) -> R; } /// This describes tags that the `TaggedPtr` struct can hold. @@ -124,11 +113,6 @@ unsafe impl Pointer for Box { unsafe fn from_ptr(ptr: NonNull) -> Self { Box::from_raw(ptr.as_ptr()) } - - unsafe fn with_ref R>(ptr: NonNull, f: F) -> R { - let raw = ManuallyDrop::new(Self::from_ptr(ptr)); - f(&raw) - } } unsafe impl Pointer for Rc { @@ -143,11 +127,6 @@ unsafe impl Pointer for Rc { unsafe fn from_ptr(ptr: NonNull) -> Self { Rc::from_raw(ptr.as_ptr()) } - - unsafe fn with_ref R>(ptr: NonNull, f: F) -> R { - let raw = ManuallyDrop::new(Self::from_ptr(ptr)); - f(&raw) - } } unsafe impl Pointer for Arc { @@ -162,11 +141,6 @@ unsafe impl Pointer for Arc { unsafe fn from_ptr(ptr: NonNull) -> Self { Arc::from_raw(ptr.as_ptr()) } - - unsafe fn with_ref R>(ptr: NonNull, f: F) -> R { - let raw = ManuallyDrop::new(Self::from_ptr(ptr)); - f(&raw) - } } unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a T { @@ -181,10 +155,6 @@ unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a T { unsafe fn from_ptr(ptr: NonNull) -> Self { ptr.as_ref() } - - unsafe fn with_ref R>(ptr: NonNull, f: F) -> R { - f(&ptr.as_ref()) - } } unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a mut T { @@ -199,10 +169,6 @@ unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a mut T { unsafe fn from_ptr(mut ptr: NonNull) -> Self { ptr.as_mut() } - - unsafe fn with_ref R>(mut ptr: NonNull, f: F) -> R { - f(&ptr.as_mut()) - } } /// Returns the number of bits available for use for tags in a pointer to `T` diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 09d55b20ab4de..2900b883264af 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -2,6 +2,7 @@ use super::{Pointer, Tag}; use crate::stable_hasher::{HashStable, StableHasher}; use std::fmt; use std::marker::PhantomData; +use std::mem::ManuallyDrop; use std::num::NonZeroUsize; use std::ops::{Deref, DerefMut}; use std::ptr::NonNull; @@ -85,6 +86,24 @@ where self.packed.map_addr(|addr| unsafe { NonZeroUsize::new_unchecked(addr.get() << T::BITS) }) } + /// This provides a reference to the `P` pointer itself, rather than the + /// `Deref::Target`. It is used for cases where we want to call methods + /// that may be implement differently for the Pointer than the Pointee + /// (e.g., `Rc::clone` vs cloning the inner value). + pub(super) fn with_pointer_ref(&self, f: impl FnOnce(&P) -> R) -> R { + // Safety: + // - `self.raw.pointer_raw()` is originally returned from `P::into_ptr` + // and as such is valid for `P::from_ptr`. + // - This also allows us to not care whatever `f` panics or not. + // - Even though we create a copy of the pointer, we store it inside + // `ManuallyDrop` and only access it by-ref, so we don't double-drop. + // + // Semantically this is just `f(&self.pointer)` (where `self.pointer` + // is non-packed original pointer). + let ptr = unsafe { ManuallyDrop::new(P::from_ptr(self.pointer_raw())) }; + f(&ptr) + } + pub fn pointer(self) -> P where P: Copy, @@ -189,9 +208,7 @@ where T: Tag + HashStable, { fn hash_stable(&self, hcx: &mut HCX, hasher: &mut StableHasher) { - unsafe { - Pointer::with_ref(self.pointer_raw(), |p: &P| p.hash_stable(hcx, hasher)); - } + self.with_pointer_ref(|ptr| ptr.hash_stable(hcx, hasher)); self.tag().hash_stable(hcx, hasher); } } diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index c734dadefe9be..a2b119e15b8a5 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -1,8 +1,8 @@ -use super::{Pointer, Tag}; -use crate::stable_hasher::{HashStable, StableHasher}; use std::fmt; use super::CopyTaggedPtr; +use super::{Pointer, Tag}; +use crate::stable_hasher::{HashStable, StableHasher}; /// A TaggedPtr implementing `Drop`. /// @@ -23,7 +23,9 @@ where T: Tag, { fn clone(&self) -> Self { - unsafe { Self::new(P::with_ref(self.raw.pointer_raw(), |p| p.clone()), self.raw.tag()) } + let ptr = self.raw.with_pointer_ref(P::clone); + + Self::new(ptr, self.tag()) } } From c7c0b85f676eff0811a15a99acb01bc98cb2c4d4 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 11:30:45 +0000 Subject: [PATCH 09/22] Make tagged pointers debug impls print the pointer Does not really matter, but may be nicer in case the pointer has some specific debug impl. --- compiler/rustc_data_structures/src/tagged_ptr/copy.rs | 10 ++++------ compiler/rustc_data_structures/src/tagged_ptr/drop.rs | 10 ++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 2900b883264af..235c92da69958 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -163,15 +163,13 @@ where impl fmt::Debug for CopyTaggedPtr where - P: Pointer, - P::Target: fmt::Debug, + P: Pointer + fmt::Debug, T: Tag + fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("CopyTaggedPtr") - .field("pointer", &self.pointer_ref()) - .field("tag", &self.tag()) - .finish() + self.with_pointer_ref(|ptr| { + f.debug_struct("CopyTaggedPtr").field("pointer", ptr).field("tag", &self.tag()).finish() + }) } } diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index a2b119e15b8a5..d3fba0c30a9d0 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -85,15 +85,13 @@ where impl fmt::Debug for TaggedPtr where - P: Pointer, - P::Target: fmt::Debug, + P: Pointer + fmt::Debug, T: Tag + fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("TaggedPtr") - .field("pointer", &self.pointer_ref()) - .field("tag", &self.tag()) - .finish() + self.raw.with_pointer_ref(|ptr| { + f.debug_struct("TaggedPtr").field("pointer", ptr).field("tag", &self.tag()).finish() + }) } } From 8f408202c3a99fb8d4798996287211240dab39f4 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 11:41:41 +0000 Subject: [PATCH 10/22] Remove `pointer_{ref,mut}` from tagged pointers Just use `deref{,_mut}`! --- .../src/tagged_ptr/copy.rs | 24 +++++++------------ .../src/tagged_ptr/drop.rs | 7 ++---- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 235c92da69958..644f1415e732e 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -115,19 +115,6 @@ where unsafe { P::from_ptr(self.pointer_raw()) } } - pub fn pointer_ref(&self) -> &P::Target { - // SAFETY: pointer_raw returns the original pointer - unsafe { self.pointer_raw().as_ref() } - } - - pub fn pointer_mut(&mut self) -> &mut P::Target - where - P: DerefMut, - { - // SAFETY: pointer_raw returns the original pointer - unsafe { self.pointer_raw().as_mut() } - } - #[inline] pub fn tag(&self) -> T { unsafe { T::from_usize(self.packed.addr().get() >> Self::TAG_BIT_SHIFT) } @@ -147,7 +134,10 @@ where type Target = P::Target; fn deref(&self) -> &Self::Target { - self.pointer_ref() + // Safety: + // `pointer_raw` returns the original pointer from `P::into_ptr` which, + // by the `Pointer`'s contract, must be valid. + unsafe { self.pointer_raw().as_ref() } } } @@ -157,7 +147,11 @@ where T: Tag, { fn deref_mut(&mut self) -> &mut Self::Target { - self.pointer_mut() + // Safety: + // `pointer_raw` returns the original pointer from `P::into_ptr` which, + // by the `Pointer`'s contract, must be valid for writes if + // `P: DerefMut`. + unsafe { self.pointer_raw().as_mut() } } } diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index d3fba0c30a9d0..6e51916838fd9 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -41,9 +41,6 @@ where TaggedPtr { raw: CopyTaggedPtr::new(pointer, tag) } } - pub fn pointer_ref(&self) -> &P::Target { - self.raw.pointer_ref() - } pub fn tag(&self) -> T { self.raw.tag() } @@ -56,7 +53,7 @@ where { type Target = P::Target; fn deref(&self) -> &Self::Target { - self.raw.pointer_ref() + self.raw.deref() } } @@ -66,7 +63,7 @@ where T: Tag, { fn deref_mut(&mut self) -> &mut Self::Target { - self.raw.pointer_mut() + self.raw.deref_mut() } } From 3df9a7bde32e7ae9fab45024fd38db827531c48d Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 11:44:52 +0000 Subject: [PATCH 11/22] Shorten `COMPARE_PACKED` => `CP` where it is not important why can't I _ it :'( --- .../rustc_data_structures/src/tagged_ptr/copy.rs | 14 +++++++------- .../rustc_data_structures/src/tagged_ptr/drop.rs | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 644f1415e732e..edf429abfe991 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -24,7 +24,7 @@ where tag_ghost: PhantomData, } -impl Copy for CopyTaggedPtr +impl Copy for CopyTaggedPtr where P: Pointer, T: Tag, @@ -32,7 +32,7 @@ where { } -impl Clone for CopyTaggedPtr +impl Clone for CopyTaggedPtr where P: Pointer, T: Tag, @@ -46,7 +46,7 @@ where // We pack the tag into the *upper* bits of the pointer to ease retrieval of the // value; a left shift is a multiplication and those are embeddable in // instruction encoding. -impl CopyTaggedPtr +impl CopyTaggedPtr where P: Pointer, T: Tag, @@ -126,7 +126,7 @@ where } } -impl Deref for CopyTaggedPtr +impl Deref for CopyTaggedPtr where P: Pointer, T: Tag, @@ -141,7 +141,7 @@ where } } -impl DerefMut for CopyTaggedPtr +impl DerefMut for CopyTaggedPtr where P: Pointer + DerefMut, T: Tag, @@ -155,7 +155,7 @@ where } } -impl fmt::Debug for CopyTaggedPtr +impl fmt::Debug for CopyTaggedPtr where P: Pointer + fmt::Debug, T: Tag + fmt::Debug, @@ -194,7 +194,7 @@ where } } -impl HashStable for CopyTaggedPtr +impl HashStable for CopyTaggedPtr where P: Pointer + HashStable, T: Tag + HashStable, diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index 6e51916838fd9..a722a0f1f3215 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -17,7 +17,7 @@ where raw: CopyTaggedPtr, } -impl Clone for TaggedPtr +impl Clone for TaggedPtr where P: Pointer + Clone, T: Tag, @@ -32,7 +32,7 @@ where // We pack the tag into the *upper* bits of the pointer to ease retrieval of the // value; a right shift is a multiplication and those are embeddable in // instruction encoding. -impl TaggedPtr +impl TaggedPtr where P: Pointer, T: Tag, @@ -46,7 +46,7 @@ where } } -impl std::ops::Deref for TaggedPtr +impl std::ops::Deref for TaggedPtr where P: Pointer, T: Tag, @@ -57,7 +57,7 @@ where } } -impl std::ops::DerefMut for TaggedPtr +impl std::ops::DerefMut for TaggedPtr where P: Pointer + std::ops::DerefMut, T: Tag, @@ -67,7 +67,7 @@ where } } -impl Drop for TaggedPtr +impl Drop for TaggedPtr where P: Pointer, T: Tag, @@ -80,7 +80,7 @@ where } } -impl fmt::Debug for TaggedPtr +impl fmt::Debug for TaggedPtr where P: Pointer + fmt::Debug, T: Tag + fmt::Debug, @@ -119,7 +119,7 @@ where } } -impl HashStable for TaggedPtr +impl HashStable for TaggedPtr where P: Pointer + HashStable, T: Tag + HashStable, From 6f64ae3fbcf7a353537d250f91981ac336e12880 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 11:50:45 +0000 Subject: [PATCH 12/22] Move code around --- .../src/tagged_ptr/copy.rs | 77 +++++++++---------- .../src/tagged_ptr/drop.rs | 36 +++++---- 2 files changed, 57 insertions(+), 56 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index edf429abfe991..90500b2de89d1 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -1,6 +1,7 @@ use super::{Pointer, Tag}; use crate::stable_hasher::{HashStable, StableHasher}; use std::fmt; +use std::hash::{Hash, Hasher}; use std::marker::PhantomData; use std::mem::ManuallyDrop; use std::num::NonZeroUsize; @@ -24,25 +25,6 @@ where tag_ghost: PhantomData, } -impl Copy for CopyTaggedPtr -where - P: Pointer, - T: Tag, - P: Copy, -{ -} - -impl Clone for CopyTaggedPtr -where - P: Pointer, - T: Tag, - P: Copy, -{ - fn clone(&self) -> Self { - *self - } -} - // We pack the tag into the *upper* bits of the pointer to ease retrieval of the // value; a left shift is a multiplication and those are embeddable in // instruction encoding. @@ -55,6 +37,27 @@ where Self { packed: Self::pack(P::into_ptr(pointer), tag), tag_ghost: PhantomData } } + pub fn pointer(self) -> P + where + P: Copy, + { + // SAFETY: pointer_raw returns the original pointer + // + // Note that this isn't going to double-drop or anything because we have + // P: Copy + unsafe { P::from_ptr(self.pointer_raw()) } + } + + #[inline] + pub fn tag(&self) -> T { + unsafe { T::from_usize(self.packed.addr().get() >> Self::TAG_BIT_SHIFT) } + } + + #[inline] + pub fn set_tag(&mut self, tag: T) { + self.packed = Self::pack(self.pointer_raw(), tag); + } + const TAG_BIT_SHIFT: usize = usize::BITS as usize - T::BITS; const ASSERTION: () = { assert!(T::BITS <= P::BITS) }; @@ -103,26 +106,22 @@ where let ptr = unsafe { ManuallyDrop::new(P::from_ptr(self.pointer_raw())) }; f(&ptr) } +} - pub fn pointer(self) -> P - where - P: Copy, - { - // SAFETY: pointer_raw returns the original pointer - // - // Note that this isn't going to double-drop or anything because we have - // P: Copy - unsafe { P::from_ptr(self.pointer_raw()) } - } - - #[inline] - pub fn tag(&self) -> T { - unsafe { T::from_usize(self.packed.addr().get() >> Self::TAG_BIT_SHIFT) } - } +impl Copy for CopyTaggedPtr +where + P: Pointer + Copy, + T: Tag, +{ +} - #[inline] - pub fn set_tag(&mut self, tag: T) { - self.packed = Self::pack(self.pointer_raw(), tag); +impl Clone for CopyTaggedPtr +where + P: Pointer + Copy, + T: Tag, +{ + fn clone(&self) -> Self { + *self } } @@ -184,12 +183,12 @@ where { } -impl std::hash::Hash for CopyTaggedPtr +impl Hash for CopyTaggedPtr where P: Pointer, T: Tag, { - fn hash(&self, state: &mut H) { + fn hash(&self, state: &mut H) { self.packed.hash(state); } } diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index a722a0f1f3215..60f3e1d24610c 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -1,4 +1,6 @@ use std::fmt; +use std::hash::{Hash, Hasher}; +use std::ops::{Deref, DerefMut}; use super::CopyTaggedPtr; use super::{Pointer, Tag}; @@ -17,18 +19,6 @@ where raw: CopyTaggedPtr, } -impl Clone for TaggedPtr -where - P: Pointer + Clone, - T: Tag, -{ - fn clone(&self) -> Self { - let ptr = self.raw.with_pointer_ref(P::clone); - - Self::new(ptr, self.tag()) - } -} - // We pack the tag into the *upper* bits of the pointer to ease retrieval of the // value; a right shift is a multiplication and those are embeddable in // instruction encoding. @@ -46,7 +36,19 @@ where } } -impl std::ops::Deref for TaggedPtr +impl Clone for TaggedPtr +where + P: Pointer + Clone, + T: Tag, +{ + fn clone(&self) -> Self { + let ptr = self.raw.with_pointer_ref(P::clone); + + Self::new(ptr, self.tag()) + } +} + +impl Deref for TaggedPtr where P: Pointer, T: Tag, @@ -57,9 +59,9 @@ where } } -impl std::ops::DerefMut for TaggedPtr +impl DerefMut for TaggedPtr where - P: Pointer + std::ops::DerefMut, + P: Pointer + DerefMut, T: Tag, { fn deref_mut(&mut self) -> &mut Self::Target { @@ -109,12 +111,12 @@ where { } -impl std::hash::Hash for TaggedPtr +impl Hash for TaggedPtr where P: Pointer, T: Tag, { - fn hash(&self, state: &mut H) { + fn hash(&self, state: &mut H) { self.raw.hash(state); } } From 5e4577ec65a3dc327feee9618fa91a44797771d4 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 12:35:43 +0000 Subject: [PATCH 13/22] Add `TaggedPtr::set_tag` --- compiler/rustc_data_structures/src/tagged_ptr/drop.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index 60f3e1d24610c..de253a0b2552a 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -34,6 +34,10 @@ where pub fn tag(&self) -> T { self.raw.tag() } + + pub fn set_tag(&mut self, tag: T) { + self.raw.set_tag(tag) + } } impl Clone for TaggedPtr From 6f9b15c40c909e80acc27aca3118654cb506241e Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 12:46:19 +0000 Subject: [PATCH 14/22] Add tests for tagged pointers --- .../src/tagged_ptr/copy.rs | 30 ++++++ .../src/tagged_ptr/copy/tests.rs | 61 +++++++++++ .../src/tagged_ptr/drop.rs | 30 ++++++ .../src/tagged_ptr/drop/tests.rs | 101 ++++++++++++++++++ 4 files changed, 222 insertions(+) create mode 100644 compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs create mode 100644 compiler/rustc_data_structures/src/tagged_ptr/drop/tests.rs diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 90500b2de89d1..aebf24ebbdeb7 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -203,3 +203,33 @@ where self.tag().hash_stable(hcx, hasher); } } + +/// Test that `new` does not compile if there is not enough alignment for the +/// tag in the pointer. +/// +/// ```compile_fail,E0080 +/// use rustc_data_structures::tagged_ptr::{CopyTaggedPtr, Tag}; +/// +/// #[derive(Copy, Clone, Debug, PartialEq, Eq)] +/// enum Tag2 { B00 = 0b00, B01 = 0b01, B10 = 0b10, B11 = 0b11 }; +/// +/// unsafe impl Tag for Tag2 { +/// const BITS: usize = 2; +/// +/// fn into_usize(self) -> usize { todo!() } +/// unsafe fn from_usize(tag: usize) -> Self { todo!() } +/// } +/// +/// let value = 12u16; +/// let reference = &value; +/// let tag = Tag2::B01; +/// +/// let _ptr = CopyTaggedPtr::<_, _, true>::new(reference, tag); +/// ``` +// For some reason miri does not get the compile error +// probably it `check`s instead of `build`ing? +#[cfg(not(miri))] +const _: () = (); + +#[cfg(test)] +mod tests; diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs new file mode 100644 index 0000000000000..77544f9c0324d --- /dev/null +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs @@ -0,0 +1,61 @@ +use std::ptr; + +use crate::tagged_ptr::{CopyTaggedPtr, Pointer, Tag}; + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum Tag2 { + B00 = 0b00, + B01 = 0b01, + B10 = 0b10, + B11 = 0b11, +} + +unsafe impl Tag for Tag2 { + const BITS: usize = 2; + + fn into_usize(self) -> usize { + self as _ + } + + unsafe fn from_usize(tag: usize) -> Self { + const B00: usize = Tag2::B00 as _; + const B01: usize = Tag2::B01 as _; + const B10: usize = Tag2::B10 as _; + const B11: usize = Tag2::B11 as _; + match tag { + B00 => Tag2::B00, + B01 => Tag2::B01, + B10 => Tag2::B10, + B11 => Tag2::B11, + _ => unreachable!(), + } + } +} + +#[test] +fn smoke() { + let value = 12u32; + let reference = &value; + let tag = Tag2::B01; + + let ptr = tag_ptr(reference, tag); + + assert_eq!(ptr.tag(), tag); + assert_eq!(*ptr, 12); + assert!(ptr::eq(ptr.pointer(), reference)); + + let copy = ptr; + + let mut ptr = ptr; + ptr.set_tag(Tag2::B00); + assert_eq!(ptr.tag(), Tag2::B00); + + assert_eq!(copy.tag(), tag); + assert_eq!(*copy, 12); + assert!(ptr::eq(copy.pointer(), reference)); +} + +/// Helper to create tagged pointers without specifying `COMPARE_PACKED` if it does not matter. +fn tag_ptr(ptr: P, tag: T) -> CopyTaggedPtr { + CopyTaggedPtr::new(ptr, tag) +} diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index de253a0b2552a..286951ce0e9c1 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -134,3 +134,33 @@ where self.raw.hash_stable(hcx, hasher); } } + +/// Test that `new` does not compile if there is not enough alignment for the +/// tag in the pointer. +/// +/// ```compile_fail,E0080 +/// use rustc_data_structures::tagged_ptr::{TaggedPtr, Tag}; +/// +/// #[derive(Copy, Clone, Debug, PartialEq, Eq)] +/// enum Tag2 { B00 = 0b00, B01 = 0b01, B10 = 0b10, B11 = 0b11 }; +/// +/// unsafe impl Tag for Tag2 { +/// const BITS: usize = 2; +/// +/// fn into_usize(self) -> usize { todo!() } +/// unsafe fn from_usize(tag: usize) -> Self { todo!() } +/// } +/// +/// let value = 12u16; +/// let reference = &value; +/// let tag = Tag2::B01; +/// +/// let _ptr = TaggedPtr::<_, _, true>::new(reference, tag); +/// ``` +// For some reason miri does not get the compile error +// probably it `check`s instead of `build`ing? +#[cfg(not(miri))] +const _: () = (); + +#[cfg(test)] +mod tests; diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop/tests.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop/tests.rs new file mode 100644 index 0000000000000..0c61cebaf7e6a --- /dev/null +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop/tests.rs @@ -0,0 +1,101 @@ +use std::{ptr, sync::Arc}; + +use crate::tagged_ptr::{Pointer, Tag, TaggedPtr}; + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum Tag2 { + B00 = 0b00, + B01 = 0b01, + B10 = 0b10, + B11 = 0b11, +} + +unsafe impl Tag for Tag2 { + const BITS: usize = 2; + + fn into_usize(self) -> usize { + self as _ + } + + unsafe fn from_usize(tag: usize) -> Self { + const B00: usize = Tag2::B00 as _; + const B01: usize = Tag2::B01 as _; + const B10: usize = Tag2::B10 as _; + const B11: usize = Tag2::B11 as _; + match tag { + B00 => Tag2::B00, + B01 => Tag2::B01, + B10 => Tag2::B10, + B11 => Tag2::B11, + _ => unreachable!(), + } + } +} + +#[test] +fn smoke() { + let value = 12u32; + let reference = &value; + let tag = Tag2::B01; + + let ptr = tag_ptr(reference, tag); + + assert_eq!(ptr.tag(), tag); + assert_eq!(*ptr, 12); + + let clone = ptr.clone(); + assert_eq!(clone.tag(), tag); + assert_eq!(*clone, 12); + + let mut ptr = ptr; + ptr.set_tag(Tag2::B00); + assert_eq!(ptr.tag(), Tag2::B00); + + assert_eq!(clone.tag(), tag); + assert_eq!(*clone, 12); + assert!(ptr::eq(&*ptr, &*clone)) +} + +#[test] +fn boxed() { + let value = 12u32; + let boxed = Box::new(value); + let tag = Tag2::B01; + + let ptr = tag_ptr(boxed, tag); + + assert_eq!(ptr.tag(), tag); + assert_eq!(*ptr, 12); + + let clone = ptr.clone(); + assert_eq!(clone.tag(), tag); + assert_eq!(*clone, 12); + + let mut ptr = ptr; + ptr.set_tag(Tag2::B00); + assert_eq!(ptr.tag(), Tag2::B00); + + assert_eq!(clone.tag(), tag); + assert_eq!(*clone, 12); + assert!(!ptr::eq(&*ptr, &*clone)) +} + +#[test] +fn arclones() { + let value = 12u32; + let arc = Arc::new(value); + let tag = Tag2::B01; + + let ptr = tag_ptr(arc, tag); + + assert_eq!(ptr.tag(), tag); + assert_eq!(*ptr, 12); + + let clone = ptr.clone(); + assert!(ptr::eq(&*ptr, &*clone)) +} + +/// Helper to create tagged pointers without specifying `COMPARE_PACKED` if it does not matter. +fn tag_ptr(ptr: P, tag: T) -> TaggedPtr { + TaggedPtr::new(ptr, tag) +} From 838c5491a4296bb92891403c1a6d0e9d84991b51 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 16:21:28 +0000 Subject: [PATCH 15/22] Document tagged pointers better --- .../rustc_data_structures/src/tagged_ptr.rs | 73 +++++++++++++++---- .../src/tagged_ptr/copy.rs | 71 +++++++++++++++--- .../src/tagged_ptr/drop.rs | 17 +++-- 3 files changed, 130 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index 9a3d76fd4d4da..f10c12ceeda8e 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -3,15 +3,17 @@ //! In order to utilize the pointer packing, you must have two types: a pointer, //! and a tag. //! -//! The pointer must implement the `Pointer` trait, with the primary requirement -//! being conversion to and from a usize. Note that the pointer must be -//! dereferenceable, so raw pointers generally cannot implement the `Pointer` -//! trait. This implies that the pointer must also be nonzero. +//! The pointer must implement the [`Pointer`] trait, with the primary +//! requirement being convertible to and from a raw pointer. Note that the +//! pointer must be dereferenceable, so raw pointers generally cannot implement +//! the [`Pointer`] trait. This implies that the pointer must also be non-null. //! -//! Many common pointer types already implement the `Pointer` trait. +//! Many common pointer types already implement the [`Pointer`] trait. //! -//! The tag must implement the `Tag` trait. We assert that the tag and `Pointer` -//! are compatible at compile time. +//! The tag must implement the [`Tag`] trait. +//! +//! We assert that the tag and the [`Pointer`] types are compatible at compile +//! time. use std::ops::Deref; use std::ptr::NonNull; @@ -71,32 +73,66 @@ pub unsafe trait Pointer: Deref { /// [`Self::Target`]: Deref::Target const BITS: usize; + /// Turns this pointer into a raw, non-null pointer. + /// + /// The inverse of this function is [`from_ptr`]. + /// + /// This function guarantees that the least-significant [`Self::BITS`] bits + /// are zero. + /// + /// [`from_ptr`]: Pointer::from_ptr + /// [`Self::BITS`]: Pointer::BITS fn into_ptr(self) -> NonNull; + /// Re-creates the original pointer, from a raw pointer returned by [`into_ptr`]. + /// /// # Safety /// - /// The passed `ptr` must be returned from `into_usize`. + /// The passed `ptr` must be returned from [`into_ptr`]. + /// + /// This acts as [`ptr::read::()`] semantically, it should not be called more than + /// once on non-[`Copy`] `Pointer`s. /// - /// This acts as `ptr::read` semantically, it should not be called more than - /// once on non-`Copy` `Pointer`s. + /// [`into_ptr`]: Pointer::into_ptr + /// [`ptr::read::()`]: std::ptr::read unsafe fn from_ptr(ptr: NonNull) -> Self; } -/// This describes tags that the `TaggedPtr` struct can hold. +/// This describes tags that the [`TaggedPtr`] struct can hold. /// /// # Safety /// -/// The BITS constant must be correct. +/// The [`BITS`] constant must be correct. +/// +/// No more than [`BITS`] least significant bits may be set in the returned usize. /// -/// No more than `BITS` least significant bits may be set in the returned usize. +/// [`BITS`]: Tag::BITS pub unsafe trait Tag: Copy { + /// Number of least-significant bits in the return value of [`into_usize`] + /// which may be non-zero. In other words this is the bit width of the + /// value. + /// + /// [`into_usize`]: Tag::into_usize const BITS: usize; + /// Turns this tag into an integer. + /// + /// The inverse of this function is [`from_usize`]. + /// + /// This function guarantees that only the least-significant [`Self::BITS`] + /// bits can be non-zero. + /// + /// [`from_usize`]: Pointer::from_usize + /// [`Self::BITS`]: Tag::BITS fn into_usize(self) -> usize; + /// Re-creates the tag from the integer returned by [`into_usize`]. + /// /// # Safety /// - /// The passed `tag` must be returned from `into_usize`. + /// The passed `tag` must be returned from [`into_usize`]. + /// + /// [`into_usize`]: Tag::into_usize unsafe fn from_usize(tag: usize) -> Self; } @@ -111,6 +147,7 @@ unsafe impl Pointer for Box { #[inline] unsafe fn from_ptr(ptr: NonNull) -> Self { + // Safety: `ptr` comes from `into_ptr` which calls `Box::into_raw` Box::from_raw(ptr.as_ptr()) } } @@ -120,11 +157,13 @@ unsafe impl Pointer for Rc { #[inline] fn into_ptr(self) -> NonNull { + // Safety: pointers from `Rc::into_raw` are valid & non-null unsafe { NonNull::new_unchecked(Rc::into_raw(self).cast_mut()) } } #[inline] unsafe fn from_ptr(ptr: NonNull) -> Self { + // Safety: `ptr` comes from `into_ptr` which calls `Rc::into_raw` Rc::from_raw(ptr.as_ptr()) } } @@ -134,11 +173,13 @@ unsafe impl Pointer for Arc { #[inline] fn into_ptr(self) -> NonNull { + // Safety: pointers from `Arc::into_raw` are valid & non-null unsafe { NonNull::new_unchecked(Arc::into_raw(self).cast_mut()) } } #[inline] unsafe fn from_ptr(ptr: NonNull) -> Self { + // Safety: `ptr` comes from `into_ptr` which calls `Arc::into_raw` Arc::from_raw(ptr.as_ptr()) } } @@ -153,6 +194,8 @@ unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a T { #[inline] unsafe fn from_ptr(ptr: NonNull) -> Self { + // Safety: + // `ptr` comes from `into_ptr` which gets the pointer from a reference ptr.as_ref() } } @@ -167,6 +210,8 @@ unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a mut T { #[inline] unsafe fn from_ptr(mut ptr: NonNull) -> Self { + // Safety: + // `ptr` comes from `into_ptr` which gets the pointer from a reference ptr.as_mut() } } diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index aebf24ebbdeb7..02dcbd389dfdd 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -8,35 +8,75 @@ use std::num::NonZeroUsize; use std::ops::{Deref, DerefMut}; use std::ptr::NonNull; -/// A `Copy` TaggedPtr. +/// A [`Copy`] tagged pointer. /// -/// You should use this instead of the `TaggedPtr` type in all cases where -/// `P: Copy`. +/// This is essentially `{ pointer: P, tag: T }` packed in a single pointer. +/// +/// You should use this instead of the [`TaggedPtr`] type in all cases where +/// `P` implements [`Copy`]. /// /// If `COMPARE_PACKED` is true, then the pointers will be compared and hashed without -/// unpacking. Otherwise we don't implement PartialEq/Eq/Hash; if you want that, -/// wrap the TaggedPtr. +/// unpacking. Otherwise we don't implement [`PartialEq`], [`Eq`] and [`Hash`]; +/// if you want that, wrap the [`CopyTaggedPtr`]. +/// +/// [`TaggedPtr`]: crate::tagged_ptr::TaggedPtr pub struct CopyTaggedPtr where P: Pointer, T: Tag, { + /// This is semantically a pair of `pointer: P` and `tag: T` fields, + /// however we pack them in a single pointer, to save space. + /// + /// We pack the tag into the **most**-significant bits of the pointer to + /// ease retrieval of the value. A left shift is a multiplication and + /// those are embeddable in instruction encoding, for example: + /// + /// ```asm + /// // (https://godbolt.org/z/jqcYPWEr3) + /// example::shift_read3: + /// mov eax, dword ptr [8*rdi] + /// ret + /// + /// example::mask_read3: + /// and rdi, -8 + /// mov eax, dword ptr [rdi] + /// ret + /// ``` + /// + /// This is ASM outputted by rustc for reads of values behind tagged + /// pointers for different approaches of tagging: + /// - `shift_read3` uses `<< 3` (the tag is in the most-significant bits) + /// - `mask_read3` uses `& !0b111` (the tag is in the least-significant bits) + /// + /// The shift approach thus produces less instructions and is likely faster. + /// + /// Encoding diagram: + /// ```text + /// [ packed.addr ] + /// [ tag ] [ pointer.addr >> T::BITS ] <-- usize::BITS - T::BITS bits + /// ^ + /// | + /// T::BITS bits + /// ``` + /// + /// The tag can be retrieved by `packed.addr() >> T::BITS` and the pointer + /// can be retrieved by `packed.map_addr(|addr| addr << T::BITS)`. packed: NonNull, tag_ghost: PhantomData, } -// We pack the tag into the *upper* bits of the pointer to ease retrieval of the -// value; a left shift is a multiplication and those are embeddable in -// instruction encoding. impl CopyTaggedPtr where P: Pointer, T: Tag, { + /// Tags `pointer` with `tag`. pub fn new(pointer: P, tag: T) -> Self { Self { packed: Self::pack(P::into_ptr(pointer), tag), tag_ghost: PhantomData } } + /// Retrieves the pointer. pub fn pointer(self) -> P where P: Copy, @@ -48,11 +88,18 @@ where unsafe { P::from_ptr(self.pointer_raw()) } } + /// Retrieves the tag. #[inline] pub fn tag(&self) -> T { - unsafe { T::from_usize(self.packed.addr().get() >> Self::TAG_BIT_SHIFT) } + // Unpack the tag, according to the `self.packed` encoding scheme + let tag = self.packed.addr().get() >> Self::TAG_BIT_SHIFT; + + // Safety: + // + unsafe { T::from_usize(tag) } } + /// Sets the tag to a new value. #[inline] pub fn set_tag(&mut self, tag: T) { self.packed = Self::pack(self.pointer_raw(), tag); @@ -61,7 +108,8 @@ where const TAG_BIT_SHIFT: usize = usize::BITS as usize - T::BITS; const ASSERTION: () = { assert!(T::BITS <= P::BITS) }; - /// Pack pointer `ptr` that comes from [`P::into_ptr`] with a `tag`. + /// Pack pointer `ptr` that comes from [`P::into_ptr`] with a `tag`, + /// according to `self.packed` encoding scheme. /// /// [`P::into_ptr`]: Pointer::into_ptr fn pack(ptr: NonNull, tag: T) -> NonNull { @@ -71,7 +119,7 @@ where let packed_tag = tag.into_usize() << Self::TAG_BIT_SHIFT; ptr.map_addr(|addr| { - // SAFETY: + // Safety: // - The pointer is `NonNull` => it's address is `NonZeroUsize` // - `P::BITS` least significant bits are always zero (`Pointer` contract) // - `T::BITS <= P::BITS` (from `Self::ASSERTION`) @@ -85,6 +133,7 @@ where }) } + /// Retrieves the original raw pointer from `self.packed`. pub(super) fn pointer_raw(&self) -> NonNull { self.packed.map_addr(|addr| unsafe { NonZeroUsize::new_unchecked(addr.get() << T::BITS) }) } diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index 286951ce0e9c1..6ca6c7d1283b4 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -6,11 +6,16 @@ use super::CopyTaggedPtr; use super::{Pointer, Tag}; use crate::stable_hasher::{HashStable, StableHasher}; -/// A TaggedPtr implementing `Drop`. +/// A tagged pointer that supports pointers that implement [`Drop`]. +/// +/// This is essentially `{ pointer: P, tag: T }` packed in a single pointer. +/// +/// You should use [`CopyTaggedPtr`] instead of the this type in all cases +/// where `P` implements [`Copy`]. /// /// If `COMPARE_PACKED` is true, then the pointers will be compared and hashed without -/// unpacking. Otherwise we don't implement PartialEq/Eq/Hash; if you want that, -/// wrap the TaggedPtr. +/// unpacking. Otherwise we don't implement [`PartialEq`], [`Eq`] and [`Hash`]; +/// if you want that, wrap the [`TaggedPtr`]. pub struct TaggedPtr where P: Pointer, @@ -19,22 +24,22 @@ where raw: CopyTaggedPtr, } -// We pack the tag into the *upper* bits of the pointer to ease retrieval of the -// value; a right shift is a multiplication and those are embeddable in -// instruction encoding. impl TaggedPtr where P: Pointer, T: Tag, { + /// Tags `pointer` with `tag`. pub fn new(pointer: P, tag: T) -> Self { TaggedPtr { raw: CopyTaggedPtr::new(pointer, tag) } } + /// Retrieves the tag. pub fn tag(&self) -> T { self.raw.tag() } + /// Sets the tag to a new value. pub fn set_tag(&mut self, tag: T) { self.raw.set_tag(tag) } From dc19dc29c9c384d2285a2a659111d670483562bb Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Wed, 12 Apr 2023 19:00:27 +0000 Subject: [PATCH 16/22] doc fixes --- compiler/rustc_data_structures/src/tagged_ptr.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index f10c12ceeda8e..2a74db1265499 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -40,11 +40,12 @@ pub use drop::TaggedPtr; /// [`into_ptr`] must be valid for writes (and thus calling [`NonNull::as_mut`] /// on it must be safe). /// -/// The `BITS` constant must be correct. At least `BITS` bits, least-significant, -/// must be zero on all pointers returned from [`into_ptr`]. +/// The [`BITS`] constant must be correct. At least [`BITS`] least significant +/// bits, must be zero on all pointers returned from [`into_ptr`]. /// /// For example, if the alignment of [`Self::Target`] is 2, then `BITS` should be 1. /// +/// [`BITS`]: Pointer::BITS /// [`into_ptr`]: Pointer::into_ptr /// [valid]: std::ptr#safety /// [`::Target`]: Deref::Target @@ -122,7 +123,7 @@ pub unsafe trait Tag: Copy { /// This function guarantees that only the least-significant [`Self::BITS`] /// bits can be non-zero. /// - /// [`from_usize`]: Pointer::from_usize + /// [`from_usize`]: Tag::from_usize /// [`Self::BITS`]: Tag::BITS fn into_usize(self) -> usize; From c155d5149fe49736cfb5b24eaa1c854b836019f3 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 13 Apr 2023 16:45:07 +0000 Subject: [PATCH 17/22] Implement `Send`/`Sync` for `CopyTaggedPtr` --- .../src/tagged_ptr/copy.rs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 02dcbd389dfdd..68d660f48b4da 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -253,6 +253,26 @@ where } } +// Safety: +// `CopyTaggedPtr` is semantically just `{ ptr: P, tag: T }`, as such +// it's ok to implement `Sync` as long as `P: Sync, T: Sync` +unsafe impl Sync for CopyTaggedPtr +where + P: Sync + Pointer, + T: Sync + Tag, +{ +} + +// Safety: +// `CopyTaggedPtr` is semantically just `{ ptr: P, tag: T }`, as such +// it's ok to implement `Send` as long as `P: Send, T: Send` +unsafe impl Send for CopyTaggedPtr +where + P: Send + Pointer, + T: Send + Tag, +{ +} + /// Test that `new` does not compile if there is not enough alignment for the /// tag in the pointer. /// From 8d49e948a83faf8b88cd7f3327e9f65a7296a0f5 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 14 Apr 2023 11:04:20 +0000 Subject: [PATCH 18/22] Doc fixes from review --- .../rustc_data_structures/src/tagged_ptr.rs | 12 +++++++---- .../src/tagged_ptr/copy.rs | 21 ++++++++++++++++--- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index 2a74db1265499..750affa468b56 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -40,8 +40,8 @@ pub use drop::TaggedPtr; /// [`into_ptr`] must be valid for writes (and thus calling [`NonNull::as_mut`] /// on it must be safe). /// -/// The [`BITS`] constant must be correct. At least [`BITS`] least significant -/// bits, must be zero on all pointers returned from [`into_ptr`]. +/// The [`BITS`] constant must be correct. [`BITS`] least-significant bits, +/// must be zero on all pointers returned from [`into_ptr`]. /// /// For example, if the alignment of [`Self::Target`] is 2, then `BITS` should be 1. /// @@ -52,9 +52,12 @@ pub use drop::TaggedPtr; /// [`Self::Target`]: Deref::Target /// [`DerefMut`]: std::ops::DerefMut pub unsafe trait Pointer: Deref { - /// Number of unused (always zero) **least significant bits** in this + /// Number of unused (always zero) **least-significant bits** in this /// pointer, usually related to the pointees alignment. /// + /// For example if [`BITS`] = `2`, then given `ptr = Self::into_ptr(..)`, + /// `ptr.addr() & 0b11 == 0` must be true. + /// /// Most likely the value you want to use here is the following, unless /// your [`Self::Target`] type is unsized (e.g., `ty::List` in rustc) /// or your pointer is over/under aligned, in which case you'll need to @@ -71,6 +74,7 @@ pub unsafe trait Pointer: Deref { /// # } /// ``` /// + /// [`BITS`]: Pointer::BITS /// [`Self::Target`]: Deref::Target const BITS: usize; @@ -105,7 +109,7 @@ pub unsafe trait Pointer: Deref { /// /// The [`BITS`] constant must be correct. /// -/// No more than [`BITS`] least significant bits may be set in the returned usize. +/// No more than [`BITS`] least-significant bits may be set in the returned usize. /// /// [`BITS`]: Tag::BITS pub unsafe trait Tag: Copy { diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 68d660f48b4da..83dfdb0bd876d 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -33,7 +33,7 @@ where /// those are embeddable in instruction encoding, for example: /// /// ```asm - /// // (https://godbolt.org/z/jqcYPWEr3) + /// // () /// example::shift_read3: /// mov eax, dword ptr [8*rdi] /// ret @@ -49,7 +49,8 @@ where /// - `shift_read3` uses `<< 3` (the tag is in the most-significant bits) /// - `mask_read3` uses `& !0b111` (the tag is in the least-significant bits) /// - /// The shift approach thus produces less instructions and is likely faster. + /// The shift approach thus produces less instructions and is likely faster + /// (see ). /// /// Encoding diagram: /// ```text @@ -66,12 +67,21 @@ where tag_ghost: PhantomData, } +// Note that even though `CopyTaggedPtr` is only really expected to work with +// `P: Copy`, can't add `P: Copy` bound, because `CopyTaggedPtr` is used in the +// `TaggedPtr`'s implementation. impl CopyTaggedPtr where P: Pointer, T: Tag, { /// Tags `pointer` with `tag`. + /// + /// Note that this leaks `pointer`: it won't be dropped when + /// `CopyTaggedPtr` is dropped. If you have a pointer with a significant + /// drop, use [`TaggedPtr`] instead. + /// + /// [`TaggedPtr`]: crate::tagged_ptr::TaggedPtr pub fn new(pointer: P, tag: T) -> Self { Self { packed: Self::pack(P::into_ptr(pointer), tag), tag_ghost: PhantomData } } @@ -95,7 +105,8 @@ where let tag = self.packed.addr().get() >> Self::TAG_BIT_SHIFT; // Safety: - // + // The shift retrieves the original value from `T::into_usize`, + // satisfying `T::from_usize`'s preconditions. unsafe { T::from_usize(tag) } } @@ -152,6 +163,10 @@ where // // Semantically this is just `f(&self.pointer)` (where `self.pointer` // is non-packed original pointer). + // + // Note that even though `CopyTaggedPtr` is only really expected to + // work with `P: Copy`, we have to assume `P: ?Copy`, because + // `CopyTaggedPtr` is used in the `TaggedPtr`'s implementation. let ptr = unsafe { ManuallyDrop::new(P::from_ptr(self.pointer_raw())) }; f(&ptr) } From 251f662e4dc50055e8376f3c1bb8ac61ac7148c1 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 14 Apr 2023 11:09:26 +0000 Subject: [PATCH 19/22] Share `Tag2` impl between `CopyTaggedPtr` and `TaggedPtr` tests --- .../rustc_data_structures/src/tagged_ptr.rs | 29 +++++++++++++++++ .../src/tagged_ptr/copy/tests.rs | 32 +------------------ .../src/tagged_ptr/drop/tests.rs | 32 +------------------ 3 files changed, 31 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index 750affa468b56..14f282dcbebfe 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -232,3 +232,32 @@ pub const fn bits_for() -> usize { bits as usize } + +/// A tag type used in [`CopyTaggedPtr`] and [`TaggedPtr`] tests. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[cfg(test)] +enum Tag2 { + B00 = 0b00, + B01 = 0b01, + B10 = 0b10, + B11 = 0b11, +} + +#[cfg(test)] +unsafe impl Tag for Tag2 { + const BITS: usize = 2; + + fn into_usize(self) -> usize { + self as _ + } + + unsafe fn from_usize(tag: usize) -> Self { + match tag { + 0b00 => Tag2::B00, + 0b01 => Tag2::B01, + 0b10 => Tag2::B10, + 0b11 => Tag2::B11, + _ => unreachable!(), + } + } +} diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs index 77544f9c0324d..09e7b59fd64d5 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs @@ -1,36 +1,6 @@ use std::ptr; -use crate::tagged_ptr::{CopyTaggedPtr, Pointer, Tag}; - -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -enum Tag2 { - B00 = 0b00, - B01 = 0b01, - B10 = 0b10, - B11 = 0b11, -} - -unsafe impl Tag for Tag2 { - const BITS: usize = 2; - - fn into_usize(self) -> usize { - self as _ - } - - unsafe fn from_usize(tag: usize) -> Self { - const B00: usize = Tag2::B00 as _; - const B01: usize = Tag2::B01 as _; - const B10: usize = Tag2::B10 as _; - const B11: usize = Tag2::B11 as _; - match tag { - B00 => Tag2::B00, - B01 => Tag2::B01, - B10 => Tag2::B10, - B11 => Tag2::B11, - _ => unreachable!(), - } - } -} +use crate::tagged_ptr::{CopyTaggedPtr, Pointer, Tag, Tag2}; #[test] fn smoke() { diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop/tests.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop/tests.rs index 0c61cebaf7e6a..2c17d678d3aa0 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop/tests.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop/tests.rs @@ -1,36 +1,6 @@ use std::{ptr, sync::Arc}; -use crate::tagged_ptr::{Pointer, Tag, TaggedPtr}; - -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -enum Tag2 { - B00 = 0b00, - B01 = 0b01, - B10 = 0b10, - B11 = 0b11, -} - -unsafe impl Tag for Tag2 { - const BITS: usize = 2; - - fn into_usize(self) -> usize { - self as _ - } - - unsafe fn from_usize(tag: usize) -> Self { - const B00: usize = Tag2::B00 as _; - const B01: usize = Tag2::B01 as _; - const B10: usize = Tag2::B10 as _; - const B11: usize = Tag2::B11 as _; - match tag { - B00 => Tag2::B00, - B01 => Tag2::B01, - B10 => Tag2::B10, - B11 => Tag2::B11, - _ => unreachable!(), - } - } -} +use crate::tagged_ptr::{Pointer, Tag, Tag2, TaggedPtr}; #[test] fn smoke() { From 36f5918bf169a7ab5ae24a5aad12dd6ecd20b8c4 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 14 Apr 2023 11:59:53 +0000 Subject: [PATCH 20/22] Test `CopyTaggedPtr`'s `HashStable` impl --- .../rustc_data_structures/src/tagged_ptr.rs | 7 +++++++ .../src/tagged_ptr/copy/tests.rs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index 14f282dcbebfe..1fc5dad1d95b4 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -261,3 +261,10 @@ unsafe impl Tag for Tag2 { } } } + +#[cfg(test)] +impl crate::stable_hasher::HashStable for Tag2 { + fn hash_stable(&self, hcx: &mut HCX, hasher: &mut crate::stable_hasher::StableHasher) { + (*self as u8).hash_stable(hcx, hasher); + } +} diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs index 09e7b59fd64d5..bfcc2e603de43 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy/tests.rs @@ -1,5 +1,6 @@ use std::ptr; +use crate::stable_hasher::{HashStable, StableHasher}; use crate::tagged_ptr::{CopyTaggedPtr, Pointer, Tag, Tag2}; #[test] @@ -25,6 +26,24 @@ fn smoke() { assert!(ptr::eq(copy.pointer(), reference)); } +#[test] +fn stable_hash_hashes_as_tuple() { + let hash_packed = { + let mut hasher = StableHasher::new(); + tag_ptr(&12, Tag2::B11).hash_stable(&mut (), &mut hasher); + + hasher.finalize() + }; + + let hash_tupled = { + let mut hasher = StableHasher::new(); + (&12, Tag2::B11).hash_stable(&mut (), &mut hasher); + hasher.finalize() + }; + + assert_eq!(hash_packed, hash_tupled); +} + /// Helper to create tagged pointers without specifying `COMPARE_PACKED` if it does not matter. fn tag_ptr(ptr: P, tag: T) -> CopyTaggedPtr { CopyTaggedPtr::new(ptr, tag) From 014c6f208e59534cf36b11fdf43f8c90a304073f Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 14 Apr 2023 12:29:10 +0000 Subject: [PATCH 21/22] Use `ptr::Alignment` for extra coolness points --- compiler/rustc_data_structures/src/aligned.rs | 10 +++---- compiler/rustc_data_structures/src/lib.rs | 1 + .../rustc_data_structures/src/tagged_ptr.rs | 28 ++++++++----------- .../src/tagged_ptr/copy.rs | 4 +-- .../src/tagged_ptr/drop.rs | 2 +- compiler/rustc_middle/src/lib.rs | 1 + compiler/rustc_middle/src/ty/list.rs | 6 ++-- compiler/rustc_middle/src/ty/mod.rs | 4 ++- 8 files changed, 27 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_data_structures/src/aligned.rs b/compiler/rustc_data_structures/src/aligned.rs index 2d0adfe2ae35b..7ac073539fb4b 100644 --- a/compiler/rustc_data_structures/src/aligned.rs +++ b/compiler/rustc_data_structures/src/aligned.rs @@ -1,10 +1,10 @@ -use std::mem; +use std::ptr::Alignment; /// Returns the ABI-required minimum alignment of a type in bytes. /// /// This is equivalent to [`mem::align_of`], but also works for some unsized /// types (e.g. slices or rustc's `List`s). -pub const fn align_of() -> usize { +pub const fn align_of() -> Alignment { T::ALIGN } @@ -19,13 +19,13 @@ pub const fn align_of() -> usize { /// [`mem::align_of()`]: mem::align_of pub unsafe trait Aligned { /// Alignment of `Self`. - const ALIGN: usize; + const ALIGN: Alignment; } unsafe impl Aligned for T { - const ALIGN: usize = mem::align_of::(); + const ALIGN: Alignment = Alignment::of::(); } unsafe impl Aligned for [T] { - const ALIGN: usize = mem::align_of::(); + const ALIGN: Alignment = Alignment::of::(); } diff --git a/compiler/rustc_data_structures/src/lib.rs b/compiler/rustc_data_structures/src/lib.rs index ea1f71d7115ca..7768e0fdeb13b 100644 --- a/compiler/rustc_data_structures/src/lib.rs +++ b/compiler/rustc_data_structures/src/lib.rs @@ -30,6 +30,7 @@ #![feature(lint_reasons)] #![feature(unwrap_infallible)] #![feature(strict_provenance)] +#![feature(ptr_alignment_type)] #![allow(rustc::default_hash_types)] #![allow(rustc::potential_query_instability)] #![deny(rustc::untranslatable_diagnostic)] diff --git a/compiler/rustc_data_structures/src/tagged_ptr.rs b/compiler/rustc_data_structures/src/tagged_ptr.rs index 1fc5dad1d95b4..c26bffac67823 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr.rs @@ -70,13 +70,13 @@ pub unsafe trait Pointer: Deref { /// # struct T; /// # impl Deref for T { type Target = u8; fn deref(&self) -> &u8 { &0 } } /// # impl T { - /// const BITS: usize = bits_for::<::Target>(); + /// const BITS: u32 = bits_for::<::Target>(); /// # } /// ``` /// /// [`BITS`]: Pointer::BITS /// [`Self::Target`]: Deref::Target - const BITS: usize; + const BITS: u32; /// Turns this pointer into a raw, non-null pointer. /// @@ -118,7 +118,7 @@ pub unsafe trait Tag: Copy { /// value. /// /// [`into_usize`]: Tag::into_usize - const BITS: usize; + const BITS: u32; /// Turns this tag into an integer. /// @@ -142,7 +142,7 @@ pub unsafe trait Tag: Copy { } unsafe impl Pointer for Box { - const BITS: usize = bits_for::(); + const BITS: u32 = bits_for::(); #[inline] fn into_ptr(self) -> NonNull { @@ -158,7 +158,7 @@ unsafe impl Pointer for Box { } unsafe impl Pointer for Rc { - const BITS: usize = bits_for::(); + const BITS: u32 = bits_for::(); #[inline] fn into_ptr(self) -> NonNull { @@ -174,7 +174,7 @@ unsafe impl Pointer for Rc { } unsafe impl Pointer for Arc { - const BITS: usize = bits_for::(); + const BITS: u32 = bits_for::(); #[inline] fn into_ptr(self) -> NonNull { @@ -190,7 +190,7 @@ unsafe impl Pointer for Arc { } unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a T { - const BITS: usize = bits_for::(); + const BITS: u32 = bits_for::(); #[inline] fn into_ptr(self) -> NonNull { @@ -206,7 +206,7 @@ unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a T { } unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a mut T { - const BITS: usize = bits_for::(); + const BITS: u32 = bits_for::(); #[inline] fn into_ptr(self) -> NonNull { @@ -223,14 +223,8 @@ unsafe impl<'a, T: 'a + ?Sized + Aligned> Pointer for &'a mut T { /// Returns the number of bits available for use for tags in a pointer to `T` /// (this is based on `T`'s alignment). -pub const fn bits_for() -> usize { - let bits = crate::aligned::align_of::().trailing_zeros(); - - // This is a replacement for `.try_into().unwrap()` unavailable in `const` - // (it's fine to make an assert here, since this is only called in compile time) - assert!((bits as u128) < usize::MAX as u128); - - bits as usize +pub const fn bits_for() -> u32 { + crate::aligned::align_of::().as_nonzero().trailing_zeros() } /// A tag type used in [`CopyTaggedPtr`] and [`TaggedPtr`] tests. @@ -245,7 +239,7 @@ enum Tag2 { #[cfg(test)] unsafe impl Tag for Tag2 { - const BITS: usize = 2; + const BITS: u32 = 2; fn into_usize(self) -> usize { self as _ diff --git a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs index 83dfdb0bd876d..691e92f196a26 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/copy.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/copy.rs @@ -116,7 +116,7 @@ where self.packed = Self::pack(self.pointer_raw(), tag); } - const TAG_BIT_SHIFT: usize = usize::BITS as usize - T::BITS; + const TAG_BIT_SHIFT: u32 = usize::BITS - T::BITS; const ASSERTION: () = { assert!(T::BITS <= P::BITS) }; /// Pack pointer `ptr` that comes from [`P::into_ptr`] with a `tag`, @@ -298,7 +298,7 @@ where /// enum Tag2 { B00 = 0b00, B01 = 0b01, B10 = 0b10, B11 = 0b11 }; /// /// unsafe impl Tag for Tag2 { -/// const BITS: usize = 2; +/// const BITS: u32 = 2; /// /// fn into_usize(self) -> usize { todo!() } /// unsafe fn from_usize(tag: usize) -> Self { todo!() } diff --git a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs index 6ca6c7d1283b4..d418c06b7ebb4 100644 --- a/compiler/rustc_data_structures/src/tagged_ptr/drop.rs +++ b/compiler/rustc_data_structures/src/tagged_ptr/drop.rs @@ -150,7 +150,7 @@ where /// enum Tag2 { B00 = 0b00, B01 = 0b01, B10 = 0b10, B11 = 0b11 }; /// /// unsafe impl Tag for Tag2 { -/// const BITS: usize = 2; +/// const BITS: u32 = 2; /// /// fn into_usize(self) -> usize { todo!() } /// unsafe fn from_usize(tag: usize) -> Self { todo!() } diff --git a/compiler/rustc_middle/src/lib.rs b/compiler/rustc_middle/src/lib.rs index b4edb02f6c48d..51ed66dbafcc4 100644 --- a/compiler/rustc_middle/src/lib.rs +++ b/compiler/rustc_middle/src/lib.rs @@ -59,6 +59,7 @@ #![feature(result_option_inspect)] #![feature(const_option)] #![feature(trait_alias)] +#![feature(ptr_alignment_type)] #![recursion_limit = "512"] #![allow(rustc::potential_query_instability)] diff --git a/compiler/rustc_middle/src/ty/list.rs b/compiler/rustc_middle/src/ty/list.rs index 590beef7f7d42..30f036e471c42 100644 --- a/compiler/rustc_middle/src/ty/list.rs +++ b/compiler/rustc_middle/src/ty/list.rs @@ -1,5 +1,5 @@ use crate::arena::Arena; -use rustc_data_structures::aligned::Aligned; +use rustc_data_structures::aligned::{align_of, Aligned}; use rustc_serialize::{Encodable, Encoder}; use std::alloc::Layout; use std::cmp::Ordering; @@ -203,13 +203,13 @@ unsafe impl Sync for List {} // Layouts of `Equivalent` and `List` are the same, modulo opaque tail, // thus aligns of `Equivalent` and `List` must be the same. unsafe impl Aligned for List { - const ALIGN: usize = { + const ALIGN: ptr::Alignment = { #[repr(C)] struct Equivalent { _len: usize, _data: [T; 0], } - mem::align_of::>() + align_of::>() }; } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index c856bb25e1474..3d47f56d03122 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1626,7 +1626,8 @@ struct ParamTag { } unsafe impl rustc_data_structures::tagged_ptr::Tag for ParamTag { - const BITS: usize = 2; + const BITS: u32 = 2; + #[inline] fn into_usize(self) -> usize { match self { @@ -1636,6 +1637,7 @@ unsafe impl rustc_data_structures::tagged_ptr::Tag for ParamTag { Self { reveal: traits::Reveal::All, constness: hir::Constness::Const } => 3, } } + #[inline] unsafe fn from_usize(ptr: usize) -> Self { match ptr { From 5571dd061dac07644b9d9a3e799deefbd9971496 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 14 Apr 2023 13:04:58 +0000 Subject: [PATCH 22/22] fix broken intradoclinks --- compiler/rustc_data_structures/src/aligned.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_data_structures/src/aligned.rs b/compiler/rustc_data_structures/src/aligned.rs index 7ac073539fb4b..0e5ecfd9bff6e 100644 --- a/compiler/rustc_data_structures/src/aligned.rs +++ b/compiler/rustc_data_structures/src/aligned.rs @@ -4,6 +4,8 @@ use std::ptr::Alignment; /// /// This is equivalent to [`mem::align_of`], but also works for some unsized /// types (e.g. slices or rustc's `List`s). +/// +/// [`mem::align_of`]: std::mem::align_of pub const fn align_of() -> Alignment { T::ALIGN } @@ -16,7 +18,7 @@ pub const fn align_of() -> Alignment { /// is [`mem::align_of()`], for unsized types it depends on the type, for /// example `[T]` has alignment of `T`. /// -/// [`mem::align_of()`]: mem::align_of +/// [`mem::align_of()`]: std::mem::align_of pub unsafe trait Aligned { /// Alignment of `Self`. const ALIGN: Alignment;