Add a safe way to use NonZero

[Diggsey]

It should be possible to go from a T: Zeroable to an Option<NonZero<T>> without requiring unsafe code, so this PR adds a new method, NonZero::new_option(...) for that purpose.

This should be a no-op, and ideally it would use mem::transmute, but that isn't possible in generic code, so I had to fall back to this slightly less safe alternative.

[rust-highfive]

r? @nikomatsakis

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

[Diggsey]

Some(Nonzero::new(inner))

Wouldn't that risk undefined behaviour when inner = 0? Normally a Some(_) cannot equal None. I'll happily change it if that's not the case though.

I consider it fairly surprising that this fn sometimes yields a None variant.

The purpose was to provide a safe, checked version of NonZero::new(), which would return None if the value passed in is not considered "non-zero" rather than causing undefined behaviour. I strongly believe such functionality should exist in some form.

it needs more comments.

That's actually a mistake, I must have pressed undo too many times after finding out that transmute wouldn't work, and it deleted my comments 😠 I've added them back now.

[Gankra]

@Diggsey ... Are you arguing with yourself?

[Diggsey]

@gankro No, I was replying to @nikomatsakis's comments, but they disappeared when I pushed the new changes. Should I not have squashed them yet?

[Gankra]

@Diggsey yeah we prefer not to squash, but it's weird niko's comments totally disappeared. shrug

[nikomatsakis]

@Diggsey ok, with the comments, I certainly find the fn less mysterious, and I can see why you preferred the transmute over my Some proposal (since the fn doesn't always yield Some).

[nikomatsakis]

So, I think this seems like a fine addition, given that this is an unstable API etc. But I don't really know the policy here (i.e., what amount of approval is needed to expand an unstable API). Perhaps someone on the @rust-lang/libs team, e.g. @aturon @alexcrichton, would clarify?

[aturon]

But I don't really know the policy here (i.e., what amount of approval is needed to expand an unstable API).

We don't have a super clear-cut policy in a case like this. Personally, I'd like to see a kind of long-term vision for NonZero-like functionality that can eventually be stabilized -- I know @gankro has some thoughts about that that don't involve stabilizing NonZero itself -- and I'd rather not slowly accumulate unstable functionality without such clarity. For example, the Zeroable trait itself has never gone through an RFC process or been fully vetted by the libs team.

@gankro, IIRC your ideas about NonZero related to Unique and Shared. Is that right, and if so, how soon do you anticipate producing RFCs for those?

[Gankra]

Yeah I haven't seen a really good usecase for NonZero that wasn't just Unique or Shared. More generally I believe we should want some unsafe ForbiddenValues interface to hook into the sort of stuff I describe in rust-lang/rfcs#1230

But I'm a bit biased here -- I get really sad when I see something like Option<NonZero<*mut T>>.

[Diggsey]

I was using NonZero<u64> to represent IDs for use over the network. It was useful to reserve ID 0, and have an Option<Id> take up no extra space over just an Id, as well as being compatible with other languages which don't support algebraic data types.

[alexcrichton]

I agree with @aturon that I'd rather not accumulate little bits of unstable functionality for such nebulous types like this. Using NonZero requires nightly Rust anyway, and something like this can be defined locally in a nightly crate as well.

[nikomatsakis]

As an aside, I've definitely wanted Option<i32> (or some other integer type) where either 0 or (more commonly, for me) 0xFFFF_FFFF was a sentinel value. Usually though I achieve this with an opaque newtype wrapper.

[alexcrichton]

Closing due to inactivity, and it sounds like we may want to consider a more comprehensive redesign of NonZero, so we may want to hold off on functionality like this until we've got an RFC on the topic.