Avoid unused clones in Cloned<I> and Copied<I>

[thaliaarchi]

Avoid cloning in Cloned<I> or copying in Copied<I> when elements are only needed by reference or not at all. There is already some precedent for this, given that __iterator_get_unchecked is implemented, which can skip elements. The reduced clones are technically observable by a user impl of Clone.

r? libs-api

[thaliaarchi]

I was trying to implement Iterator::try_find for them, but I had difficulty figuring out how to map the residual. Perhaps this is missing from the try_trait_v2 API? Or, more likely, I'm missing something. I'd appreciate help on this.

[joboet]

I was trying to implement Iterator::try_find for them, but I had difficulty figuring out how to map the residual. Perhaps this is missing from the try_trait_v2 API? Or, more likely, I'm missing something. I'd appreciate help on this.

The easiest way is to use try-blocks:

try { it.try_find(..)?.cloned() }

[thaliaarchi]

Thanks. That's indeed simpler. Unfortunately, it's not the full missing piece to the puzzle.

With that, it's this:

impl<'a, I, T: 'a> Iterator for Cloned<I>
where
    I: Iterator<Item = &'a T>,
    T: Clone,
{
    fn try_find<R>(
        &mut self,
        f: impl FnMut(&Self::Item) -> R,
    ) -> ChangeOutputType<R, Option<Self::Item>>
    where
        R: Try<Output = bool, Residual: Residual<Option<Self::Item>>>,
    {
        try { self.it.try_find(move |x| f(&x))?.cloned() }
    }
}

It yields this error:

error[E0277]: the trait bound `<R as Try>::Residual: Residual<Option<&'a T>>` is not satisfied
    --> library/core/src/iter/adapters/cloned.rs:97:23
     |
97   |         try { self.it.try_find(move |x| f(&x))?.cloned() }
     |                       ^^^^^^^^ the trait `Residual<Option<&'a T>>` is not implemented for `<R as Try>::Residual`
     |
note: required by a bound in `iterator::Iterator::try_find`
    --> library/core/src/iter/traits/iterator.rs:2930:41
     |
2924 |     fn try_find<R>(
     |        -------- required by a bound in this associated function
...
2930 |         R: Try<Output = bool, Residual: Residual<Option<Self::Item>>>,
     |                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Iterator::try_find`

This is the same error I was encountering with my draft version. I think it is equivalent or close to using try blocks:

match self.it.try_find(move |x| f(&x)).branch() {
    ControlFlow::Continue(x) => Try::from_output(x),
    ControlFlow::Break(r) => FromResidual::from_residual(r),
}

Upon further reflection, I think try_find is incompatible with this PR. As I understand the Residual<T> bound, the T is the type of the output and the type of the residual is unconstrained. Since it has access to the T of the output, the residual could be derived from it. As the iterator items differ, residuals which use that type will differ and cannot be converted between each other.

[joboet]

Ah, yes, that's true...

I don't think this needs libs-api approval, I'd consider these changes as implementation details. So:
@bors r+ rollup
r? joboet

[bors]

📌 Commit ed5f31a has been approved by joboet

It is now in the queue for this repository.