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

Allow async-await macros to be used without std #1891

Merged
merged 1 commit into from Jan 29, 2020

Conversation

@arcnmx
Copy link
Contributor

arcnmx commented Sep 28, 2019

select!() is the only macro that requires std due to implementation details, so allow everything else through.

@arcnmx

This comment has been minimized.

Copy link
Contributor Author

arcnmx commented Sep 28, 2019

I wanted to also address random on no-std but it seemed a bit too intrusive of a change for me to make all the design decisions...

  • armv6m only has AtomicUsize::{load, store} which means reducing the xorshift state width depending on target pointer size
  • Also means being okay with the state potentially repeating in the cases where calls overlap during the load+store and clobber each other?
  • I think I actually want a non-randomized select!() anyway that polls in priority order so would need to reimplement select anyway.

I'm probably reading too much into it given how weak the randomness requirement here really is but I left that out for now... though I am tempted to leave the macro itself in so it could still be used if you override the random fn using a facade.

@taiki-e

This comment has been minimized.

Copy link
Contributor

taiki-e commented Sep 28, 2019

(FYI: There is a discussion in #1805 about adding a select that doesn't use randomness. and seems implementation is not difficult: taiki-e@2691aa2)

@arcnmx

This comment has been minimized.

Copy link
Contributor Author

arcnmx commented Sep 28, 2019

Yeah that would be really nice, is there anything blocking that from becoming a PR? just docs? or name, default, syntax bikeshed? uncertainty around whether it should be provided?

@taiki-e

This comment has been minimized.

Copy link
Contributor

taiki-e commented Sep 28, 2019

is there anything blocking that from becoming a PR? just docs? or name, default, syntax bikeshed? uncertainty around whether it should be provided?

I didn't open the PR because the document was incomplete, but in practice, we will need to decide which is the default.

@arcnmx

This comment has been minimized.

Copy link
Contributor Author

arcnmx commented Sep 28, 2019

Yeah, I can see the argument made for keeping unbiased as the default, though don't have any personal opinions there as long as the ability to choose can be made available.

Also, I threw up what I had for no-std random and could PR in case it's something that could be accepted?

@arcnmx arcnmx force-pushed the arcnmx:async-await-nostd branch from 3a8561a to bd745d3 Oct 13, 2019
@arcnmx arcnmx mentioned this pull request Oct 13, 2019
@arcnmx arcnmx force-pushed the arcnmx:async-await-nostd branch from bd745d3 to 541a2ae Oct 16, 2019
@martell

This comment has been minimized.

Copy link

martell commented Oct 20, 2019

@arcnmx firstly thanks for looking at this,
Is there anything stopping this from being merged?
I'm looking forward to using async await without std upstream.

@arcnmx arcnmx force-pushed the arcnmx:async-await-nostd branch from 541a2ae to dbb0164 Dec 3, 2019
@arcnmx

This comment has been minimized.

Copy link
Contributor Author

arcnmx commented Dec 3, 2019

Rebased onto master, so select_biased! is now an option on no-std with these changes. select! still requires #1912 but the biased alternative being available makes it a bit less of a blocker now.

@arcnmx arcnmx force-pushed the arcnmx:async-await-nostd branch from dbb0164 to e8096cd Dec 13, 2019
@arcnmx arcnmx force-pushed the arcnmx:async-await-nostd branch from e8096cd to f5c6439 Jan 9, 2020
`select!()` is the only macro that requires std due to implementation
details, so allow everything else through.
@arcnmx arcnmx force-pushed the arcnmx:async-await-nostd branch from f5c6439 to 9d8f56e Jan 24, 2020
@arcnmx

This comment has been minimized.

Copy link
Contributor Author

arcnmx commented Jan 24, 2020

@taiki-e you've approved this previously, any chance it could be considered for merging? I've been experimentally making use of these changes for quite a while now and it'd be nice to have these available upstream - it seems like a benign change, and select_biased has made it even more appealing now.

(cc @Nemo157 as this is somewhat your domain as well)

@taiki-e

This comment has been minimized.

Copy link
Contributor

taiki-e commented Jan 29, 2020

I'd like to merge this. cc @cramertj

@cramertj

This comment has been minimized.

Copy link
Member

cramertj commented Jan 29, 2020

This looks fine to me, thanks for the ping! I'll get a release out shortly.

@cramertj cramertj merged commit 653d272 into rust-lang:master Jan 29, 2020
1 check passed
1 check passed
Travis CI - Pull Request Build Passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.