Open ijackson opened 11 months ago
See #128 for a similar recent proposal.
Does by_ref().rev()
not work for your case?
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>
.)
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.
Since Rev is stateless maybe you produce the
rev()
on-demand (with a shorter lifetime) and carry around theChars
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) :-).
What about making .rev().rev()
return the original type:
impl<T> Rev<T> {
fn rev(self) -> T {
self.0
}
}
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.
Yeah it would be a breaking change
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
.
Proposal
Problem statement
Having wrapped an iterator in
std::iter::Rev
, withIterator::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 dos.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
withstd::str::Chars::as_str
. But that requires getting atChars
and we only haveRev<Chars>
.Solution sketch
Make
iter::Rev
's field public:(Currently the implementation is a named-fields struct; we would want to make
Rev
a tuple struct.)Alternatives
Provide a deconstructor
(Shown this way for clarity;
Rev
's single field is actually callediter
)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 thats.chars().rev().rev()
gives youChars
rather thanRev<Rev<Chars>>
but that is probably too confusing.Downside
This API may not be sufficient in every case.
&mut impl Iterator
implementsIterator
, so there might be situations where you'd wantRev::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. LikeReverse
andWrapping
, 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):
Second, if there's a concrete solution: