Skip to content
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.

Sign up for GitHub

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

Jump to bottom

impl FromStr for NonZeroU8, NonZeroU16, ... #58604

Closed
bcmyers opened this issue Feb 20, 2019 · 3 comments
Closed

impl FromStr for NonZeroU8, NonZeroU16, ... #58604

bcmyers opened this issue Feb 20, 2019 · 3 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@bcmyers
Copy link

bcmyers commented Feb 20, 2019

Is there any reason why the following types couldn't implement core::str::FromStr?

  • core::num::NonZeroU8
  • core::num::NonZeroU16
  • core::num::NonZeroU32
  • core::num::NonZeroUsize
  • core::num::NonZeroU64
  • core::num::NonZeroU128
@jonas-schievink jonas-schievink added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 20, 2019
@scottmcm
Copy link
Member

Sounds plausible, though what the error type should be is an interesting question -- there's nothing in IntErrorKind that seems to be a good match for "NonZeroIsize was zero".

@bcmyers
Copy link
Author

bcmyers commented Feb 22, 2019

IntErrorKind::Underflow maybe? Although I agree that's a stretch. Otherwise, you'd have to either:

  • Introduce a new error type (ParseNonZeroUintError???), or
  • Add a variant to IntErrorKind (which might be OK since it's meant to be non-exhaustive (I think)).

In any case, these kinds of decisions are above my pay grade :). Just seems a shame that this integer functionality (FromStr) isn't available for NonZero types as well.

@hellow554
Copy link
Contributor

hellow554 commented Feb 23, 2019 •
edited
Loading

I'm taking this issue 🎉

@hellow554 hellow554 mentioned this issue Feb 25, 2019
Merged
Centril added a commit to Centril/rust that referenced this issue Mar 27, 2019
@Centril
Rollup merge of rust-lang#58717 - hellow554:nonzero_parse, r=Amanieu
c12096b 
Add FromStr impl for NonZero types

This is a WIP implementation because I do have some questions regarding the solution.

Somebody should ping the lang team on this I guess.
Please see the annotations on the code for more details.

Closes rust-lang#58604
Centril added a commit to Centril/rust that referenced this issue Mar 27, 2019
@Centril
Rollup merge of rust-lang#58717 - hellow554:nonzero_parse, r=Amanieu
7070d87 
Add FromStr impl for NonZero types

This is a WIP implementation because I do have some questions regarding the solution.

Somebody should ping the lang team on this I guess.
Please see the annotations on the code for more details.

Closes rust-lang#58604
Centril added a commit to Centril/rust that referenced this issue Mar 27, 2019
@Centril
Rollup merge of rust-lang#58717 - hellow554:nonzero_parse, r=Amanieu
44a7b9c 
Add FromStr impl for NonZero types

This is a WIP implementation because I do have some questions regarding the solution.

Somebody should ping the lang team on this I guess.
Please see the annotations on the code for more details.

Closes rust-lang#58604
Centril added a commit to Centril/rust that referenced this issue Mar 28, 2019
@Centril
Rollup merge of rust-lang#58717 - hellow554:nonzero_parse, r=oli-obk
f90ac4f 
Add FromStr impl for NonZero types

This is a WIP implementation because I do have some questions regarding the solution.

Somebody should ping the lang team on this I guess.
Please see the annotations on the code for more details.

Closes rust-lang#58604
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@hellow554 @jonas-schievink @bcmyers @scottmcm