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

Add Future and task system to the standard library #51263

Merged
merged 1 commit into from
Jun 6, 2018
Merged

Add Future and task system to the standard library #51263

merged 1 commit into from
Jun 6, 2018

Conversation

cramertj
Copy link
Member

This adds preliminary versions of the std::future and std::task modules in order to unblock development of async/await (#50547). These shouldn't be considered as final forms of these libraries-- design questions about the libraries should be left on rust-lang/rfcs#2418. Once that RFC (or a successor) is merged, these APIs will be adjusted as necessary.

r? @aturon

@cramertj cramertj requested a review from withoutboats May 31, 2018 23:47
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 31, 2018
@rust-highfive

This comment has been minimized.

Sign in to view
@cramertj

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
aturon
aturon reviewed Jun 5, 2018
View reviewed changes
src/liballoc/task.rs Outdated
/// be `poll`ed. This function is like `wake`, but can only be called from the
/// thread on which this `Wake` was created.
///
/// Executors generally maintain a queue of "ready" tasks; `wake` should place
Copy link
Member

Choose a reason for hiding this comment

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

wake_local

aturon
aturon approved these changes Jun 5, 2018
View reviewed changes
Copy link
Member

@aturon aturon left a comment

Choose a reason for hiding this comment

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

Looks great! I quite like the way the LocalWaker pieces have worked out.

aturon
aturon reviewed Jun 5, 2018
View reviewed changes
src/liballoc/task.rs Outdated
/// is local to the current thread. The resulting `LocalWaker` will call
/// `wake.wake()` when awoken.
#[inline]
pub fn local_waker_from_nonlocal<W: Wake + 'static>(wake: Arc<W>) -> LocalWaker {
Copy link
Member

Choose a reason for hiding this comment

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

Could this be a From coercion? Or do you prefer to keep it explicit?

The fact that we can't add it as an inherent constructor is a shame. But we can iterate on this later.

Copy link
Member Author

Choose a reason for hiding this comment

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

LocalWaker isn't defined in liballoc, so we can't add a From<impl Wake + 'static> for LocalWaker here. In any case, I think it might be a bit too tricky for users to figure out that wake_local won't be called via the From impl.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@aturon
Copy link
Member

aturon commented Jun 6, 2018

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 6, 2018

📌 Commit 83b69db has been approved by aturon

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 6, 2018
@bors
Copy link
Contributor

bors commented Jun 6, 2018

⌛ Testing commit 83b69db209f461c1bdd2e3631838e6af295d1b01 with merge 37bd8b0c04855cd0d082c2d1899763b4531a748b...

@bors
Copy link
Contributor

bors commented Jun 6, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 6, 2018
@rust-highfive

This comment has been minimized.

Sign in to view
1 similar comment
@rust-highfive

This comment has been minimized.

Sign in to view
@Mark-Simulacrum
Copy link
Member 

[01:10:15] error[E0432]: unresolved import `arc`
[01:10:15] --> liballoc/task.rs:13:5
[01:10:15]    |
[01:10:15]    | use arc::Arc;
[01:10:15] 
[01:10:15] error[E0207]: the type parameter `T` is not constrained by the impl trait, self type, or predicates[0m
[01:10:15] --> liballoc/task.rs:74:6
[01:10:15]    | impl<T> From<Arc<T>> for Waker

@cramertj
Copy link
Member Author

cramertj commented Jun 6, 2018

TIL Arc is cfg'd out on some platforms...

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 6, 2018
@bors
Copy link
Contributor

bors commented Jun 6, 2018

📌 Commit a6055c8 has been approved by aturon

@rust-lang rust-lang deleted a comment from bors Jun 6, 2018
@bors
Copy link
Contributor

bors commented Jun 6, 2018

⌛ Testing commit a6055c8 with merge 19d0b53...

bors added a commit that referenced this pull request Jun 6, 2018
@bors
Auto merge of #51263 - cramertj:futures-in-core, r=aturon
19d0b53 
Add Future and task system to the standard library

This adds preliminary versions of the `std::future` and `std::task` modules in order to unblock development of async/await (#50547). These shouldn't be considered as final forms of these libraries-- design questions about the libraries should be left on rust-lang/rfcs#2418. Once that RFC (or a successor) is merged, these APIs will be adjusted as necessary.

r? @aturon
@bors
Copy link
Contributor

bors commented Jun 6, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: aturon
Pushing 19d0b53 to master...

@bors bors merged commit a6055c8 into rust-lang:master Jun 6, 2018
@bors bors mentioned this pull request Jun 6, 2018
Closed
@cramertj cramertj deleted the futures-in-core branch June 6, 2018 22:46
@aturon aturon mentioned this pull request Jun 6, 2018
Closed
13 tasks
@fluxxu fluxxu mentioned this pull request Jun 8, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aturon aturon aturon approved these changes

@withoutboats withoutboats Awaiting requested review from withoutboats

Assignees

@aturon aturon

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cramertj @rust-highfive @aturon @bors @Mark-Simulacrum