Add methods to access the wrapped iterator of Cloned

[tbu-]

I wanted to call .as_slice() on a slice::Iter, but the iterator was wrapped in a .cloned() call.

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[the8472]

Imo dconstructing adapters does not set a good precedent because ultimately it's not possible to do this for all adapters or would constrain future implementation changes. And since it wouldn't be available for many adapters it wouldn't carry well across refactorings for users.

So it'd be better to encourage the use of by_ref() instead which lets you recover the inner iterator once you drop the ref.

See rust-lang/libs-team#128 for a general discussion on the topic

[tbu-]

Imo dconstructing adapters does not set a good precedent because ultimately it's not possible to do this for all adapters or would constrain future implementation changes.

This seems more like a slippery slope argument. I think on Cloned, this does not constrain future implementation changes because there's just one canonical implementation for Cloned.

So it'd be better to encourage the use of by_ref() instead which lets you recover the inner iterator once you drop the ref.

The iterator is part of my data structure, so this would need self-referential types to work.

[the8472]

The iterator is part of my data structure

It doesn't have to be. Cloned has no state, so you can keep the slice iterator and build a by_ref().cloned() on the fly when you need an iterator that emits owned items.

[tbu-]

Indeed, it doesn't have to be. But this means that Cloned (or other iterator implementations) are essentially unusable for putting them into data structures if you also need to call methods on the underlying iterator. You can work around that by reimplementing what Cloned does.

But maybe the standard library also wants the iterator adapters to be embeddable into data structures (and not just reimplemented everywhere).

[the8472]

But maybe the standard library also wants the iterator adapters to be embeddable into data structures

And there's the slope 😉 . If we add it there'd be questions why it's on this adapter and not some other one. Making it a general feature is incompatible with some adapters. E.g. StepBy cannot expose its internals since they get mangled.

this does not constrain future implementation changes because there's just one canonical implementation for Cloned.

That's not correct. With associated type specialization structs can substitute their default fields with optimized representations for specific types, which would be incompatible with returning a reference to the inner iterator.

And as the StepBy specialization shows that with some limitations we don't even need to wait for the associated type kind to do some creative reinterpretation of inner structs. And since such transformations can be slightly lossy it would also make into_inner() impossible.

So it very much does bind the future.

---

To clarify, I do get that this is quite a convenient feature. The standard library itself even unpacks iterator abstractions in various places. It's just that the usefulness-to-users conflicts with useful-to-the-implementation properties that are enabled by having exclusive, private access to internals.

So if possible it's preferable that people use by_ref() instead to not tie our hands. If there's an important use-case that's impossible to do with by_ref() then we can weigh it against giving up implementation flexibility.

[tbu-]

And as the StepBy specialization shows that with some limitations we don't even need to wait for the associated type kind to do some creative reinterpretation of inner structs. And since such transformations can be slightly lossy it would also make into_inner() impossible.

So it very much does bind the future.

I see. So the question is really: Can we imagine an optimized implementation for Cloned that a) does not contain the original iterator (for the reference methods) or b) that cannot reconstruct the original iterator (for into_inner). I see why this is a hard question to ask, especially for all future optimizations.

So if possible it's preferable that people use by_ref() instead to not tie our hands. If there's an important use-case that's impossible to do with by_ref() then we can weigh it against giving up implementation flexibility.

That's not possible in my case. I can construct the iterator on the fly, but then I'd lose out on the optimization benefits. So the correct solution might just be to reimplement all the iterator abstractions on my own so that I can also access the inner iterator.

[the8472]

I see. So the question is really: Can we imagine an optimized implementation for Cloned that a) does not contain the original iterator (for the reference methods) or b) that cannot reconstruct the original iterator (for into_inner). I see why this is a hard question to ask, especially for all future optimizations.

That is quite hard indeed. Not only does it depend on future optimizations but also future iterator types that might require new optimizations.

A simple "existence proof" for a) involves lending iterators. When you have an unfortunate API that takes a lending iterator but the underlying source is owning then we may be able to excise the step that takes an owned item and turns it into a short-lived reference which the consumer then clones and just return the original item. By unpacking the iterator two levels deep this would no longer contain the field that temporarily holds the owned value that's being lent.

b) might involve somthing reference-counted where if Cloned knows it is the last, exclusive owner it can just steal the values instead of cloning them. Accessing original source could then lead to surprising results, depending on which APIs the source exposes. We have no such thing in std today afaik.

but then I'd lose out on the optimization benefits

Which ones are relevant for you? Cloned is somewhat less optimized than Copied anyway. I think it's based on the assumption that clone impls will do things that are more likely to inhibit optimizations. TrustedLen does propagated through by_ref().

[tbu-]

For future optimization potential, we don't want to guarantee that Cloned contains its inner iterator or can even reconstruct it. Since my use case has a work-around (manually implement Cloned in my code), the decision is that we don't want this PR.