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 Termination for Infallible and then make the Result impls of Termination more generic #97803

Merged
merged 2 commits into from
Jun 18, 2022
Merged

Impl Termination for Infallible and then make the Result impls of Termination more generic #97803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1502713
Impl Termination for Infallible and then make the Result impls of Ter…
Gankra Jun 6, 2022
0894660
bless new test result, it's a regression but seemingly a compiler bug
Gankra Jun 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 15 additions & 22 deletions library/std/src/process.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2140,16 +2140,6 @@ impl Termination for () {
}
}

#[stable(feature = "termination_trait_lib", since = "1.61.0")]
impl<E: fmt::Debug> Termination for Result<(), E> {
fn report(self) -> ExitCode {
match self {
Ok(()) => ().report(),
Err(err) => Err::<!, _>(err).report(),
}
}
}

#[stable(feature = "termination_trait_lib", since = "1.61.0")]
impl Termination for ! {
fn report(self) -> ExitCode {
Expand All @@ -2158,28 +2148,31 @@ impl Termination for ! {
}

#[stable(feature = "termination_trait_lib", since = "1.61.0")]
impl<E: fmt::Debug> Termination for Result<!, E> {
impl Termination for Infallible {
fn report(self) -> ExitCode {
let Err(err) = self;
// Ignore error if the write fails, for example because stderr is
// already closed. There is not much point panicking at this point.
let _ = writeln!(io::stderr(), "Error: {err:?}");
ExitCode::FAILURE
match self {}
}
}

#[stable(feature = "termination_trait_lib", since = "1.61.0")]
impl<E: fmt::Debug> Termination for Result<Infallible, E> {
impl Termination for ExitCode {
#[inline]
fn report(self) -> ExitCode {
let Err(err) = self;
Err::<!, _>(err).report()
self
}
}

#[stable(feature = "termination_trait_lib", since = "1.61.0")]
impl Termination for ExitCode {
#[inline]
impl<T: Termination, E: fmt::Debug> Termination for Result<T, E> {
Comment on lines 2165 to +2166
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly this impl is technically new but is backfilling removed impls that were since 1.61.0 so..?

Copy link
Member

@yaahc yaahc Jun 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo backdating them both seems correct since they're both representing existing impls from that release even if they both add new impls as well. We don't have a rule for this AFAIK.

I guess the most pertinent question is "who are these versions for and why?". The only time I end up using these is when I'm curious how long a certain API has been stable or when a decision was made. I also thought that people might be using these versions for MSRV checks but my experience is that I usually just actually test against the MSRV I am aiming for supporting, so I don't think we should consider the MSRV user experience in these attributes, which will likely be inaccurate either way and would be too time consuming to determine manually.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for the stable_features diagnostic.

warning: the feature `pin` has been stable since 1.33.0 and no longer requires an attribute to enable
 --> src/main.rs:1:12
  |
1 | #![feature(pin)]
  |            ^^^
  |
  = note: `#[warn(stable_features)]` on by default

which means it is pretty much meaningless for impls, since you don't get impls by enabling a feature.

fn report(self) -> ExitCode {
self
match self {
Ok(val) => val.report(),
Err(err) => {
// Ignore error if the write fails, for example because stderr is
// already closed. There is not much point panicking at this point.
let _ = writeln!(io::stderr(), "Error: {err:?}");
ExitCode::FAILURE
}
}
}
}
9 changes: 3 additions & 6 deletions src/test/ui/rfc-1937-termination-trait/termination-trait-test-wrong-type.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0277]: `main` has invalid return type `Result<f32, ParseFloatError>`
error[E0277]: `main` has invalid return type `f32`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error now is a bit off 🤔

--> $DIR/termination-trait-test-wrong-type.rs:6:1
|
LL | #[test]
Expand All @@ -8,11 +8,8 @@ LL | | "0".parse()
LL | | }
| |_^ `main` can only return types that implement `Termination`
|
= help: the trait `Termination` is not implemented for `Result<f32, ParseFloatError>`
= help: the following other types implement trait `Termination`:
Result<!, E>
Result<(), E>
Result<Infallible, E>
= help: the trait `Termination` is not implemented for `f32`
= note: required because of the requirements on the impl of `Termination` for `Result<f32, ParseFloatError>`
note: required by a bound in `assert_test_result`
--> $SRC_DIR/test/src/lib.rs:LL:COL
|
Expand Down