Add NonZero{I,U}{8,16,32,64,128,size}::{MIN,MAX}

Currently, there is no way to obtain a NonZero* value without introducing unsafe codes or unwrap() calls, providing these constants can make it easier to use these types. For example:

use std::num::NonZeroU64;

fn foo(x: u64) -> NonZeroU64 {
    x.checked_add(1).and_then(NonZeroU64::new).unwrap_or(NonZeroU64::MAX)
}

I think this sounds like a good feature, and I would like to claim it for a first contribution.

@rustbot

@rustbot claim

Not that I mind the addition of these constants in general, but your motivating example is a bit unfortunate in that it's a slightly obfuscated saturating_add. It would be better served by:

#![feature(nonzero_ops)]
use std::num::NonZeroU64;

pub fn baz(x: u64) -> NonZeroU64 {
    NonZeroU64::new(1).unwrap().saturating_add(x)

    // or `NonZeroU64::new(x.saturating_add(1)).unwrap()` if you need a solution on stable
}

Yes, that does contain an unwrap, but it's provably unreachable and really the right tool for the job here. (Slightly better codegen too https://rust.godbolt.org/z/Tansn3Y61)

There are projects that forbid the usage of unwrap with Clippy. Using unwrap will require explicit #[allow(...)] attribute and maybe proper comment, which seems a little too much.

This might just be my head being in a more Ada space, but not having immediate access to the MIN and MAX for a given scalar type feels wrong to me. Obviously with a little forethought you can come up with a totally safe unwrap, but when you're doing something simple like initializing values for a comparison algorithm, it seems like it might be a bit better if you can use a constant which took care of that thinking about safety for you.

@mjclements

@mjclements: Thank you. This:

but when you're doing something simple like initializing values for a comparison algorithm

is a good motivating example if you ask me.

@EFanZh: Sounds like these projects blanket forbid unwrap as a very coarse heuristic for the behavior they actually want to disallow. If this unwanted behavior is "there is an input that causes the program to panic (ignoring OOM)", you could teach Clippy to allow unwrap in const contexts. That way you get a compilation error if the panic path ever fires. The example you provided would then look like:

#![feature(nonzero_ops)]
#![feature(const_option)]
use std::num::NonZeroU64;

pub fn bax(x: u64) -> NonZeroU64 {
    const ONE: NonZeroU64 = NonZeroU64::new(1).unwrap();
    ONE.saturating_add(x)
}

Of course that currently requires nightly features and not sure how hard it would be to improve Clippy. Hopefully that will become viable for stable projects soon.

Some interrelated thoughts on the stated problems this is trying to solve:

NonZeroI*::{MIN,MAX} are a little weird, because in all other integer cases, all integers between MIN and MAX exist, but in the case of nonzero signed integers, there's also a hole in the middle. This seems an oddity worth thinking about and possibly documenting.
I also notice that the code example at the beginning of this thread cannot be written using these constants for NonZeroI* because while NonZeroU*::MIN is 1, NonZeroI*::MIN is not. Also the code would be clearer in both cases if the value was named ONE rather than MIN. This suggests to me that possibly there should be a ONE constant for NonZero types, or at least the signed ones.
- If there were to be ONE, then the question of why not TWO, TEN, etc. comes up. The plausible response to adding any of them is "it's all moot once const panics are stable", but NonZeroI64::new(5).unwrap() is still a long and tedious bit of code to write whenever you want a number literal. It would be nice if there was a specific or general way to improve such fallible-but-only-at-compile-time code. (I have some thoughts on this but they're further off-topic for this issue.)

(None of this is intended to argue that MIN and MAX shouldn't be added.)

@kpreid

I like the points @kpreid made. I would concur that ONE as a constant might be useful, but should not be instead of a MIN, even for the signed integers. I think having the lowest possible value clearly demarcated as such has value.

(It would be kinda nice to have a fast way of making number literals of this type. I think that would probably help with adoption. Feel free to tag me if you open an issue!)

NonZeroI*::{MIN,MAX} are a little weird, because in all other integer cases, all integers between MIN and MAX exist, but in the case of nonzero signed integers, there's also a hole in the middle. This seems an oddity worth thinking about and possibly documenting.

Agreed.

I also notice that the code example at the beginning of this thread cannot be written using these constants for NonZeroI* because while NonZeroU*::MIN is 1, NonZeroI*::MIN is not. Also the code would be clearer in both cases if the value was named ONE rather than MIN. This suggests to me that possibly there should be a ONE constant for NonZero types, or at least the signed ones.

Not sure I follow. The code example at the beginning used MAX and not MIN. Yes, it does happen to add 1 - which incidentally also happens to be the value of NonZeroU*::MIN - so you technically could implement the given example as:

use std::num::NonZeroU64;

pub fn bax(x: u64) -> NonZeroU64 {
    NonZeroU64::MIN.saturating_add(x)
}

Luckily, I haven't seen anybody propose such a gross abuse of the MIN constant.

If there were to be ONE, then the question of why not TWO, TEN, etc. comes up. The plausible response to adding any of them is "it's all moot once const panics are stable", but NonZeroI64::new(5).unwrap() is still a long and tedious bit of code to write whenever you want a number literal. It would be nice if there was a specific or general way to improve such fallible-but-only-at-compile-time code. (I have some thoughts on this but they're further off-topic for this issue.)

Yes, getting a literal NonZero* is currently overly clunky and should be made easier and quicker. No doubt about that, but adding otherwise meaningless constants (be it ONE or TWO or TEN) isn't a good solution. Not sure how solutions could look that that are both ergonomic and feasible, but that's indeed going too far off-topic. If you end up writing down your thoughts on that, kindly cc me.

Rollup merge of rust-lang#93293 - nvzqz:nonzero-min-max, r=joshtriplett

This is now available on nightly (docs).

Excellent, glad these have landed. Am discussing having these MIN/MAX available from traits:
https://internals.rust-lang.org/t/please-can-we-add-a-basic-num-trait/16449/9

Is there anything blocking the stabilization of this feature?

@dtolnay

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

🔔 This is now entering its final comment period, as per the review above. 🔔

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

Rollup merge of rust-lang#106633 - c410-f3r:stabilize-nonzero_bits, r…

Rollup merge of #106633 - c410-f3r:stabilize-nonzero_bits, r=dtolnay

Navigation Menu

Repository navigation

Add `NonZero{I,U}{8,16,32,64,128,size}::{MIN,MAX}` #89065

4 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Add NonZero{I,U}{8,16,32,64,128,size}::{MIN,MAX} #89065

Description

Activity

mjclements commented on Sep 18, 2021

mjclements commented on Sep 18, 2021

LingMan commented on Sep 19, 2021

EFanZh commented on Sep 19, 2021

mjclements commented on Sep 19, 2021

LingMan commented on Sep 20, 2021

kpreid commented on Sep 20, 2021

mjclements commented on Sep 21, 2021

LingMan commented on Sep 21, 2021

nvzqz commented on Mar 12, 2022

gilescope commented on Apr 11, 2022

c410-f3r commented on Jul 22, 2022

4 remaining items

rfcbot commented on Jan 23, 2023

rfcbot commented on Feb 28, 2023

rfcbot commented on Mar 10, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Add `NonZero{I,U}{8,16,32,64,128,size}::{MIN,MAX}` #89065