Skip to content

Introduce WithExecutor adaptor #902

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

Merged
merged 1 commit into from
Apr 3, 2018
Merged

Conversation

srijs
Copy link
Contributor

@srijs srijs commented Mar 24, 2018

Introduces the WithExecutor adapter, which assigns an executor to be used when spawning tasks from within the future.

The main driver for me is to enable the use-case of overriding the default thread pool for block_on:

block_on(future.with_executor(pool))

By making this a combinator it opens up more use-cases, such as this pattern:

spawn_with_handle(future).with_executor(pool)

///
/// This advanced method is primarily used when building "internal
/// schedulers" within a task.
pub fn with_executor<'b>(&'b mut self, executor: Option<&'b mut Executor>)
Copy link
Member

Choose a reason for hiding this comment

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

IMO this should take the executor directly instead of an Option: the executor value should only ever be None when using the no-std version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, okay! I think I was just a little confused by the fact that the Context::executor method is also flagged if_std! and returns an Option. Will change this!

@srijs
Copy link
Contributor Author

srijs commented Apr 2, 2018

Addressed feedback and rebased!

@cramertj
Copy link
Member

cramertj commented Apr 3, 2018

Thanks!

@cramertj cramertj merged commit dbb5a0b into rust-lang:master Apr 3, 2018
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.

2 participants