Add doc for NonZero* const creation
#146924
Conversation
|
r? @ibraheemdev rustbot has assigned @ibraheemdev. Use |
|
Since this issue is not restricted to NonZero, but more generally applies to const functions returning an Option or Result, another idea is to add such information instead to https://doc.rust-lang.org/nightly/std/keyword.const.html#compile-time-constants. |
True but I did not come across this issue anywhere besides non-zero values really. |
| /// ``` | ||
| #[doc = concat!("use std::num::", stringify!($Ty), ";")] | ||
| /// | ||
| #[doc = concat!("const TEN: ", stringify!($Ty), " = ", stringify!($Ty) , r#"::new(10).expect("ten is non-zero");"#)] |
There was a problem hiding this comment.
How about using an inline-const-block here?
There was a problem hiding this comment.
So what @clubby789 asked to remove? #146924 (comment)
There was a problem hiding this comment.
No, but the whole const TEN item:
| #[doc = concat!("const TEN: ", stringify!($Ty), " = ", stringify!($Ty) , r#"::new(10).expect("ten is non-zero");"#)] | |
| #[doc = concat!("let ten = const { ", stringify!($Ty) , r#"::new(10).expect("ten is non-zero") };"#)] |
There was a problem hiding this comment.
I think both are fine. Why should we prefer this one?
There was a problem hiding this comment.
I prefer the const TEN: - we're probably
8000
documenting this to someone relatively new to the language, and a global const variable is more familiar/conceptually simpler than an inline const block imo
There was a problem hiding this comment.
We can have both. const X for https://doc.rust-lang.org/nightly/std/keyword.const.html#compile-time-constants and const {} for https://doc.rust-lang.org/nightly/std/keyword.const.html#compile-time-blocks. (Or do both here, but I've seen no evidence that NonZero is the dominant pattern for this.)
There was a problem hiding this comment.
we're probably documenting this to someone relatively new to the language
That's mostly why I advocated for const-blocks – I'd like more people to be aware of them. But I don't feel strongly, the current version is fine.
|
✌️ @cptpiepmatz, you can now approve this pull request! If @joboet told you to " |
55487c1 to
185ae69
Compare
…tion, r=joboet Add doc for `NonZero*` const creation I ran into trouble using `NonZero*` values because I didn’t see any clear way to create them at compile time. At first I ended up using `NonZero*::new_unchecked` a lot, until I realized that `Option::unwrap` and `Option::expect` are `const` and can be used in a `const` context. With that, you can create non-zero values at compile time safely, without touching `unsafe`. This wasn’t obvious to me and my peers who’ve been using Rust for a while, so I thought adding a note to the docs would make it easier for others to discover. If this should be worded differently or placed in another location, we can do that. I just want to make this more obvious.
Rollup of 8 pull requests Successful merges: - #116882 (rustdoc: hide `#[repr]` if it isn't part of the public ABI) - #135771 ([rustdoc] Add support for associated items in "jump to def" feature) - #141032 (avoid violating `slice::from_raw_parts` safety contract in `Vec::extract_if`) - #142401 (Add proper name mangling for pattern types) - #146293 (feat: non-panicking `Vec::try_remove`) - #146859 (BTreeMap: Don't leak allocators when initializing nodes) - #146924 (Add doc for `NonZero*` const creation) - #146933 (Make `render_example_with_highlighting` return an `impl fmt::Display`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #146924 - cptpiepmatz:doc-nonzero-const-creation, r=joboet Add doc for `NonZero*` const creation I ran into trouble using `NonZero*` values because I didn’t see any clear way to create them at compile time. At first I ended up using `NonZero*::new_unchecked` a lot, until I realized that `Option::unwrap` and `Option::expect` are `const` and can be used in a `const` context. With that, you can create non-zero values at compile time safely, without touching `unsafe`. This wasn’t obvious to me and my peers who’ve been using Rust for a while, so I thought adding a note to the docs would make it easier for others to discover. If this should be worded differently or placed in another location, we can do that. I just want to make this more obvious.
Rollup of 8 pull requests Successful merges: - rust-lang/rust#116882 (rustdoc: hide `#[repr]` if it isn't part of the public ABI) - rust-lang/rust#135771 ([rustdoc] Add support for associated items in "jump to def" feature) - rust-lang/rust#141032 (avoid violating `slice::from_raw_parts` safety contract in `Vec::extract_if`) - rust-lang/rust#142401 (Add proper name mangling for pattern types) - rust-lang/rust#146293 (feat: non-panicking `Vec::try_remove`) - rust-lang/rust#146859 (BTreeMap: Don't leak allocators when initializing nodes) - rust-lang/rust#146924 (Add doc for `NonZero*` const creation) - rust-lang/rust#146933 (Make `render_example_with_highlighting` return an `impl fmt::Display`) r? `@ghost` `@rustbot` modify labels: rollup
I ran into trouble using
NonZero*values because I didn’t see any clear way to create them at compile time. At first I ended up usingNonZero*::new_uncheckeda lot, until I realized thatOption::unwrapandOption::expectareconstand can be used in aconstcontext. With that, you can create non-zero values at compile time safely, without touchingunsafe. This wasn’t obvious to me and my peers who’ve been using Rust for a while, so I thought adding a note to the docs would make it easier for others to discover.If this should be worded differently or placed in another location, we can do that. I just want to make this more obvious.