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.

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

Replace empty enums with Never type #1681

Merged
merged 3 commits into from
Jul 2, 2019
Merged

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Jun 19, 2019

We have two empty enums. It should be preferable to unify this.

@taiki-e
Copy link
Member Author

taiki-e commented Jun 19, 2019

Depending on the state of rust-lang/rust#57012, I think we can use Never instead of real ! type in FutureExt::never_error.

cc @Nemo157 Any thoughts?

@Nemo157
Copy link
Member

Nemo157 commented Jun 19, 2019

rust-lang/rust#58733 is tempting...

futures-core/src/never.rs Outdated Show resolved Hide resolved
@taiki-e
Copy link
Member Author

taiki-e commented Jun 19, 2019

rust-lang/rust#58733 is tempting...

It is certainly tempting but feels potentially unstable because it seems that some versions cannot compile (although they are not versions supported by this crate...).


I was thinking of copying "Future compatibility" section of std::convert::Infallible's document, but in the first place, we can use Infallible instead of having our own Never enum here.

Use Infallible directly or type Never = core::convert::Infallible may be better, considering that compatibility is taken into account when never type stabilizes.
Then when Infallible becomes type Infallible = !, we can switch to #[cfg(never_type_stabilized)] type Never = !.

@taiki-e taiki-e changed the title Replace DrainError and VecSinkError with Never Replace empty enums with Never type Jun 19, 2019
@taiki-e taiki-e force-pushed the never branch 6 times, most recently from 298f633 to a5a7225 Compare July 2, 2019 01:32
@taiki-e
Copy link
Member Author

taiki-e commented Jul 2, 2019

Use Infallible directly or type Never = core::convert::Infallible may be better, considering that compatibility is taken into account when never type stabilizes.
Then when Infallible becomes type Infallible = !, we can switch to #[cfg(never_type_stabilized)] type Never = !.

Currently this PR does this.

@taiki-e
Copy link
Member Author

taiki-e commented Jul 2, 2019

😕 This seems to happen frequently today.

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received

@taiki-e
Copy link
Member Author

taiki-e commented Jul 2, 2019

It seems that the total build time has been greatly reduced with a943d65.
before(master): 2 hrs 42 min 44 sec (https://travis-ci.com/rust-lang-nursery/futures-rs/builds/117603909)
after(this pr): 43 min 9 sec (https://travis-ci.com/rust-lang-nursery/futures-rs/builds/117710514)

@taiki-e
Copy link
Member Author

taiki-e commented Jul 2, 2019

r? @cramertj

///
/// [never]: https://doc.rust-lang.org/nightly/std/primitive.never.html
/// [infallible]: https://doc.rust-lang.org/nightly/std/convert/enum.Infallible.html#future-compatibility
pub type Never = core::convert::Infallible;
Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to impl std::error::Error for this so that it can be used as a Never error type? I know so far we've used a bunch of bounds in tower like so S::Error: Into<Box<std::error::Error>> and this would make it compat with that.

Copy link
Member Author

@taiki-e taiki-e Jul 2, 2019

Choose a reason for hiding this comment

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

@LucioFranco Infallible does not implement std::error::Error, but Into<Box<std::error::Error>> you mentioned seems to work. https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=cda064b0d4459549673d30c79d70c886

EDIT: see #1681 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thank you!

Copy link
Member Author

Choose a reason for hiding this comment

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

Infallible seems to implement std::error::Error via std::string::ParseError.

@cramertj cramertj merged commit 08dfdc6 into rust-lang:master Jul 2, 2019
@taiki-e taiki-e deleted the never branch July 2, 2019 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants