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

Step::steps_between does not distinguish overflow and unimplemented (unstable) #48117

Open
scottmcm opened this issue Feb 10, 2018 · 2 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Feb 10, 2018

It returns Option<size>: https://doc.rust-lang.org/std/iter/trait.Step.html#tymethod.steps_between

And the comment says it uses None for overflow: https://doc.rust-lang.org/src/core/iter/range.rs.html#29

But it also uses None for unimplemented https://doc.rust-lang.org/src/core/iter/range.rs.html#155

And as a result, Range needs to return (0, None) when it gets None: https://doc.rust-lang.org/src/core/iter/range.rs.html#235-240

It would be nice to either

  • Require that the method is implemented accurately (easy for fundamental integers; harder if Step is expected to support graph walks, but maybe not harder than PartialOrd)
  • Change the type to something else, maybe Option<Option<usize>> or be more hint-like with (usize, Option<usize>)
  • Something else

cc #42168

@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 10, 2018
@CAD97
Copy link
Contributor

CAD97 commented May 15, 2020

The links appear to have lost their original position.

Does the original concern still stand? (Does it stand with #69659?)

@scottmcm
Copy link
Member Author

scottmcm commented Aug 4, 2020

@CAD97 Yes, I think there's still essentially the same problem -- the Invariants section still has None for both unreachable (steps_between(1, 0)) and overflow (steps_between(0_u128, 1<< 127)).

So if this is used for size_hint, then getting None back from steps_between means you're stuck returning (0, None), when you'd rather be able to return either (0, Some(0)) or (usize::MAX, None).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants