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.
This commit is contained in:
Nicholas Nethercote 2023-02-16 12:06:36 +11:00
parent 9d5cf0f0bf
commit c8237db3ee
2 changed files with 87 additions and 70 deletions

View File

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

View File

@ -69,38 +69,37 @@ pub trait Interner: Sized {
type PlaceholderRegion: Clone + Debug + Hash + Ord; 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; 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 where
I: Iterator<Item = Self>,
F: FnOnce(&[T]) -> R; F: FnOnce(&[T]) -> R;
} }
impl<I, T, R, E> InternAs<T, R> for I /// The blanket impl that always collects all elements and applies `f`.
where impl<T, R> CollectAndApply<T, R> for T {
E: InternIteratorElement<T, R>, type Output = R;
I: Iterator<Item = E>,
{ /// Equivalent to `f(&iter.collect::<Vec<_>>())`.
type Output = E::Output; fn collect_and_apply<I, F>(mut iter: I, f: F) -> R
fn intern_with<F>(self, f: F) -> Self::Output
where where
I: Iterator<Item = T>,
F: FnOnce(&[T]) -> R, 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 // This code is hot enough that it's worth specializing for the most
// common length lists, to avoid the overhead of `SmallVec` creation. // common length lists, to avoid the overhead of `SmallVec` creation.
// Lengths 0, 1, and 2 typically account for ~95% of cases. If // 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>; type Output = Result<R, E>;
fn intern_with<I: Iterator<Item = Self>, F: FnOnce(&[T]) -> R>(
mut iter: I, /// Equivalent to `Ok(f(&iter.collect::<Result<Vec<_>>>()?))`.
f: F, fn collect_and_apply<I, F>(mut iter: I, f: F) -> Result<R, E>
) -> Self::Output { where
I: Iterator<Item = Result<T, E>>,
F: FnOnce(&[T]) -> R,
{
// This code is hot enough that it's worth specializing for the most // This code is hot enough that it's worth specializing for the most
// common length lists, to avoid the overhead of `SmallVec` creation. // common length lists, to avoid the overhead of `SmallVec` creation.
// Lengths 0, 1, and 2 typically account for ~95% of cases. If // Lengths 0, 1, and 2 typically account for ~95% of cases. If