-
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
Mention Result<!, E> in never docs. #49988
Conversation
r? @sfackler (rust_highfive has picked a reviewer for you, use r? to override) |
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 |
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 |
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 |
src/libstd/primitive_docs.rs
Outdated
@@ -112,6 +112,8 @@ mod prim_bool { } | |||
/// | |||
/// # `!` and generics | |||
/// | |||
/// ## Infalliable errors |
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.
Typo: Infalliable -> 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.
this still needs fixed
src/libstd/primitive_docs.rs
Outdated
@@ -140,9 +142,59 @@ mod prim_bool { } | |||
/// [`Ok`] variant. This illustrates another behaviour of `!` - it can be used to "delete" certain | |||
/// enum variants from generic types like `Result`. | |||
/// | |||
/// ## Infinite loops | |||
/// | |||
/// While [`Result<T, !>`] is very useful for removing errors, `!` can also be used to removed |
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.
Typo: removed -> remove
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.
same
src/libstd/primitive_docs.rs
Outdated
/// instead: | ||
/// | ||
/// ```ignore (hypothetical-example) | ||
/// fn server_loop() -> Result<!, ConnectionError> { |
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 really like this example 👍 A thought: with #49162, it could even be
fn main() -> Result<!, ConnectionError>
Also, I think the Ok(
isn't needed because loop returns !
, which will coerce to the Result
: https://play.rust-lang.org/?gist=1cc0b7e8032bec565768ab95f42c454d&version=nightly
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.
My main reasoning for not making this main
is that the idea is that you might in principle want a larger loop that handles the error. Also, while I agree that the loop can be coerced to Ok
, I think that wrapping in an Ok
is best to draw parallels to a non-infinite loop. If it seems more clear the other way though, I'll definitely change it.
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'd prefer to leave off the Ok, but I can agree that this being not main makes a lot of sense
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'll leave off the Ok
and explain the coercion.
Ping from triage @steveklabnik! This PR needs your review! |
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.
Some typos, but other than that, looks good! Thank you!
src/libstd/primitive_docs.rs
Outdated
@@ -112,6 +112,8 @@ mod prim_bool { } | |||
/// | |||
/// # `!` and generics | |||
/// | |||
/// ## Infalliable errors |
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.
this still needs fixed
src/libstd/primitive_docs.rs
Outdated
@@ -140,9 +142,59 @@ mod prim_bool { } | |||
/// [`Ok`] variant. This illustrates another behaviour of `!` - it can be used to "delete" certain | |||
/// enum variants from generic types like `Result`. | |||
/// | |||
/// ## Infinite loops | |||
/// | |||
/// While [`Result<T, !>`] is very useful for removing errors, `!` can also be used to removed |
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.
same
/// | ||
/// For example, consider the case of a simple web server, which can be simplified to: | ||
/// | ||
/// ```ignore (hypothetical-example) |
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've never seen this notation, does this work?
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 actually copied it from an example earlier in the file, so, I assume so?
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.
It doesn't do anything. It's recognized as non-rust code. That's pretty much all.
src/libstd/primitive_docs.rs
Outdated
/// instead: | ||
/// | ||
/// ```ignore (hypothetical-example) | ||
/// fn server_loop() -> Result<!, ConnectionError> { |
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'd prefer to leave off the Ok, but I can agree that this being not main makes a lot of sense
I'll try and make the desired changes later today. |
@bors: rollup |
Err, I haven't made the changes yet. |
Actually made the changes; sorry it took so long. |
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 |
Ping from triage @steveklabnik ! This PR is awaiting your review. |
@bors: r+ rollup thanks! |
📌 Commit fc6d6c9 has been approved by |
Mention Result<!, E> in never docs. Fixes rust-lang#48096.
Rollup of 11 pull requests Successful merges: - #49988 (Mention Result<!, E> in never docs.) - #50148 (turn `ManuallyDrop::new` into a constant function) - #50456 (Update the Cargo submodule) - #50460 (Make `String::new()` const) - #50464 (Remove some transmutes) - #50505 (Added regression function match value test) - #50511 (Add some explanations for #[must_use]) - #50525 (Optimize string handling in lit_token().) - #50527 (Cleanup a `use` in a raw_vec test) - #50539 (Add more logarithm constants) - #49523 (Update RELEASES.md for 1.26.0) Failed merges:
Fixes #48096.