From c55819ae606046e2a7b6b92c3f0821bd171f34b0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 21 Jan 2022 10:14:18 +1100 Subject: [PATCH 1/2] Make stability interning follow the usual pattern. --- compiler/rustc_middle/src/ty/context.rs | 32 ++++++++++--------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index e7b99995ca4ae..de435b8f58c72 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -113,6 +113,12 @@ pub struct CtxtInterners<'tcx> { bound_variable_kinds: InternedSet<'tcx, List>, layout: InternedSet<'tcx, Layout>, adt_def: InternedSet<'tcx, AdtDef>, + + /// `#[stable]` and `#[unstable]` attributes + stability: InternedSet<'tcx, attr::Stability>, + + /// `#[rustc_const_stable]` and `#[rustc_const_unstable]` attributes + const_stability: InternedSet<'tcx, attr::ConstStability>, } impl<'tcx> CtxtInterners<'tcx> { @@ -134,6 +140,8 @@ impl<'tcx> CtxtInterners<'tcx> { bound_variable_kinds: Default::default(), layout: Default::default(), adt_def: Default::default(), + stability: Default::default(), + const_stability: Default::default(), } } @@ -1035,12 +1043,6 @@ pub struct GlobalCtxt<'tcx> { /// Data layout specification for the current target. pub data_layout: TargetDataLayout, - /// `#[stable]` and `#[unstable]` attributes - stability_interner: ShardedHashMap<&'tcx attr::Stability, ()>, - - /// `#[rustc_const_stable]` and `#[rustc_const_unstable]` attributes - const_stability_interner: ShardedHashMap<&'tcx attr::ConstStability, ()>, - /// Stores memory for globals (statics/consts). pub(crate) alloc_map: Lock>, @@ -1092,16 +1094,6 @@ impl<'tcx> TyCtxt<'tcx> { self.create_memory_alloc(alloc) } - // FIXME(eddyb) move to `direct_interners!`. - pub fn intern_stability(self, stab: attr::Stability) -> &'tcx attr::Stability { - self.stability_interner.intern(stab, |stab| self.arena.alloc(stab)) - } - - // FIXME(eddyb) move to `direct_interners!`. - pub fn intern_const_stability(self, stab: attr::ConstStability) -> &'tcx attr::ConstStability { - self.const_stability_interner.intern(stab, |stab| self.arena.alloc(stab)) - } - /// Returns a range of the start/end indices specified with the /// `rustc_layout_scalar_valid_range` attribute. // FIXME(eddyb) this is an awkward spot for this method, maybe move it? @@ -1185,8 +1177,6 @@ impl<'tcx> TyCtxt<'tcx> { evaluation_cache: Default::default(), crate_name: Symbol::intern(crate_name), data_layout, - stability_interner: Default::default(), - const_stability_interner: Default::default(), alloc_map: Lock::new(interpret::AllocMap::new()), output_filenames: Arc::new(output_filenames), } @@ -1935,11 +1925,11 @@ impl<'tcx> TyCtxt<'tcx> { writeln!(fmt, "InternalSubsts interner: #{}", self.0.interners.substs.len())?; writeln!(fmt, "Region interner: #{}", self.0.interners.region.len())?; - writeln!(fmt, "Stability interner: #{}", self.0.stability_interner.len())?; + writeln!(fmt, "Stability interner: #{}", self.0.interners.stability.len())?; writeln!( fmt, "Const Stability interner: #{}", - self.0.const_stability_interner.len() + self.0.interners.const_stability.len() )?; writeln!( fmt, @@ -2072,6 +2062,8 @@ direct_interners! { const_allocation: intern_const_alloc(Allocation), layout: intern_layout(Layout), adt_def: intern_adt_def(AdtDef), + stability: intern_stability(attr::Stability), + const_stability: intern_const_stability(attr::ConstStability), } macro_rules! slice_interners { From d46ed5d3333770ac490d587878d708c3be17f137 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 21 Jan 2022 13:25:26 +1100 Subject: [PATCH 2/2] Clarify some code relating to interning and types. I have found this code very confusing at times. This commit clarifies things. In particular, the commit explains the requirements that the `Borrow` impls put on the `Eq` and `Hash` impls, which are non-obvious. And it puts the `Borrow` impls first, since they force `Eq` and `Hash` to have particular forms. The commit also notes `TyS`'s uniqueness requirements. --- compiler/rustc_middle/src/ty/context.rs | 64 +++++++++++++++---------- compiler/rustc_middle/src/ty/mod.rs | 17 +++++++ 2 files changed, 57 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index de435b8f58c72..7b7c5fa2436ae 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -1946,7 +1946,10 @@ impl<'tcx> TyCtxt<'tcx> { } } -/// An entry in an interner. +// This type holds a `T` in the interner. The `T` is stored in the arena and +// this type just holds a pointer to it, but it still effectively owns it. It +// impls `Borrow` so that it can be looked up using the original +// (non-arena-memory-owning) types. struct Interned<'tcx, T: ?Sized>(&'tcx T); impl<'tcx, T: 'tcx + ?Sized> Clone for Interned<'tcx, T> { @@ -1954,6 +1957,7 @@ impl<'tcx, T: 'tcx + ?Sized> Clone for Interned<'tcx, T> { Interned(self.0) } } + impl<'tcx, T: 'tcx + ?Sized> Copy for Interned<'tcx, T> {} impl<'tcx, T: 'tcx + ?Sized> IntoPointer for Interned<'tcx, T> { @@ -1961,9 +1965,18 @@ impl<'tcx, T: 'tcx + ?Sized> IntoPointer for Interned<'tcx, T> { self.0 as *const _ as *const () } } -// N.B., an `Interned` compares and hashes as a `TyKind`. + +#[allow(rustc::usage_of_ty_tykind)] +impl<'tcx> Borrow> for Interned<'tcx, TyS<'tcx>> { + fn borrow<'a>(&'a self) -> &'a TyKind<'tcx> { + &self.0.kind() + } +} + impl<'tcx> PartialEq for Interned<'tcx, TyS<'tcx>> { fn eq(&self, other: &Interned<'tcx, TyS<'tcx>>) -> bool { + // The `Borrow` trait requires that `x.borrow() == y.borrow()` equals + // `x == y`. self.0.kind() == other.0.kind() } } @@ -1972,19 +1985,21 @@ impl<'tcx> Eq for Interned<'tcx, TyS<'tcx>> {} impl<'tcx> Hash for Interned<'tcx, TyS<'tcx>> { fn hash(&self, s: &mut H) { + // The `Borrow` trait requires that `x.borrow().hash(s) == x.hash(s)`. self.0.kind().hash(s) } } -#[allow(rustc::usage_of_ty_tykind)] -impl<'tcx> Borrow> for Interned<'tcx, TyS<'tcx>> { - fn borrow<'a>(&'a self) -> &'a TyKind<'tcx> { - &self.0.kind() +impl<'tcx> Borrow>> for Interned<'tcx, PredicateInner<'tcx>> { + fn borrow<'a>(&'a self) -> &'a Binder<'tcx, PredicateKind<'tcx>> { + &self.0.kind } } -// N.B., an `Interned` compares and hashes as a `PredicateKind`. + impl<'tcx> PartialEq for Interned<'tcx, PredicateInner<'tcx>> { fn eq(&self, other: &Interned<'tcx, PredicateInner<'tcx>>) -> bool { + // The `Borrow` trait requires that `x.borrow() == y.borrow()` equals + // `x == y`. self.0.kind == other.0.kind } } @@ -1993,19 +2008,21 @@ impl<'tcx> Eq for Interned<'tcx, PredicateInner<'tcx>> {} impl<'tcx> Hash for Interned<'tcx, PredicateInner<'tcx>> { fn hash(&self, s: &mut H) { + // The `Borrow` trait requires that `x.borrow().hash(s) == x.hash(s)`. self.0.kind.hash(s) } } -impl<'tcx> Borrow>> for Interned<'tcx, PredicateInner<'tcx>> { - fn borrow<'a>(&'a self) -> &'a Binder<'tcx, PredicateKind<'tcx>> { - &self.0.kind +impl<'tcx, T> Borrow<[T]> for Interned<'tcx, List> { + fn borrow<'a>(&'a self) -> &'a [T] { + &self.0[..] } } -// N.B., an `Interned>` compares and hashes as its elements. impl<'tcx, T: PartialEq> PartialEq for Interned<'tcx, List> { fn eq(&self, other: &Interned<'tcx, List>) -> bool { + // The `Borrow` trait requires that `x.borrow() == y.borrow()` equals + // `x == y`. self.0[..] == other.0[..] } } @@ -2014,20 +2031,23 @@ impl<'tcx, T: Eq> Eq for Interned<'tcx, List> {} impl<'tcx, T: Hash> Hash for Interned<'tcx, List> { fn hash(&self, s: &mut H) { + // The `Borrow` trait requires that `x.borrow().hash(s) == x.hash(s)`. self.0[..].hash(s) } } -impl<'tcx, T> Borrow<[T]> for Interned<'tcx, List> { - fn borrow<'a>(&'a self) -> &'a [T] { - &self.0[..] - } -} - macro_rules! direct_interners { ($($name:ident: $method:ident($ty:ty),)+) => { - $(impl<'tcx> PartialEq for Interned<'tcx, $ty> { + $(impl<'tcx> Borrow<$ty> for Interned<'tcx, $ty> { + fn borrow<'a>(&'a self) -> &'a $ty { + &self.0 + } + } + + impl<'tcx> PartialEq for Interned<'tcx, $ty> { fn eq(&self, other: &Self) -> bool { + // The `Borrow` trait requires that `x.borrow() == y.borrow()` + // equals `x == y`. self.0 == other.0 } } @@ -2036,16 +2056,12 @@ macro_rules! direct_interners { impl<'tcx> Hash for Interned<'tcx, $ty> { fn hash(&self, s: &mut H) { + // The `Borrow` trait requires that `x.borrow().hash(s) == + // x.hash(s)`. self.0.hash(s) } } - impl<'tcx> Borrow<$ty> for Interned<'tcx, $ty> { - fn borrow<'a>(&'a self) -> &'a $ty { - &self.0 - } - } - impl<'tcx> TyCtxt<'tcx> { pub fn $method(self, v: $ty) -> &'tcx $ty { self.interners.$name.intern(v, |v| { diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index d6e89e52b95ed..00af16e79842c 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -376,15 +376,28 @@ pub struct CReaderCacheKey { pub pos: usize, } +/// Represents a type. +/// +/// IMPORTANT: Every `TyS` is *required* to have unique contents. The type's +/// correctness relies on this, *but it does not enforce it*. Therefore, any +/// code that creates a `TyS` must ensure uniqueness itself. In practice this +/// is achieved by interning. #[allow(rustc::usage_of_ty_tykind)] pub struct TyS<'tcx> { /// This field shouldn't be used directly and may be removed in the future. /// Use `TyS::kind()` instead. kind: TyKind<'tcx>, + + /// This field provides fast access to information that is also contained + /// in `kind`. + /// /// This field shouldn't be used directly and may be removed in the future. /// Use `TyS::flags()` instead. flags: TypeFlags, + /// This field provides fast access to information that is also contained + /// in `kind`. + /// /// This is a kind of confusing thing: it stores the smallest /// binder such that /// @@ -436,6 +449,8 @@ impl<'tcx> PartialOrd for TyS<'tcx> { impl<'tcx> PartialEq for TyS<'tcx> { #[inline] fn eq(&self, other: &TyS<'tcx>) -> bool { + // Pointer equality implies equality (due to the unique contents + // assumption). ptr::eq(self, other) } } @@ -443,6 +458,8 @@ impl<'tcx> Eq for TyS<'tcx> {} impl<'tcx> Hash for TyS<'tcx> { fn hash(&self, s: &mut H) { + // Pointer hashing is sufficient (due to the unique contents + // assumption). (self as *const TyS<'_>).hash(s) } }