Skip to content

Implement useful steps_between for all integers #59444

New issue

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

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

Already on GitHub? # to your account

Merged
merged 3 commits into from
Apr 2, 2019

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Mar 26, 2019

We can use usize::try_from to convert steps from any size of integer.
This enables a meaningful size_hint() for larger ranges, rather than
always just (0, None). Now they return the true (len, Some(len))
when it fits, otherwise (usize::MAX, None) for overflow.

@rust-highfive
Copy link
Contributor

r? @kennytm

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 26, 2019
We can use `usize::try_from` to convert steps from any size of integer.
This enables a meaningful `size_hint()` for larger ranges, rather than
always just `(0, None)`. Now they return the true `(len, Some(len))`
when it fits, otherwise `(usize::MAX, None)` for overflow.
@Centril
Copy link
Contributor

Centril commented Mar 26, 2019

r? @scottmcm

@rust-highfive rust-highfive assigned scottmcm and unassigned kennytm Mar 26, 2019
@cuviper
Copy link
Member Author

cuviper commented Mar 26, 2019

rather than always just (0, None).

Note, this former behavior meant that a lot of the TrustedLen impls failed the contract -- e.g. Range<u64> was wrong for platforms with less than 64-bit usize. But I think it would now be correct to have TrustedLen for all integers -- should I expand that? (edit: done)

@Centril
Copy link
Contributor

Centril commented Mar 30, 2019

Ping from triage, @scottmcm :)

@jonas-schievink jonas-schievink added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 1, 2019
@scottmcm
Copy link
Member

scottmcm commented Apr 2, 2019

It looks like #43127 was worried about TryFrom previously, but #43248 has probably resolved that, so I like getting these fixed up and removing the platform-dependence.

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 2, 2019

📌 Commit a548d83 has been approved by scottmcm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 2, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 2, 2019
Implement useful steps_between for all integers

We can use `usize::try_from` to convert steps from any size of integer.
This enables a meaningful `size_hint()` for larger ranges, rather than
always just `(0, None)`. Now they return the true `(len, Some(len))`
when it fits, otherwise `(usize::MAX, None)` for overflow.
bors added a commit that referenced this pull request Apr 2, 2019
Rollup of 8 pull requests

Successful merges:

 - #59262 (Remove duplicated code from Iterator::{ne, lt, le, gt, ge})
 - #59286 (Refactor async fn return type lowering)
 - #59444 (Implement useful steps_between for all integers)
 - #59452 (Speed up rustdoc run a bit)
 - #59533 (Support allocating iterators with arenas)
 - #59585 (Fixes for shallow borrows)
 - #59607 (Renames `EvalErrorKind` to `InterpError`)
 - #59613 (SGX target: convert a bunch of panics to aborts)

Failed merges:

 - #59630 (Shrink `mir::Statement`.)

r? @ghost
@bors bors merged commit a548d83 into rust-lang:master Apr 2, 2019
@cuviper
Copy link
Member Author

cuviper commented Apr 2, 2019

@scottmcm great, thanks for the added background!

@cuviper cuviper deleted the steps_between branch April 2, 2019 17:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants