-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Document the Termination trait's impls #96819
Conversation
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon. Please see the contribution instructions for more information. |
/// If the result is `Ok`, returns `ExitCode::SUCCESS`. | ||
/// | ||
/// If the result is `Err`, writes the [`fmt::Debug`] representation of | ||
/// the error value to [`io::stderr`] and returns `ExitCode::FAILURE`. |
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 don't want to guarantee this behavior, because I expect us to change it in the future.
In particular, if we add the "generic member access" mechanism, we could then change this trait implementation to attempt to retrieve an ExitCode
from E
, and use that if available.
/// Writes the [`fmt::Debug`] representation of the error value to | ||
/// [`io::stderr`] and returns `ExitCode::FAILURE`. |
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.
As above, I expect us to change this in the future.
/// Writes the [`fmt::Debug`] representation of the error value to | ||
/// [`io::stderr`] and returns `ExitCode::FAILURE`. |
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.
As above, I expect us to change this in the future.
cc @yaahc |
I'd recommend that someone should document this lack of guarantee, then, otherwise it seems likely that people will rely on the current behaviour. (Well, to be honest I think people will rely on the current behaviour whatever the documentation says.) |
@mattheww I'd actually like to see a form of this PR go in; I just think it needs a little bit more information suggesting that the behavior may change. Would you be up for adding that language? |
I would not. That text will have to be written by someone who knows what the intended semantics are, and I see no value in it passing through my fingers. |
The library docs should be describing what its implementations of
Termination::report
do, because this is behaviour we intend users to rely on.There is some related discussion in rust-lang/reference#1196.