-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Stabilize TryFrom and TryInto with a convert::Infallible empty enum #58302
Conversation
r? @rkruppe (rust_highfive has picked a reviewer for you, use r? to override) |
@rfcbot fcp merge |
Team member @SimonSapin 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 none object), 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. |
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 |
/// With `Infallible` being an enum, this code is valid. | ||
/// However when `Infallible` becomes an alias for the never type, | ||
/// the two `impl`s will start to overlap | ||
/// and therefore will be disallowed by the language’s trait coherence rules. |
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.
I think we want to add something here that says “so, uh, don’t do this?”. Wording suggestions welcome.
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.
Is using one over the other preferable? Either way, maybe the answer to that should be in the docs.
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.
r=me on the code changes, thanks @SimonSapin!
src/libcore/convert.rs
Outdated
} | ||
} | ||
|
||
use fmt; |
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 this be moved up to the top of the file?
} | ||
|
||
#[stable(feature = "convert_infallible", since = "1.34.0")] | ||
impl Eq for Infallible {} |
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.
With PartialEq
and Eq
perhaps the Ord
-style traits could be included as well?
🎉 Huzzah! 🎉 Hopefully it sticks this time. (Aside: #33417 (comment)) |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
☔ The latest upstream changes (presumably #58389) made this pull request unmergeable. Please resolve the merge conflicts. |
/// | ||
/// # Future compatibility | ||
/// | ||
/// This enum has the same role as [the `!` “never” type][never], |
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.
Maybe using regular quotes instead? "never"
/// pub type Infallible = !; | ||
/// ``` | ||
/// | ||
/// … and eventually deprecate `Infallible`. |
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.
Maybe using three plain dots
/// | ||
/// | ||
/// However there is one case where `!` syntax can be used | ||
/// before `!` is stabilized as a full-fleged type: in the position of a function’s return type. |
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.
'
/// With `Infallible` being an enum, this code is valid. | ||
/// However when `Infallible` becomes an alias for the never type, | ||
/// the two `impl`s will start to overlap | ||
/// and therefore will be disallowed by the language’s trait coherence rules. |
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.
'
@rust-lang/docs, any opinion on the use of non-ASCII v.s. ASCII punctuation in docs? I use the former out of habit. |
We use it everywhere else in the doc so please use the ascii version of it. :) |
To my knowledge, the docs team has no agreed upon preference for smart quotes or ASCII quotes, so either is fine! I personally use smart quotes. |
/// | ||
/// | ||
/// However there is one case where `!` syntax can be used | ||
/// before `!` is stabilized as a full-fleged type: in the position of a function’s return type. |
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.
/// before `!` is stabilized as a full-fleged type: in the position of a function’s return type. | |
/// before `!` is stabilized as a full-fledged type: in the position of a function’s return type. |
"full-fledged" still looks wrong to me as a NZer, but Merriam-Webster seems to imply that "fully-fledged" is only a British thing.
⌛ Testing commit cf26754 with merge 2e616be96c6c14265a4f55ca59f90f143e8b36a1... |
💔 Test failed - checks-travis |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. 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 |
@bors retry |
Stabilize TryFrom and TryInto with a convert::Infallible empty enum This is the plan proposed in #33417 (comment)
☀️ Test successful - checks-travis, status-appveyor |
☀️ Test successful - checks-travis, status-appveyor |
We're missing |
library: fix some stability annotations This PR updates some stability attributes to correctly reflect when some items actually got stabilized. Found while testing rust-lang#132481. ### `core::char` / `std::char` In rust-lang#26192, the `core::char` module got "stabilized" for 1.2.0, but the `core` crate itself was still unstable until 1.6.0. In rust-lang#49698, the `std::char` module was changed to a re-export of `core::char`, making `std::char` appear as "stable since 1.2.0", even though it was already stable in 1.0.0. By marking `core::char` as stable since 1.0.0, the docs will show correct versions for both `core::char` (since 1.6.0) and `std::char` (since 1.0.0). This is also consistent with the stabilities of similar re-exported modules like `core::mem`/`std::mem` for example. ### `{core,std}::array` and `{core,std}::array::TryFromSliceError` In rust-lang#58302, the `core::array::TryFromSliceError` type got stabilized for 1.34.0, together with `TryFrom`. At that point the `core::array` module was still unstable and a `std::array` re-export didn't exist, but `core::array::TryFromSliceError` could still be named due to rust-lang#95956 to existing yet. Then, `core::array` got stabilized and `std::array` got added, first targeting 1.36.0 in rust-lang#60657, but then getting backported for 1.35.0 in rust-lang#60838. This means that `core::array` and `std::array` actually got stabilized in 1.35.0 and `core::array::TryFromSliceError` was accessible through the unstable module in 1.34.0 -- mark them as such so that the docs display the correct versions.
Rollup merge of rust-lang#132482 - lukas-code:stab-attrs, r=Noratrieb library: fix some stability annotations This PR updates some stability attributes to correctly reflect when some items actually got stabilized. Found while testing rust-lang#132481. ### `core::char` / `std::char` In rust-lang#26192, the `core::char` module got "stabilized" for 1.2.0, but the `core` crate itself was still unstable until 1.6.0. In rust-lang#49698, the `std::char` module was changed to a re-export of `core::char`, making `std::char` appear as "stable since 1.2.0", even though it was already stable in 1.0.0. By marking `core::char` as stable since 1.0.0, the docs will show correct versions for both `core::char` (since 1.6.0) and `std::char` (since 1.0.0). This is also consistent with the stabilities of similar re-exported modules like `core::mem`/`std::mem` for example. ### `{core,std}::array` and `{core,std}::array::TryFromSliceError` In rust-lang#58302, the `core::array::TryFromSliceError` type got stabilized for 1.34.0, together with `TryFrom`. At that point the `core::array` module was still unstable and a `std::array` re-export didn't exist, but `core::array::TryFromSliceError` could still be named due to rust-lang#95956 to existing yet. Then, `core::array` got stabilized and `std::array` got added, first targeting 1.36.0 in rust-lang#60657, but then getting backported for 1.35.0 in rust-lang#60838. This means that `core::array` and `std::array` actually got stabilized in 1.35.0 and `core::array::TryFromSliceError` was accessible through the unstable module in 1.34.0 -- mark them as such so that the docs display the correct versions.
This is the plan proposed in #33417 (comment)