Skip to content

Implement Step for NonZeroUxx #130

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

Closed
SOF3 opened this issue Oct 30, 2022 · 8 comments
Closed

Implement Step for NonZeroUxx #130

SOF3 opened this issue Oct 30, 2022 · 8 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@SOF3
Copy link

SOF3 commented Oct 30, 2022

Proposal

Problem statement

Ranges of non-zero unsigned integers does not contain zero, and should be step-able.

Motivation, use-cases

I want to iterate over a Range<NonZeroUsize>.

Solution sketches

It might be tricky to implement backward_checked properly since we cannot guarantee that the input usize is less than start.

It is also inconsistent that NonZeroIxx are not implemented.

Links and related work

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 in its weekly meeting. You should receive feedback within a week or two.

@SOF3 SOF3 added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Oct 30, 2022
@SUPERCILEX
Copy link

If all you want to do is iterate, why can't you use the underlying type? Though in general I don't see a reason to not implement step, so this seems like a good idea.

In terms of the ACP, it should be mentioned that ranges need to implement step for them to be iterable, hence why this ACP is necessary.

@SOF3
Copy link
Author

SOF3 commented Apr 18, 2023

If all you want to do is iterate, why can't you use the underlying type?

That's my alternative solution now.

https://github.com/SOF3/dynec/blob/bade2b3083f9348b740dd83e34af4478dadee353/src/entity/raw.rs#L68-L73

The range is used as a parameter to a BTreeSet<NonZeroU32>, which has a range associated function that accepts a Range. As for why it doesn't just contain BTreeSet<u32>... I just want the types to be as explicit as possible.

@jalil-salame
Copy link

I posted about this in the Internals Forum.

There I state another (although a bit esoteric) use case: when solving Sudokus letting 0 mean an empty cell is quite natural, you can make it explicit using Option<NonZeroU*>, but when you want to iterate over the values, using the underlying type is painful, as stated before.

@SOF3
Copy link
Author

SOF3 commented Apr 25, 2023

In my use case, I was developing an ECS library where entity IDs are always nonzero (to optimize Option<Entity> memory usage), but I frequently have the need to iterate over a range of allocated entities (for SIMD purposes).

@marshrayms
Copy link

I was bitten by this today.
I need the NonZeroUxx value inside the iteration.

@khuey
Copy link

khuey commented Dec 24, 2023

Is there a reason not to move forward on this or does it just need someone to actually do the work?

@the8472
Copy link
Member

the8472 commented Dec 24, 2023

libs-api discusses change proposals during meetings, there's a backlog and people open new ones almost as fast as they get processed so it just takes a while.

In principle the ACP process is optional and one can make a PR instead, but then one may be faced with the same questions about motivation, alternatives, etc. especially when the reviewer is unsure if the addition makes sense.

@m-ou-se
Copy link
Member

m-ou-se commented Jul 9, 2024

We discussed this in the libs-api meeting. We're happy to see these impls added.

Feel free to open a PR to rust-lang/rust to add them, at which point we'll propose an FCP for official team confirmation before stabilziation.

@m-ou-se m-ou-se closed this as completed Jul 9, 2024
@m-ou-se m-ou-se added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Jul 9, 2024
jalil-salame added a commit to jalil-salame/rust that referenced this issue Jul 9, 2024
Implement Step for NonZero unsigned integers as discussed in
[libs-team#130][1].

[1]: rust-lang/libs-team#130

`step_nonzero_impls` was adapted from `step_integer_impls` and the tests
were adapted from the step tests.
jalil-salame added a commit to jalil-salame/rust that referenced this issue Jul 10, 2024
Implement Step for NonZero unsigned integers as discussed in
[libs-team#130][1].

[1]: rust-lang/libs-team#130

`step_nonzero_impls` was adapted from `step_integer_impls` and the tests
were adapted from the step tests.
jalil-salame added a commit to jalil-salame/rust that referenced this issue Jul 10, 2024
Implement Step for NonZero unsigned integers as discussed in
[libs-team#130][1].

[1]: rust-lang/libs-team#130

`step_nonzero_impls` was adapted from `step_integer_impls` and the tests
were adapted from the step tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

7 participants