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

Update Tokio to use std::future. #1120

Merged
merged 27 commits into from
Jun 24, 2019
Merged

Update Tokio to use std::future. #1120

merged 27 commits into from
Jun 24, 2019

Conversation

carllerche
Copy link
Member

Remove usage of 0.1 futures and switch to std::future.

Work in progress. Additional changes will be submitted via PRs to this branch.

impl<T: ?Sized + AsyncWrite> AsyncWrite for Box<T> {
fn shutdown(&mut self) -> Poll<(), std_io::Error> {
(**self).shutdown()
impl<T: ?Sized + Unpin + AsyncWrite> AsyncWrite for Box<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

crazy nit, but maybe the AsyncX and Unpin should be sorted in a consistent order?

@quininer
Copy link
Member

I noticed that tokio-io and tokio-futures have some features similar to futures-io and futures-sink. Will this be a long-term split?

@carllerche
Copy link
Member Author

@quininer I have not been following developments in the futures crate nor am I aware of any of their intent w.r.t. Tokio. As of now, I intend to keep things in Tokio. Once things settle, we can evaluate.

@quininer
Copy link
Member

I think we should establish contact with futures-rs as early as possible. @cramertj what do you think?

@qm3ster
Copy link
Contributor

qm3ster commented Jun 18, 2019

Perhaps it's best to first convert to what the futures compatibility layer would produce for the tokio constructs, release that, and only then consider what breaking changes to make to simplify api (such as removing some errors).

They must be suitable, otherwise these conversions for io and streams wouldn't have been possible.

@cramertj
Copy link
Contributor

@quininer Yes, futures-io covers the definition of the AsyncRead/AsyncWrite/AsyncBufRead/AsyncSeek traits, and futures-util provides the combinators for them. i'd expect that the corresponding parts of tokio-io could be replaced with these.

With respect to tokio-futures, it looks like it's a re-definition of the ready! macro, the Sink trait (from futures-sink, and a re-export of core::future::Future and futures_core::stream::Stream. I don't have any particular opinion on this, although it would be nice to use the same Sink trait.

Equivalent utilities will be re-introduced in other crates.
@carllerche carllerche marked this pull request as ready for review June 24, 2019 19:18
@carllerche carllerche merged commit 06c473e into master Jun 24, 2019
@taiki-e
Copy link
Member

taiki-e commented Jun 30, 2019

@cramertj

it looks like it's a re-definition of the ready! macro,

I think we can move ready! macro to futures-core from futures-util (It is stable enough, imo).

@cramertj
Copy link
Contributor

cramertj commented Jul 1, 2019

@taiki-e Sounds good to me!

}
}

// ===== impl Read / Write for &'a =====
Copy link

Choose a reason for hiding this comment

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

Why were these removed?

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.

7 participants