Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std::iter::Chain should implement ExactSizeIterator when possible #34433

Closed
tomaka opened this issue Jun 23, 2016 · 17 comments
Closed

std::iter::Chain should implement ExactSizeIterator when possible #34433

tomaka opened this issue Jun 23, 2016 · 17 comments

Comments

@tomaka
Copy link
Contributor

tomaka commented Jun 23, 2016

When both iterators of the chain implement ExactSizeIterator, the chain itself should implement that trait as well.

@Eh2406
Copy link
Contributor

Eh2406 commented Jun 23, 2016

Is it just

impl<A, B> ExactSizeIterator for Chain<A, B> where
    A: ExactSizeIterator,
    B: ExactSizeIterator<Item=A::Item>, {}

does this nead tests? if so where and how?

@ollie27
Copy link
Member

ollie27 commented Jun 24, 2016

Unfortunately I don't think this is possible because sum of the the len() of the two iterators could overflow usize.

@Thiez
Copy link
Contributor

Thiez commented Jun 24, 2016

On that note, surely Range<u32> shouldn't implement ExactSizeIterator because Rust hopes to support 16-bit systems? The source code contains this snippet:

// Ranges of u64 and i64 are excluded because they cannot guarantee having
// a length <= usize::MAX, which is required by ExactSizeIterator.
range_exact_iter_impl!(usize u8 u16 u32 isize i8 i16 i32);

Seems vaguely related. Would it make sense for ExactSizeIterator to be generic in the return type of its len operation? The default would remain ExactSizeIterator<Len=usize>.

@tbu-
Copy link
Contributor

tbu- commented Jun 25, 2016

As @ollie27 noted, this is not possible:

use std::usize;

fn main() {
    let iter = (0..usize::max_value()).chain(0..usize::max_value());
    println!("{:?}", iter.size_hint());
}

@bluss
Copy link
Member

bluss commented Sep 3, 2016

It would require changing the contract for ExactSizeIterator, for example:

  • The iterator must report the smaller of: (1) usize::MAX and (2) the exact number of elements that remain in the iterator before it returns None from .next() (or .next_back() where applicable).

I don't think such a change affects how this trait can be used in practice today? It would extend its usability.

Implementation would be: saturating_add. Note that a longer than maximum chain will of course report its true exact length as soon as enough has been consumed to bring the length down into the usize value range.


5th november: One problem, .chain().rposition() and .chain().enumerate().rev() can have totally wrong results.

@frewsxcv
Copy link
Member

It doesn't seem like this can happen. Is there any hope here or can this be closed?

@bluss
Copy link
Member

bluss commented Jan 13, 2017

I think close.

My rationale would be that ExactSizeIterator was introduced to be able to move rposition from the slice to general iterators, producing element indices counted from the front of the sequence. Thus it's inherently designed to be tied to in-memory sequences.

@tbu-
Copy link
Contributor

tbu- commented Jan 14, 2017

This doesn't even work if both iterators reside in memory, e.g. on a 32bit platform:

use std::iter;

let x: Vec<u8> = iter::repeat(0).take((1 << 31) + 1).collect();
x.iter().chain(x.iter())

This produces a chain that is longer than 2**32, too large to be represented in a usize.

@bluss
Copy link
Member

bluss commented Jan 14, 2017

What I mean is that ExactSizeIterator was designed for slice.iter().rposition().

@frewsxcv
Copy link
Member

Well, it sounds like people are in agreement here that this can't happen, unfortunately

hdevalence added a commit to hdevalence/curve25519-dalek that referenced this issue Aug 1, 2017
This reverts commit 720da34.

Unfortunately, iter::chain on two ExactSizeIterators does not produce an ExactSizeIterator, for reasons described here: rust-lang/rust#34433 .
@RalfJung
Copy link
Member

On the other hand there are cases where your iterators all fit into usize comfortably and you are also okay with a panic on overflow... for such cases, not having ExactSizeIterator is really painful.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 22, 2019

This is the third time in 3 years that I have ended up in this issue. Each use case was definitely fine with panicking on overflow.

bors bot added a commit to gfx-rs/wgpu that referenced this issue Aug 17, 2020
880: Port to gfx-hal-0.6 r=kvark a=kvark

This got a little more involved than I hoped, because of rust-lang/rust#34433 which is unfortunately closed.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
@kaleidawave
Copy link

Could this instead be a method on Chain? How about Chain::try_len() -> Result<usize, ...>

@cuviper
Copy link
Member

cuviper commented Jun 19, 2022

Iterator::size_hint is already basically a try_len, if you compare the lower bound to Some upper bound. That's what the default ExactSizeIterator::len does, asserting that they are equal.

@clarfonthey
Copy link
Contributor

clarfonthey commented Jul 16, 2022

So, this already seems to have been rejected as a PR-- do people feel like this would have any chance of being approved if it had an RFC first? Or maybe people's opinions have changed?

Basically wondering what the path forward for this would be. In my latest case for this, I did Chain with a single option iterator and another iterator, which effectively just adds + 1 or + 0 to the length -- it's very frustrating that this completely leaves out an ExactSizeIterator implementation.

@Aceeri
Copy link

Aceeri commented Aug 31, 2022

I'm not sure that it would even panic if we left it up to the size_hint of the existing iterator, since it already does a saturating_add, I suppose sure it wouldn't be accurate at that point...

@the8472
Copy link
Member

the8472 commented Apr 18, 2024

Once we have generic const exprs this could be revisited by having a trait for non-ZST types and then using knowledge that some iterators (slices, vec) and things derived from those can't exceed isize::MAX items for non-ZSTs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests