-
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
Add TryFrom<{int}> for NonZero{int} #72717
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dtolnay (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
I'm sorry for the duplication, I checked before starting but I see someone else already made a PR ( #72712 ) for it in between. |
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.
Thanks for the PR! In this case we already have a variation of this going in #72712. I'll close your PR to keep the discussion in one place. If the other PR's author stops being responsive, feel free to resubmit the changes, making sure to address any discussion that took place in that PR, and drop a link to your new PR in #72712.
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 |
Since #72712 got closed for being too large and the final comment is:
Should I reopen this PR ? (Mentioning @rust-lang/libs since it concerns them) EDIT: Once again someone started another one before me, I'm sorry I pinged the people following this for nothing. |
Reopening because it looks like this is in better shape (with tests) than #72846, and was opened first anyway. |
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 |
I used the version 1.46.0 for the |
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 |
a0ab96e
to
b70d26f
Compare
b70d26f
to
2b8e2f8
Compare
8c01a90
to
6c8d8d6
Compare
@rust-lang/libs: This adds some stable TryFrom impls: impl TryFrom<i8> for NonZeroI8 {...}
impl TryFrom<i16> for NonZeroI16 {...}
impl TryFrom<i32> for NonZeroI32 {...}
impl TryFrom<i64> for NonZeroI64 {...}
impl TryFrom<i128> for NonZeroI128 {...}
impl TryFrom<isize> for NonZeroIsize {...}
impl TryFrom<u8> for NonZeroU8 {...}
impl TryFrom<u16> for NonZeroU16 {...}
impl TryFrom<u32> for NonZeroU32 {...}
impl TryFrom<u64> for NonZeroU64 {...}
impl TryFrom<u128> for NonZeroU128 {...}
impl TryFrom<usize> for NonZeroUsize {...} where those NonZero* types are all the ones under https://doc.rust-lang.org/std/num/index.html, and the |
Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
I'm in favor of this, but I'm curious what changed between #68825 being rejected and this PR being accepted? The concern then was that people wouldn't ever use it, which doesn't seem to have been discussed here. |
By the fourth PR (#68825, #72712, #72846, #72717) I lost interest in talking people out of wanting this. They're unsurprising impls even if downstream could always write their code a better way (like #68825 (comment) --> #68825 (comment)). |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
@bors r+ |
📌 Commit 6c8d8d6 has been approved by |
☀️ Test successful - checks-azure |
according to various people on tech-pkg@, there are no problems with the Firefox build Version 1.46.0 (2020-08-27) ========================== Language -------- - [`if`, `match`, and `loop` expressions can now be used in const functions.][72437] - [Additionally you are now also able to coerce and cast to slices (`&[T]`) in const functions.][73862] - [The `#[track_caller]` attribute can now be added to functions to use the function's caller's location information for panic messages.][72445] - [Recursively indexing into tuples no longer needs parentheses.][71322] E.g. `x.0.0` over `(x.0).0`. - [`mem::transmute` can now be used in static and constants.][72920] **Note** You currently can't use `mem::transmute` in constant functions. Compiler -------- - [You can now use the `cdylib` target on Apple iOS and tvOS platforms.][73516] - [Enabled static "Position Independent Executables" by default for `x86_64-unknown-linux-musl`.][70740] Libraries --------- - [`mem::forget` is now a `const fn`.][73887] - [`String` now implements `From<char>`.][73466] - [The `leading_ones`, and `trailing_ones` methods have been stabilised for all integer types.][73032] - [`vec::IntoIter<T>` now implements `AsRef<[T]>`.][72583] - [All non-zero integer types (`NonZeroU8`) now implement `TryFrom` for their zero-able equivalent (e.g. `TryFrom<u8>`).][72717] - [`&[T]` and `&mut [T]` now implement `PartialEq<Vec<T>>`.][71660] - [`(String, u16)` now implements `ToSocketAddrs`.][73007] - [`vec::Drain<'_, T>` now implements `AsRef<[T]>`.][72584] Stabilized APIs --------------- - [`Option::zip`] - [`vec::Drain::as_slice`] Cargo ----- Added a number of new environment variables that are now available when compiling your crate. - [`CARGO_BIN_NAME` and `CARGO_CRATE_NAME`][cargo/8270] Providing the name of the specific binary being compiled and the name of the crate. - [`CARGO_PKG_LICENSE`][cargo/8325] The license from the manifest of the package. - [`CARGO_PKG_LICENSE_FILE`][cargo/8387] The path to the license file. Compatibility Notes ------------------- - [The target configuration option `abi_blacklist` has been renamed to `unsupported_abis`.][74150] The old name will still continue to work. - [Rustc will now warn if you cast a C-like enum that implements `Drop`.][72331] This was previously accepted but will become a hard error in a future release. - [Rustc will fail to compile if you have a struct with `#[repr(i128)]` or `#[repr(u128)]`.][74109] This representation is currently only allowed on `enum`s. - [Tokens passed to `macro_rules!` are now always captured.][73293] This helps ensure that spans have the correct information, and may cause breakage if you were relying on receiving spans with dummy information. - [The InnoSetup installer for Windows is no longer available.][72569] This was a legacy installer that was replaced by a MSI installer a few years ago but was still being built. - [`{f32, f64}::asinh` now returns the correct values for negative numbers.][72486] - [Rustc will no longer accept overlapping trait implementations that only differ in how the lifetime was bound.][72493] - [Rustc now correctly relates the lifetime of an existential associated type.][71896] This fixes some edge cases where `rustc` would erroneously allow you to pass a shorter lifetime than expected. - [Rustc now dynamically links to `libz` (also called `zlib`) on Linux.][74420] The library will need to be installed for `rustc` to work, even though we expect it to be already available on most systems. - [Tests annotated with `#[should_panic]` are broken on ARMv7 while running under QEMU.][74820] - [Pretty printing of some tokens in procedural macros changed.][75453] The exact output returned by rustc's pretty printing is an unstable implementation detail: we recommend any macro relying on it to switch to a more robust parsing system. [75453]: rust-lang/rust#75453 [74820]: rust-lang/rust#74820 [74420]: rust-lang/rust#74420 [74109]: rust-lang/rust#74109 [74150]: rust-lang/rust#74150 [73862]: rust-lang/rust#73862 [73887]: rust-lang/rust#73887 [73466]: rust-lang/rust#73466 [73516]: rust-lang/rust#73516 [73293]: rust-lang/rust#73293 [73007]: rust-lang/rust#73007 [73032]: rust-lang/rust#73032 [72920]: rust-lang/rust#72920 [72569]: rust-lang/rust#72569 [72583]: rust-lang/rust#72583 [72584]: rust-lang/rust#72584 [72717]: rust-lang/rust#72717 [72437]: rust-lang/rust#72437 [72445]: rust-lang/rust#72445 [72486]: rust-lang/rust#72486 [72493]: rust-lang/rust#72493 [72331]: rust-lang/rust#72331 [71896]: rust-lang/rust#71896 [71660]: rust-lang/rust#71660 [71322]: rust-lang/rust#71322 [70740]: rust-lang/rust#70740 [cargo/8270]: rust-lang/cargo#8270 [cargo/8325]: rust-lang/cargo#8325 [cargo/8387]: rust-lang/cargo#8387 [`Option::zip`]: https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.zip [`vec::Drain::as_slice`]: https://doc.rust-lang.org/stable/std/vec/struct.Drain.html#method.as_slice
Pkgsrc changes: * Portability patches for Illumos have been intregrated upstream, so are no longer needed in pkgsrc. * Adjust one other patch, and update vendor/libc cargo checksum. Upstream changes: Version 1.46.0 (2020-08-27) ========================== Language -------- - [`if`, `match`, and `loop` expressions can now be used in const functions.] [72437] - [Additionally you are now also able to coerce and cast to slices (`&[T]`) in const functions.][73862] - [The `#[track_caller]` attribute can now be added to functions to use the function's caller's location information for panic messages.][72445] - [Recursively indexing into tuples no longer needs parentheses.][71322] E.g. `x.0.0` over `(x.0).0`. - [`mem::transmute` can now be used in static and constants.][72920] **Note** You currently can't use `mem::transmute` in constant functions. Compiler -------- - [You can now use the `cdylib` target on Apple iOS and tvOS platforms.][73516] - [Enabled static "Position Independent Executables" by default for `x86_64-unknown-linux-musl`.][70740] Libraries --------- - [`mem::forget` is now a `const fn`.][73887] - [`String` now implements `From<char>`.][73466] - [The `leading_ones`, and `trailing_ones` methods have been stabilised for all integer types.][73032] - [`vec::IntoIter<T>` now implements `AsRef<[T]>`.][72583] - [All non-zero integer types (`NonZeroU8`) now implement `TryFrom` for their zero-able equivalent (e.g. `TryFrom<u8>`).][72717] - [`&[T]` and `&mut [T]` now implement `PartialEq<Vec<T>>`.][71660] - [`(String, u16)` now implements `ToSocketAddrs`.][73007] - [`vec::Drain<'_, T>` now implements `AsRef<[T]>`.][72584] Stabilized APIs --------------- - [`Option::zip`] - [`vec::Drain::as_slice`] Cargo ----- Added a number of new environment variables that are now available when compiling your crate. - [`CARGO_BIN_NAME` and `CARGO_CRATE_NAME`][cargo/8270] Providing the name of the specific binary being compiled and the name of the crate. - [`CARGO_PKG_LICENSE`][cargo/8325] The license from the manifest of the package. - [`CARGO_PKG_LICENSE_FILE`][cargo/8387] The path to the license file. Compatibility Notes ------------------- - [The target configuration option `abi_blacklist` has been renamed to `unsupported_abis`.][74150] The old name will still continue to work. - [Rustc will now warn if you have a C-like enum that implements `Drop`.][72331] This was previously accepted but will become a hard error in a future release. - [Rustc will fail to compile if you have a struct with `#[repr(i128)]` or `#[repr(u128)]`.][74109] This representation is currently only allowed on `enum`s. - [Tokens passed to `macro_rules!` are now always captured.][73293] This helps ensure that spans have the correct information, and may cause breakage if you were relying on receiving spans with dummy information. - [The InnoSetup installer for Windows is no longer available.][72569] This was a legacy installer that was replaced by a MSI installer a few years ago but was still being built. - [`{f32, f64}::asinh` now returns the correct values for negative numbers.] [72486] - [Rustc will no longer accept overlapping trait implementations that only differ in how the lifetime was bound.][72493] - [Rustc now correctly relates the lifetime of an existential associated type.][71896] This fixes some edge cases where `rustc` would erroneously allow you to pass a shorter lifetime than expected. - [Rustc now dynamically links to `libz` (also called `zlib`) on Linux.][74420] The library will need to be installed for `rustc` to work, even though we expect it to be already available on most systems. - [Tests annotated with `#[should_panic]` are broken on ARMv7 while running under QEMU.][74820] - [Pretty printing of some tokens in procedural macros changed.][75453] The exact output returned by rustc's pretty printing is an unstable implementation detail: we recommend any macro relying on it to switch to a more robust parsing system. [75453]: rust-lang/rust#75453 [74820]: rust-lang/rust#74820 [74420]: rust-lang/rust#74420 [74109]: rust-lang/rust#74109 [74150]: rust-lang/rust#74150 [73862]: rust-lang/rust#73862 [73887]: rust-lang/rust#73887 [73466]: rust-lang/rust#73466 [73516]: rust-lang/rust#73516 [73293]: rust-lang/rust#73293 [73007]: rust-lang/rust#73007 [73032]: rust-lang/rust#73032 [72920]: rust-lang/rust#72920 [72569]: rust-lang/rust#72569 [72583]: rust-lang/rust#72583 [72584]: rust-lang/rust#72584 [72717]: rust-lang/rust#72717 [72437]: rust-lang/rust#72437 [72445]: rust-lang/rust#72445 [72486]: rust-lang/rust#72486 [72493]: rust-lang/rust#72493 [72331]: rust-lang/rust#72331 [71896]: rust-lang/rust#71896 [71660]: rust-lang/rust#71660 [71322]: rust-lang/rust#71322 [70740]: rust-lang/rust#70740 [cargo/8270]: rust-lang/cargo#8270 [cargo/8325]: rust-lang/cargo#8325 [cargo/8387]: rust-lang/cargo#8387 [`Option::zip`]: https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.zip [`vec::Drain::as_slice`]: https://doc.rust-lang.org/stable/std/vec/struct.Drain.html#method.as_slice
Adds
TryFrom<{int}> for NonZero{int}
.It uses the existing
NonZero{int}::new()
andOption::ok_or()
functions, meaning the checks are not repeated.I also added tests, I tried to follow the convention I saw in the test file.
I also used
#[stable(feature = "nzint_try_from_int_conv", since = "1.46.0")]
, but I have no idea if the feature/version are correctly named or even correct.