-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
impl TryFrom<{integer}> for NonZero{integer} #68825
Conversation
r? @dtolnay (rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
This doesn't work, but I'm not sure how to fix it ...
|
@jyn514 You should add // check-pass
use std::num::NonZeroUsize;
use std::convert::TryInto;
... |
We're going {integer} -> NonZero{integer}, not the reverse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you give a real world example where this impl would be needed?
The test shows let a: NonZeroUsize = 1_usize.try_into().unwrap();
but this isn't actually a thing we would expect anyone to write, right?
That's code that I tried to write just now, it's why I made the PR. 181 match usize::from_str_radix(s, 10) {
182 Ok(0) => Ok(None),
// this would be nicer as `Ok(i) => Ok(Some(i.try_into().unwrap()))`
183 Ok(i) => Ok(Some(NonZeroUsize::new(i).unwrap())),
184 Err(e) => Err(e),
185 } I could use |
#22639 would also have fixed my problem. |
I would recommend writing that snippet much more concisely as: s.parse().map(NonZeroUsize::new) Overall I'm not sure I buy the advantage of the genericism of TryFrom over the existing |
That snippet is indeed a lot nicer, thank you! |
I expected this to already exist and was surprised when it wasn't.
n.b. I don't think the UI tests are quite right, I can mess with them later today.