Rollup merge of rust-lang#93147 - nnethercote:interner-cleanups, r=lcnr

Interner cleanups

Improve some code that I have found confusing.

r? `@lcnr`

Author: matthiaskrgr

compiler/rustc_middle/src/ty/context.rs

@@ -113,6 +113,12 @@ pub struct CtxtInterners<'tcx> {
     bound_variable_kinds: InternedSet<'tcx, List<ty::BoundVariableKind>>,
     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<interpret::AllocMap<'tcx>>,
 
@@ -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),
         }
@@ -1952,11 +1942,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,
@@ -1973,24 +1963,37 @@ 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> {
     fn clone(&self) -> Self {
         Interned(self.0)
     }
 }
+
 impl<'tcx, T: 'tcx + ?Sized> Copy for Interned<'tcx, T> {}
 
 impl<'tcx, T: 'tcx + ?Sized> IntoPointer for Interned<'tcx, T> {
     fn into_pointer(&self) -> *const () {
         self.0 as *const _ as *const ()
     }
 }
-// N.B., an `Interned<Ty>` compares and hashes as a `TyKind`.
+
+#[allow(rustc::usage_of_ty_tykind)]
+impl<'tcx> Borrow<TyKind<'tcx>> 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()
     }
 }
@@ -1999,19 +2002,21 @@ impl<'tcx> Eq for Interned<'tcx, TyS<'tcx>> {}
 
 impl<'tcx> Hash for Interned<'tcx, TyS<'tcx>> {
     fn hash<H: Hasher>(&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<TyKind<'tcx>> for Interned<'tcx, TyS<'tcx>> {
-    fn borrow<'a>(&'a self) -> &'a TyKind<'tcx> {
-        &self.0.kind()
+impl<'tcx> Borrow<Binder<'tcx, PredicateKind<'tcx>>> for Interned<'tcx, PredicateInner<'tcx>> {
+    fn borrow<'a>(&'a self) -> &'a Binder<'tcx, PredicateKind<'tcx>> {
+        &self.0.kind
     }
 }
-// N.B., an `Interned<PredicateInner>` 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
     }
 }
@@ -2020,19 +2025,21 @@ impl<'tcx> Eq for Interned<'tcx, PredicateInner<'tcx>> {}
 
 impl<'tcx> Hash for Interned<'tcx, PredicateInner<'tcx>> {
     fn hash<H: Hasher>(&self, s: &mut H) {
+        // The `Borrow` trait requires that `x.borrow().hash(s) == x.hash(s)`.
         self.0.kind.hash(s)
     }
 }
 
-impl<'tcx> Borrow<Binder<'tcx, PredicateKind<'tcx>>> 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<T>> {
+    fn borrow<'a>(&'a self) -> &'a [T] {
+        &self.0[..]
     }
 }
 
-// N.B., an `Interned<List<T>>` compares and hashes as its elements.
 impl<'tcx, T: PartialEq> PartialEq for Interned<'tcx, List<T>> {
     fn eq(&self, other: &Interned<'tcx, List<T>>) -> bool {
+        // The `Borrow` trait requires that `x.borrow() == y.borrow()` equals
+        // `x == y`.
         self.0[..] == other.0[..]
     }
 }
@@ -2041,20 +2048,23 @@ impl<'tcx, T: Eq> Eq for Interned<'tcx, List<T>> {}
 
 impl<'tcx, T: Hash> Hash for Interned<'tcx, List<T>> {
     fn hash<H: Hasher>(&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<T>> {
-    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
             }
         }
@@ -2063,16 +2073,12 @@ macro_rules! direct_interners {
 
         impl<'tcx> Hash for Interned<'tcx, $ty> {
             fn hash<H: Hasher>(&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| {
@@ -2089,6 +2095,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 {

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,13 +449,17 @@ 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)
     }
 }
 impl<'tcx> Eq for TyS<'tcx> {}
 
 impl<'tcx> Hash for TyS<'tcx> {
     fn hash<H: Hasher>(&self, s: &mut H) {
+        // Pointer hashing is sufficient (due to the unique contents
+        // assumption).
         (self as *const TyS<'_>).hash(s)
     }
 }