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

RangeFrom<u8> stops at 254 #24119

Closed
phaylon opened this issue Apr 6, 2015 · 9 comments · Fixed by #24120
Closed

RangeFrom<u8> stops at 254 #24119

phaylon opened this issue Apr 6, 2015 · 9 comments · Fixed by #24120

Comments

@phaylon
Copy link
Contributor

phaylon commented Apr 6, 2015

I tried printing all valid u8s by doing

for n in 0u8.. {
    println!("{}", n);
}

and was surprised to find it only printed up to 254.

I'm unsure if this is supposed to be this way (range_inclusive can be used). If so, I'd propose adding this to the Iterators/Ranges section of the book.

@14427
Copy link
Contributor

14427 commented Apr 6, 2015

The problem is the next method of RangeFrom:

fn next(&mut self) -> Option<A> {
    self.start.step(&A::one()).map(|mut n| {
        mem::swap(&mut n, &mut self.start);
        n
    })
}

It returns the previous value, then updates itself to store the next one. The value 255 ends up being the value that represents a "done" state, so it can't return it.

I can't think of any solution that doesn't involve adding a done flag to RangeFrom.

@Stebalien
Copy link
Contributor

Or impl IntoIter instead of Iterator (and use a helper struct to keep track of the range).

@rprichard
Copy link
Contributor

Seems similar to #23635.

@AaronFriel
Copy link
Contributor

Question: What is the origin of this Rust behavior, or what is its justification? e.g.: Why an exclusive upper bound on ranges?

This caught me by total surprise, coming from Haskell, and I don't know if any Rust I've written for my own cases is wrong now.

It behaves differently from similar syntax in Haskell, Swift, and Go, so I'm wondering if it's fruitful to have behavior different from peer languages (in trying to attract people away from languages where syntax like this does not exist.)

@steveklabnik
Copy link
Member

Ruby has both exclusive and inclusive ranges

steve@warmasteve@warmachine:~/src/rust$ irb
irb(main):001:0> (1..3).to_a
=> [1, 2, 3]
irb(main):002:0> (1...3).to_a
=> [1, 2]
chine:~/src/rust$ irb
irb(main):001:0> (1..3).to_a
=> [1, 2, 3]
irb(main):002:0> (1...3).to_a
=> [1, 2]

@solson
Copy link
Member

solson commented Apr 6, 2015

The origin is related to the for-loop idiom in C. for (int i = a; i < b; ++i) corresponds to the a..b range. It has some nice properties such as how it iterates over exactly b - a elements, whereas an inclusive range does b - a + 1 and would require you to say, for example, 0..v.len()-1 for the indices of a vector if .. were inclusive.

Essentially, it seems like the exclusive behaviour is useful more often than inclusive in a systems language.

@Gankra
Copy link
Contributor

Gankra commented Apr 6, 2015

Exclusive behaviour is also a leaner implementation. For inclusive behaviour you need some kind of flag for "now yield the inclusive element". This issue is an exact example of why: How do you statefully write an iterator from 0 to u8::MAX, inclusive?

@aturon
Copy link
Member

aturon commented Apr 6, 2015

cc #24120, which adjusts this behavior by triggering an overflow on non-wrapped-arithmetic types; it makes the semantics equivalent to repeatedly updating n = n + 1.

bors added a commit that referenced this issue Apr 8, 2015
A recent change to the implementation of range iterators meant that,
even when stepping by 1, the iterators *always* involved checked
arithmetic.

This commit reverts to the earlier behavior (while retaining the
refactoring into traits).

Fixes #24095
Closes #24119
cc #24014 

r? @alexcrichton
@azar77
Copy link

azar77 commented Apr 9, 2015

cc #24120, which adjusts this behavior by triggering an overflow on non-wrapped-arithmetic types; it makes the semantics equivalent to repeatedly updating n = n + 1.

so @aturon, just to be clear, with the above PR merged, (0u8..).last() now panics. Is this the intended behavior?

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

Successfully merging a pull request may close this issue.

10 participants