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

Provide an or! macro that handles any number of futures #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshtriplett
Copy link
Contributor

This has the same effect as f1.or(f2.or(f3.or(...))) but without the
nesting.

Note that an equivalent race! macro would not be fair, since it would
be a coin-flip at each level. A fair race! macro would need to count
the number of futures, and select randomly. This could be done by adding
a version of the Race future that has a count and checks its first
future first with 1/N probability, and its second future first
otherwise.

This has the same effect as f1.or(f2.or(f3.or(...))) but without the
nesting.
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

This looks good, thank you!

I kind of wish the or! macro appeared in the future module rather than in the crate root. It might a bit tricky to achieve that though. What do you think?

@joshtriplett
Copy link
Contributor Author

@stjepang That's doable. Could you live with it appearing as both futures_lite::or_futures! (or similar placeholder) and as futures_lite::future::or!? (The alternative would, as far as I can tell, require a separate helper crate.)

Fix the documentation accordingly, and ensure it shows up in both
places.

Rename the internal macro to a more internal name.

Make the macro work without requiring the caller to import FutureExt.
@ghost
Copy link

ghost commented Aug 10, 2020

Hmm, I would be okay with a helper crate, perhaps futures-lite-macros that would allow us to export the macro into the right module. It's not like such a crate would materially impact compilation times, and I expect in the future rust version the hack will not be necessary.

@joshtriplett
Copy link
Contributor Author

joshtriplett commented Aug 10, 2020

Alright, I can try that. (It'd look a lot like this, but with the macro implementations moved to that helper crate, and then pub used here.)

@ghost
Copy link

ghost commented Aug 11, 2020

Sounds good!

@jjl
Copy link
Contributor

jjl commented Aug 11, 2020

I loved this so much I stole it for futures-micro. Also I adapted it for zip/join with mildly amusing results

@jjl
Copy link
Contributor

jjl commented Aug 11, 2020

You could actually just depend on futures-micro to get this macro now it's released, instead of extracting it out into a new crate. You'd also get an implementation of PollFn (which is itself about a third of the crate size), and i was suggesting you nick PollState anyway, which is another third of it.

@ghost
Copy link

ghost commented Aug 11, 2020

That’s actually a good idea :)

@joshtriplett
Copy link
Contributor Author

@jjl: I think the implementation of zip should pattern-match out of the nested 2-tuples, and emit an N-tuple. That isn't hard to do in a macro, and it would substantially improve usability.

@jjl
Copy link
Contributor

jjl commented Aug 12, 2020

i completely agree, I just haven't gotten around to figuring it out yet.

@jjl
Copy link
Contributor

jjl commented Aug 13, 2020

now done

jjl added a commit to jjl/futures-lite that referenced this pull request Aug 23, 2020
@jacobrosenthal
Copy link

I would use this :)

@jjl
Copy link
Contributor

jjl commented Aug 27, 2020

you already can, it's in the released futures-micro

@jacobrosenthal
Copy link

jacobrosenthal commented Aug 27, 2020 via email

@jjl
Copy link
Contributor

jjl commented Aug 27, 2020

there's a PR for that! #15

@notgull
Copy link
Member

notgull commented Apr 28, 2023

Is there still any interest in this?

Personally I'm a fan of the fact that futures-lite doesn't contain any obscure macros right now, and I think that a.or(b).or(c) looks better than or!(a, b, c). Still, it looks like people were interested in this above, so there might still be demand for it now.

@taiki-e
Copy link
Collaborator

taiki-e commented Jul 16, 2023

a.or(b).or(c)

I think such a chain is basically an anti-pattern because polling will become unfair. tokio-rs/tokio#2319

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants