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

[ER] NonZeroX Step and better constructors #73121

Open
leonardo-m opened this issue Jun 8, 2020 · 5 comments
Open

[ER] NonZeroX Step and better constructors #73121

leonardo-m opened this issue Jun 8, 2020 · 5 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@leonardo-m
Copy link

It could be nice to have std::iter::Step for the NonZeroX numbers, so in this code instead of having about 1000 unwrap:

fn main() {
    use std::num::NonZeroUsize;
    let nz = |x| NonZeroUsize::new(x).unwrap();
    for i in 1 .. 1_000 {
        println!("{:?}", nz(i));
    }
}

You only need two:

fn main() {
    use std::num::NonZeroUsize;
    let nz = |x| NonZeroUsize::new(x).unwrap();
    for i in nz(1) .. nz(1_000) {
        println!("{:?}", i);
    }
}

If also a const-generics-based constructor is added to stdlib then the number of run-time unwraps goes to zero (the two panics become compile-time):

fn main() {
    use std::num::nonzero_usize;
    for i in nonzero_usize::<1>() .. nonzero_usize::<1_000>() {
        println!("{:?}", i);
    }
}

A further improvement should come from const generics of arbitrary type (currently not allowed):

fn main() {
    use std::num::nonzero_usize;
    for i in nonzero::<1_usize>() .. nonzero::<1_000>() {
        println!("{:?}", i);
    }
}
@jonas-schievink
Copy link
Contributor

There's not really a point in calling .unwrap() less, the optimizer will get rid of all the calls anyways

@leonardo-m
Copy link
Author

The less unwraps you have, the less you have to think (or in more reliable coding to prove) about possible ways for them to fire at run-time and ruin your execution.

@crlf0710 crlf0710 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 Jun 10, 2020
@leonardo-m
Copy link
Author

leonardo-m commented Jan 1, 2021

I'd like also the .step_by(usize).

the optimizer will get rid of all the calls anyways

I think this isn't always true (I've seen such cases in my code, where the inliner wasn't able to get rid of all the unwrap calls), if you have iterators chains, like (where pred computes a division by the input argument):

(1 .. 1000)
.map(...)
.filter(pred)
...

The compiler is not always able (or willing) to inline everything, so predicates and other lambdas get called by values that are nonzero, but the functions lose the information about the input argument being nonzero.

But if you have a range of NonZero values the type system can't lose this information and new improvements like #79134 give you some extra performance in non-inlined functions:

const NZ1: NonZeroU32 = NonZeroU32::new(1).unwrap();
const NZ1000: NonZeroU32 = NonZeroU32::new(1000).unwrap();

(NZ1 .. NZ1000)
.map(...)
.filter(pred)
...

@jonas-schievink jonas-schievink added C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jan 1, 2021
@jalil-salame
Copy link
Contributor

This is also being discussed in the libs-team: rust-lang/libs-team#130.

@SOF3
Copy link
Contributor

SOF3 commented Sep 6, 2023

@leonardo-m it's reasonable that the compiler cannot optimize after you have mapped the integer, right?

@Dylan-DPC Dylan-DPC added the S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. 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

6 participants