kpreid / exhaust

Exhaustive iteration trait in Rust
https://docs.rs/exhaust/
Apache License 2.0
14 stars 2 forks source link

Remove `trait Exhaust: Clone` supertrait bound? #6

Closed kpreid closed 1 month ago

kpreid commented 2 years ago

It turns out that several standard library types don't implement Clone; notably, std::sync::Mutex. That may not be the best example of a type to implement Exhaust, but in any case, this is evidence that trait Exhaust: Clone is not ideal.

First idea: never clone an item, but clone the iterators instead. Problem with that: a Peekable iterator only implements Clone if the item type does, since it potentially holds an item. So, the algorithm for iterator combinators would need to change.

(Itertools::cartesian_product has effectively the same requirements as our current algorithm: the left item and the right iterator must both implement Clone.)

kpreid commented 1 year ago

A sketch of a solution which avoids all cloning: implementors of Exhaust produce Factorys, which are effectively functions which can be invoked repeatedly to produce items of the type. There would be a generic Factory implementation usable for clonable types.

pub trait Exhaust {
    type Iter: Iterator<Item = Self::Fact>;
    type Fact: Factory<Output = Self>;

    fn exhaust_factory() -> Self::Iter;

    fn exhaust() -> MapToValue<Self::Iter> {
        self.exhaust_factory().map(|f| Self::Fact::new(Cow::Owned(f)))
    }
}

pub trait Factory {
    type Output;
    fn new(f: Cow<'_, Self>) -> Output;
}

type MapToValue<I> = core::iter::Map<I, fn(I::Item) -> I::Item::Output>;

(This would be cleaner if type_alias_impl_trait was stable since we could just use impl Fn() -> Self as the factory trait instead of a new trait.)

kpreid commented 1 month ago

I wonder if the advantage of the Factory idea could be provided without complicating the trait. In particular, instead of defining <Mutex<T> as Exhaust>::Factory = MutexFactory<T>, we could say that MutexFactory<T> itself implements Exhaust, and that data types which contain Mutexes should implement their Exhaust by exhausting MutexFactory instead. The problem with this idea is that there still needs to be a way for derive(Exhaust) to pick the factory type for each field automatically, so this probably turns out to be equivalent.

kpreid commented 1 month ago

I started writing out the Factory strategy, and it's really annoying that every impl must map() its natural iterator into a wrapper factory. I can't just impl<T: Clone> Factory for T because that would conflict with factories that don't want that impl.

However, it's easier if we stick to the reversed associated type relationship: Exhaust has a function that takes the Factory type and produces Self. That way, a single type can be used as factories by multiple impls, and users don't have to implement two traits, either.

kpreid commented 1 month ago

Factory doesn't need to be an associated type, even; it can be the <<Self as Exhaust>::Iter as Iterator>::Item. But the redundancy may be useful for having less horrible type errors. Not sure yet.

kpreid commented 1 month ago

Done in #33.