bolts: Simplify definition of nonzero! macro

[langston-barrett]

No description provided.

[domenukk]

Looks like this doesn't work in my tests.
We could use static_assertions for this however, a crate we're already including

[langston-barrett]

Looks like this doesn't work in my tests.

Can you say more about what not working means, and what kinds of tests you were looking at? It appears to be working ok in CI.

[domenukk]

It compiles in a panic that triggers at runtime instead of actually aborting compilation

[langston-barrett]

Can you share an example with that behavior? The following example fails at compile-time (playground):

use core::num::NonZeroUsize;

#[macro_export]
macro_rules! nonzero {
    ($val:expr) => {
        const {
            match NonZeroUsize::new($val) {
                Some(x) => x,
                None => panic!("Value was zero"),
            }
        }
    };
}

fn main() {
    println!("{}", nonzero!(0));
}

error[E0080]: evaluation of `main::{constant#0}` failed
  --> src/lib.rs:16:20
   |
16 |     println!("{}", nonzero!(0));
   |                    ^^^^^^^^^^^ the evaluated program panicked at 'Value was zero', src/lib.rs:16:20
   |

[domenukk]

That's what I tried. How is my code different? :D
https://rust.godbolt.org/z/jG65badMn

[domenukk]

Your example works. I'm a bit confused but oh well..
That way we can even just use NonZero and support all numerical types, want to change it?

[langston-barrett]

The example you posted seems quite different from the solution proposed here. Namely, you use a const fn, whereas this PR uses a macro. As noted in my comment above, const fns are only guaranteed to evaluate at compile-time when placed in a const context, e.g., a const block. Here's a simpler test-case that exhibits the difference (try commenting out the first call to nonzero to see it panic at runtime):

use core::num::NonZero;

pub fn main() {
    const { nonzero::<0>() }; // panics at compile-time
    nonzero::<0>(); // panics at runtime
}

const fn nonzero<const N: usize>() -> NonZero<usize> {
    match NonZero::new(N) {
        Some(val) => val,
        None => panic!("Zero passed to `nonzero`"),
    }
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=61a3494a15fc9ca7d635d09cffdf1554

[domenukk]

I also can't fix it by adding a const { } block inside that function though, so I'm still confused

[langston-barrett]

Right. You can think of const fn as saying "this function can be evaluated in a const context". It doesn't mean "this function will always be evaluated to a constant". They can be called from non-const contexts as well, so in a way,const fns are const-ness polymorphic. However, when they are evaluated in a const context (i.e., inside of a const block), then they are guaranteed to evaluate to a constant (i.e., at compile-time).

The nice part about this is that you don't have to make const and non-const copies of the same function if you want to use a const fn in a non-const context.

[domenukk]

Ok neat, so this actually works:

pub const fn nonzero<const N: usize>() -> NonZeroUsize {
    const {
        match NonZero::new(N) {
            Some(val) => val,
            None => panic!("expected non-zero value"),
        }
    }
}

Anyway I guess the macro is fine as well / looks a bit nicer / the errors may trigger at the right place
But we should just do NonZero::new instead of NonZeroUsize in the macro, IMHO.