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

@@ -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,20 +1965,6 @@ impl<'tcx, T: 'tcx + ?Sized> IntoPointer for Interned<'tcx, T> {
         self.0 as *const _ as *const ()
     }
 }
-// N.B., an `Interned<Ty>` compares and hashes as a `TyKind`.
-impl<'tcx> PartialEq for Interned<'tcx, TyS<'tcx>> {
-    fn eq(&self, other: &Interned<'tcx, TyS<'tcx>>) -> bool {
-        self.0.kind() == other.0.kind()
-    }
-}
-
-impl<'tcx> Eq for Interned<'tcx, TyS<'tcx>> {}
-
-impl<'tcx> Hash for Interned<'tcx, TyS<'tcx>> {
-    fn hash<H: Hasher>(&self, s: &mut H) {
-        self.0.kind().hash(s)
-    }
-}
 
 #[allow(rustc::usage_of_ty_tykind)]
 impl<'tcx> Borrow<TyKind<'tcx>> for Interned<'tcx, TyS<'tcx>> {
@@ -1982,18 +1972,21 @@ impl<'tcx> Borrow<TyKind<'tcx>> for Interned<'tcx, TyS<'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 {
-        self.0.kind == other.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()
     }
 }
 
-impl<'tcx> Eq for Interned<'tcx, PredicateInner<'tcx>> {}
+impl<'tcx> Eq for Interned<'tcx, TyS<'tcx>> {}
 
-impl<'tcx> Hash for Interned<'tcx, PredicateInner<'tcx>> {
+impl<'tcx> Hash for Interned<'tcx, TyS<'tcx>> {
     fn hash<H: Hasher>(&self, s: &mut H) {
-        self.0.kind.hash(s)
+        // The `Borrow` trait requires that `x.borrow().hash(s) == x.hash(s)`.
+        self.0.kind().hash(s)
     }
 }
 
@@ -2003,18 +1996,20 @@ impl<'tcx> Borrow<Binder<'tcx, PredicateKind<'tcx>>> for Interned<'tcx, Predicat
     }
 }
 
-// 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 {
-        self.0[..] == other.0[..]
+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
     }
 }
 
-impl<'tcx, T: Eq> Eq for Interned<'tcx, List<T>> {}
+impl<'tcx> Eq for Interned<'tcx, PredicateInner<'tcx>> {}
 
-impl<'tcx, T: Hash> Hash for Interned<'tcx, List<T>> {
+impl<'tcx> Hash for Interned<'tcx, PredicateInner<'tcx>> {
     fn hash<H: Hasher>(&self, s: &mut H) {
-        self.0[..].hash(s)
+        // The `Borrow` trait requires that `x.borrow().hash(s) == x.hash(s)`.
+        self.0.kind.hash(s)
     }
 }
 
@@ -2024,10 +2019,35 @@ impl<'tcx, T> Borrow<[T]> for Interned<'tcx, List<T>> {
     }
 }
 
+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[..]
+    }
+}
+
+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)
+    }
+}
+
 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<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| {

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