The Wayback Machine - https://web.archive.org/web/20200910202652/https://github.com/rust-lang/futures-rs/pull/2112
Skip to content

/ futures-rs

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

Simpler selection futures #2112

Open
wants to merge 7 commits into
base: master
from
Open

Simpler selection futures #2112

wants to merge 7 commits into from

Conversation

@Lucretiel
Copy link
Contributor

Lucretiel commented Apr 2, 2020 •
edited

This pull request adds first, first_all, and first_ok. These are selection functions, similar to the existing select and friends, which (unlike select) don't return the incomplete futures after finishing (simply discards them instead). This allows them to have implementations that are simpler, have less overhead, and do not require Unpin on their contents.

This pull request is in progress:

  • first! macro handled by existing select! macro.
  • Tests
    • first
    • first_all
    • first_ok, first_ok_fused
  • Resolve issues related to FusedIterator (requires #2111)

Fixes #2031 and #2110
Lucretiel added 5 commits Apr 2, 2020
@Lucretiel
Added working implementations of First, FirstAll, FirstOk
08f547a
@Lucretiel
Cleaned up First* implementations:
1b92d90 
- Removed FusedIterator; .fuse() is equivalent.
- FirstOk is more explicit about its use of FusedIterator.
- This design will have to wait for #2111 to be resolved; currently the
  contract of FusedFuture is not strict enough for our needs.
@Lucretiel
Added first_ok_fused
Loading status checks…
61a787a
@Lucretiel
Added test to first_all
d73ee1c
@Lucretiel
Added tests to first_ok
Loading status checks…
31b045d
@Lucretiel
Copy link
Contributor Author

Lucretiel commented Apr 3, 2020

The remaining test failures seem unrelated to these changes
Lucretiel added 2 commits Apr 3, 2020
@Lucretiel
Fixed all warnings; Added Deref to LengthPrefixed
Loading status checks…
1906db1
@Lucretiel
Merge branch 'master' into first
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
606d247
@cramertj
Copy link
Member

cramertj commented Apr 22, 2020

This looks good to me, though the dependence on #2111 worries me because I don't feel like I have a good handle on the best way to resolve that tension. Thanks for all your hard work here, though-- these combinators do look conveniently simpler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Simple version of select for just getting the first of two futures to resolve
2 participants
@Lucretiel @cramertj
You can’t perform that action at this time.