Clarify iterator interners.

There are two traits, `InternAs` and `InternIteratorElement`. I found
them confusing to use, particularly this:
```
pub fn mk_tup<I: InternAs<Ty<'tcx>, Ty<'tcx>>>(self, iter: I) -> I::Output {
    iter.intern_with(|ts| self.intern_tup(ts))
}
```
where I thought there might have been two levels of interning going on
(there isn't) due to the `intern_with`/`InternAs` + `intern_tup` naming.

And then I found the actual traits and impls themselves *very*
confusing.
- `InternAs` has a single impl, for iterators, with four type variables.
- `InternAs` is only implemented for iterators because it wouldn't
  really make sense to implement for any other type. And you can't
  really understand the trait without seeing that single impl, which is
  suspicious.
- `InternAs` is basically just a wrapper for `InternIteratorElement`
  which does all the actual work.
- Neither trait actually does any interning. They just have `Intern` in
  their name because they are used *by* interning code.
- There are no comments.

So this commit improves things.
- It removes `InternAs` completely. This makes the `mk_*` function
  signatures slightly more verbose -- two trait bounds instead of one --
  but much easier to read, because you only need to understand one trait
  instead of two.
- It renames `InternIteratorElement` as `CollectAndApply`. Likewise, it
  renames its method `intern_with` as `collect_and_apply`. These names
  describe better what's going on: we collect the iterator elements into
  a slice and then apply a function to the slice.
- It adds comments, making clear that all this is all there just to
  provide an optimized version of `f(&iter.collect::<Vec<_>>())`.

It took me a couple of attempts to come up with this commit. My initial
attempt kept `InternAs` around, but renamed things and added comments,
and I wasn't happy with it. I think this version is much better. The
resulting code is shorter, despite the addition of the comments.

Author: nnethercote

compiler/rustc_middle/src/ty/context.rs

@@ -67,7 +67,7 @@ use rustc_target::abi::{Layout, LayoutS, TargetDataLayout, VariantIdx};
 use rustc_target::spec::abi;
 use rustc_type_ir::sty::TyKind::*;
 use rustc_type_ir::WithCachedTypeInfo;
-use rustc_type_ir::{DynKind, InternAs, InternIteratorElement, Interner, TypeFlags};
+use rustc_type_ir::{CollectAndApply, DynKind, Interner, TypeFlags};
 
 use std::any::Any;
 use std::borrow::Borrow;
@@ -1835,8 +1835,12 @@ impl<'tcx> TyCtxt<'tcx> {
         if ts.is_empty() { self.types.unit } else { self.mk_ty(Tuple(self.intern_type_list(&ts))) }
     }
 
-    pub fn mk_tup<I: InternAs<Ty<'tcx>, Ty<'tcx>>>(self, iter: I) -> I::Output {
-        iter.intern_with(|ts| self.intern_tup(ts))
+    pub fn mk_tup<I, T>(self, iter: I) -> T::Output
+    where
+        I: Iterator<Item = T>,
+        T: CollectAndApply<Ty<'tcx>, Ty<'tcx>>,
+    {
+        T::collect_and_apply(iter, |ts| self.intern_tup(ts))
     }
 
     #[inline]
@@ -2157,11 +2161,12 @@ impl<'tcx> TyCtxt<'tcx> {
         }
     }
 
-    pub fn mk_const_list<I: InternAs<ty::Const<'tcx>, &'tcx List<ty::Const<'tcx>>>>(
-        self,
-        iter: I,
-    ) -> I::Output {
-        iter.intern_with(|xs| self.intern_const_list(xs))
+    pub fn mk_const_list<I, T>(self, iter: I) -> T::Output
+    where
+        I: Iterator<Item = T>,
+        T: CollectAndApply<ty::Const<'tcx>, &'tcx List<ty::Const<'tcx>>>,
+    {
+        T::collect_and_apply(iter, |xs| self.intern_const_list(xs))
     }
 
     pub fn intern_const_list(self, cs: &[ty::Const<'tcx>]) -> &'tcx List<ty::Const<'tcx>> {
@@ -2220,48 +2225,57 @@ impl<'tcx> TyCtxt<'tcx> {
     ) -> T::Output
     where
         I: Iterator<Item = T>,
-        T: InternIteratorElement<Ty<'tcx>, ty::FnSig<'tcx>>,
+        T: CollectAndApply<Ty<'tcx>, ty::FnSig<'tcx>>,
     {
-        inputs.chain(iter::once(output)).intern_with(|xs| ty::FnSig {
+        T::collect_and_apply(inputs.chain(iter::once(output)), |xs| ty::FnSig {
             inputs_and_output: self.intern_type_list(xs),
             c_variadic,
             unsafety,
             abi,
         })
     }
 
-    pub fn mk_poly_existential_predicates<
-        I: InternAs<PolyExistentialPredicate<'tcx>, &'tcx List<PolyExistentialPredicate<'tcx>>>,
-    >(
-        self,
-        iter: I,
-    ) -> I::Output {
-        iter.intern_with(|xs| self.intern_poly_existential_predicates(xs))
+    pub fn mk_poly_existential_predicates<I, T>(self, iter: I) -> T::Output
+    where
+        I: Iterator<Item = T>,
+        T: CollectAndApply<
+                PolyExistentialPredicate<'tcx>,
+                &'tcx List<PolyExistentialPredicate<'tcx>>,
+            >,
+    {
+        T::collect_and_apply(iter, |xs| self.intern_poly_existential_predicates(xs))
     }
 
-    pub fn mk_predicates<I: InternAs<Predicate<'tcx>, &'tcx List<Predicate<'tcx>>>>(
-        self,
-        iter: I,
-    ) -> I::Output {
-        iter.intern_with(|xs| self.intern_predicates(xs))
+    pub fn mk_predicates<I, T>(self, iter: I) -> T::Output
+    where
+        I: Iterator<Item = T>,
+        T: CollectAndApply<Predicate<'tcx>, &'tcx List<Predicate<'tcx>>>,
+    {
+        T::collect_and_apply(iter, |xs| self.intern_predicates(xs))
     }
 
-    pub fn mk_type_list<I: InternAs<Ty<'tcx>, &'tcx List<Ty<'tcx>>>>(self, iter: I) -> I::Output {
-        iter.intern_with(|xs| self.intern_type_list(xs))
+    pub fn mk_type_list<I, T>(self, iter: I) -> T::Output
+    where
+        I: Iterator<Item = T>,
+        T: CollectAndApply<Ty<'tcx>, &'tcx List<Ty<'tcx>>>,
+    {
+        T::collect_and_apply(iter, |xs| self.intern_type_list(xs))
     }
 
-    pub fn mk_substs<I: InternAs<GenericArg<'tcx>, &'tcx List<GenericArg<'tcx>>>>(
-        self,
-        iter: I,
-    ) -> I::Output {
-        iter.intern_with(|xs| self.intern_substs(xs))
+    pub fn mk_substs<I, T>(self, iter: I) -> T::Output
+    where
+        I: Iterator<Item = T>,
+        T: CollectAndApply<GenericArg<'tcx>, &'tcx List<GenericArg<'tcx>>>,
+    {
+        T::collect_and_apply(iter, |xs| self.intern_substs(xs))
     }
 
-    pub fn mk_place_elems<I: InternAs<PlaceElem<'tcx>, &'tcx List<PlaceElem<'tcx>>>>(
-        self,
-        iter: I,
-    ) -> I::Output {
-        iter.intern_with(|xs| self.intern_place_elems(xs))
+    pub fn mk_place_elems<I, T>(self, iter: I) -> T::Output
+    where
+        I: Iterator<Item = T>,
+        T: CollectAndApply<PlaceElem<'tcx>, &'tcx List<PlaceElem<'tcx>>>,
+    {
+        T::collect_and_apply(iter, |xs| self.intern_place_elems(xs))
     }
 
     pub fn mk_substs_trait(
@@ -2290,13 +2304,12 @@ impl<'tcx> TyCtxt<'tcx> {
         ty::AliasTy { def_id, substs, _use_mk_alias_ty_instead: () }
     }
 
-    pub fn mk_bound_variable_kinds<
-        I: InternAs<ty::BoundVariableKind, &'tcx List<ty::BoundVariableKind>>,
-    >(
-        self,
-        iter: I,
-    ) -> I::Output {
-        iter.intern_with(|xs| self.intern_bound_variable_kinds(xs))
+    pub fn mk_bound_variable_kinds<I, T>(self, iter: I) -> T::Output
+    where
+        I: Iterator<Item = T>,
+        T: CollectAndApply<ty::BoundVariableKind, &'tcx List<ty::BoundVariableKind>>,
+    {
+        T::collect_and_apply(iter, |xs| self.intern_bound_variable_kinds(xs))
     }
 
     /// Emit a lint at `span` from a lint struct (some type that implements `DecorateLint`,

compiler/rustc_type_ir/src/lib.rs

@@ -69,38 +69,37 @@ pub trait Interner: Sized {
     type PlaceholderRegion: Clone + Debug + Hash + Ord;
 }
 
-pub trait InternAs<T: ?Sized, R> {
+/// Imagine you have a function `F: FnOnce(&[T]) -> R`, plus an iterator `iter`
+/// that produces `T` items. You could combine them with
+/// `f(&iter.collect::<Vec<_>>())`, but this requires allocating memory for the
+/// `Vec`.
+///
+/// This trait allows for faster implementations, intended for cases where the
+/// number of items produced by the iterator is small. There is a blanket impl
+/// for `T` items, but there is also a fallible impl for `Result<T, E>` items.
+pub trait CollectAndApply<T, R>: Sized {
     type Output;
-    fn intern_with<F>(self, f: F) -> Self::Output
+
+    /// Produce a result of type `Self::Output` from `iter`. The result will
+    /// typically be produced by applying `f` on the elements produced by
+    /// `iter`, though this may not happen in some impls, e.g. if an error
+    /// occured during iteration.
+    fn collect_and_apply<I, F>(iter: I, f: F) -> Self::Output
     where
+        I: Iterator<Item = Self>,
         F: FnOnce(&[T]) -> R;
 }
 
-impl<I, T, R, E> InternAs<T, R> for I
-where
-    E: InternIteratorElement<T, R>,
-    I: Iterator<Item = E>,
-{
-    type Output = E::Output;
-    fn intern_with<F>(self, f: F) -> Self::Output
+/// The blanket impl that always collects all elements and applies `f`.
+impl<T, R> CollectAndApply<T, R> for T {
+    type Output = R;
+
+    /// Equivalent to `f(&iter.collect::<Vec<_>>())`.
+    fn collect_and_apply<I, F>(mut iter: I, f: F) -> R
     where
+        I: Iterator<Item = T>,
         F: FnOnce(&[T]) -> R,
     {
-        E::intern_with(self, f)
-    }
-}
-
-pub trait InternIteratorElement<T, R>: Sized {
-    type Output;
-    fn intern_with<I: Iterator<Item = Self>, F: FnOnce(&[T]) -> R>(iter: I, f: F) -> Self::Output;
-}
-
-impl<T, R> InternIteratorElement<T, R> for T {
-    type Output = R;
-    fn intern_with<I: Iterator<Item = Self>, F: FnOnce(&[T]) -> R>(
-        mut iter: I,
-        f: F,
-    ) -> Self::Output {
         // This code is hot enough that it's worth specializing for the most
         // common length lists, to avoid the overhead of `SmallVec` creation.
         // Lengths 0, 1, and 2 typically account for ~95% of cases. If
@@ -127,12 +126,17 @@ impl<T, R> InternIteratorElement<T, R> for T {
     }
 }
 
-impl<T, R, E> InternIteratorElement<T, R> for Result<T, E> {
+/// A fallible impl that will fail, without calling `f`, if there are any
+/// errors during collection.
+impl<T, R, E> CollectAndApply<T, R> for Result<T, E> {
     type Output = Result<R, E>;
-    fn intern_with<I: Iterator<Item = Self>, F: FnOnce(&[T]) -> R>(
-        mut iter: I,
-        f: F,
-    ) -> Self::Output {
+
+    /// Equivalent to `Ok(f(&iter.collect::<Result<Vec<_>>>()?))`.
+    fn collect_and_apply<I, F>(mut iter: I, f: F) -> Result<R, E>
+    where
+        I: Iterator<Item = Result<T, E>>,
+        F: FnOnce(&[T]) -> R,
+    {
         // This code is hot enough that it's worth specializing for the most
         // common length lists, to avoid the overhead of `SmallVec` creation.
         // Lengths 0, 1, and 2 typically account for ~95% of cases. If