Provide way to deconstruct std::iter::Rev

[ijackson]

Proposal

Problem statement

Having wrapped an iterator in std::iter::Rev, with Iterator::rev, it is not possible to get it back out again. This prevents the use of methods that may be available on the underlying iterator.

Motivating examples or use cases

For example, given s: &str, one can do s.chars().rev() to walk backwards, character by character, from the end of the string. But having munched a number of characters, it would be nice to be able to turn the iterator back to &str with std::str::Chars::as_str. But that requires getting at Chars and we only have Rev<Chars>.

Solution sketch

Make iter::Rev's field public:

#[repr(transparent)]
pub struct Rev<T>(pub T);

(Currently the implementation is a named-fields struct; we would want to make Rev a tuple struct.)

Alternatives

Provide a deconstructor

impl Rev<T> {
    fn into_inner(self) -> T { self.0 }

(Shown this way for clarity; Rev's single field is actually called iter)

Naming

Is into_inner the right name? unreverse or something along those lines seems like another possibility.

An option would be to call the method .rev() so that s.chars().rev().rev() gives you Chars rather than Rev<Rev<Chars>> but that is probably too confusing.

Downside

This API may not be sufficient in every case. &mut impl Iterator implements Iterator, so there might be situations where you'd want Rev::as_inner_mut().

Do both

Possibly, we should do both of these things. They don't conflict. Depending on our naming, the deconstructor method will provide more clarity, whereas the public field solves all the use cases.

Links and related work

Precedent for making iter::Rev transparent: std::cmp::Reverse, std::num::Wrapping.

Like cmp::Reverse, iter::Rev is an API adapter newtype that simply proxies traits to an underlying implementation, with a particular semantic change. Like Reverse and Wrapping, it will never contain anything other than the underlying iterator.

Precedent for into_inner: like methods on many many stdlib types.

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

[the8472]

See #128 for a similar recent proposal.

Does by_ref().rev() not work for your case?

[ijackson]

See #128 for a similar recent proposal.

Interesting, thanks. TBH I agree with the decision in #128 not to add these deconstructors everywhere, as proposed in rust-lang/rust#103294. ISTM that for most of those adapters, that opens quite a can of worms, and exposes too much about the implementation.

I think the case for iter::Rev is a lot clearer, since - unlike most of the others - it's precisely an API veneer without any actual functionality.

As by_ref().rev() not work for your case?

Sadly not. (I did try, but in my case I wanted to clone the reversed iterator at one point, which you can do with Rev<Chars> but obviously not Rev<&mut Chars>.)

[the8472]

but in my case I wanted to clone the reversed iterator at one point, which you can do with Rev but obviously not Rev<&mut Chars>

Since Rev is stateless maybe you produce the rev() on-demand (with a shorter lifetime) and carry around the Chars instead and clone that.

[ijackson]

Since Rev is stateless maybe you produce the rev() on-demand (with a shorter lifetime) and carry around the Chars instead and clone that.

Yes. Thanks for the tips. With this issue I'm trying to improve std though, rather than fix my program (which I've found another way of writing, anyway) :-).

[pitaj]

What about making .rev().rev() return the original type:

impl<T> Rev<T> {
    fn rev(self) -> T {
        self.0
    }
}

[ijackson]

What about making .rev().rev() return the original type:

I discussed that option under "Naming" in "Alternatives". I don't think it's great, because it involves an inherent method shadowing a trait method, which can be confusing.

[pitaj]

Yeah it would be a breaking change

[ijackson]

With this issue I'm trying to improve std though, rather than fix my program (which I've found another way of writing, anyway) :-).

Well, in the version of my code that I am going to run with, I now have this:

/// Like `std::iter::Rev` but deconstructible
///
/// Upstream ACP <https://github.com/rust-lang/libs-team/issues/253>
#[derive(Clone, Debug)]
pub(crate) struct ReverseIter<I>(pub I);

impl<I: DoubleEndedIterator> Iterator for ReverseIter<I> {
    type Item = I::Item;
    fn next(&mut self) -> Option<I::Item> {
        self.0.next_back()
    }
}

Happily I don't need more of the API surface but this fortifies my view that this ough to be supported by std::iter::Rev.

[the8472]

We discussed this during today's libs-API meeting. We're not in favor of making the field pub or providing an as_inner_mut method since being able to peek at the current state would preclude future changes like specialization performing transformations on the inner iterator.
Since rev itself is reversible we do expect any such optimizations to be reversible too, so this can be undone in into_inner, which makes it acceptable for this particular adapter (but not necessarily for others).

Proposal for Rev::into_inner acecpted.

Please open a tracking issue and open a PR to rust-lang/rust to add it as an unstable feature. You can close this ACP once the tracking issue has been created.