Skip to content

Change ResultShunt to be generic over Try #93572

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 8, 2022

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Feb 2, 2022

Just a refactor (and rename) for now, so it's not Result-specific.

This could be used for a future Iterator::try_collect, or similar, but anything like that is left for a future PR.

@scottmcm scottmcm added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Feb 2, 2022
@rust-highfive
Copy link
Contributor

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 2, 2022
Copy link
Member

@yaahc yaahc left a comment

Choose a reason for hiding this comment

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

very fun. I'm pretty much ready to approve, just a couple minor comments.

@yaahc
Copy link
Member

yaahc commented Feb 4, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 4, 2022

📌 Commit 399893d has been approved by yaahc

@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 Feb 4, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 6, 2022
Change `ResultShunt` to be generic over `Try`

Just a refactor (and rename) for now, so it's not `Result`-specific.

This could be used for a future `Iterator::try_collect`, or similar, but anything like that is left for a future PR.
@matthiaskrgr
Copy link
Member

Failed in a rollup: #93702 (comment)
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 6, 2022
@scottmcm
Copy link
Member Author

scottmcm commented Feb 6, 2022

Fixes for the 2021 stuff are now in #93719 -- I'll rebase this onto that later.

@scottmcm scottmcm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 6, 2022
Just a refactor (and rename) for now, so it's not `Result`-specific.

This could be used for a future `Iterator::try_collect`, or similar, but anything like that is left for a future PR.
@scottmcm scottmcm force-pushed the generic-iter-process branch from 399893d to 413945e Compare February 7, 2022 20:57
@scottmcm
Copy link
Member Author

scottmcm commented Feb 7, 2022

@bors r=yaahc

No new code changes; just rebased onto master now that my other PR landed.

@bors
Copy link
Collaborator

bors commented Feb 7, 2022

📌 Commit 413945e has been approved by yaahc

@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-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Feb 7, 2022
@scottmcm
Copy link
Member Author

scottmcm commented Feb 7, 2022

@bors rollup=iffy (failed in rollup last time, so there might still be something I missed)

@bors
Copy link
Collaborator

bors commented Feb 8, 2022

⌛ Testing commit 413945e with merge 0c292c9...

@bors
Copy link
Collaborator

bors commented Feb 8, 2022

☀️ Test successful - checks-actions
Approved by: yaahc
Pushing 0c292c9 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 8, 2022
@bors bors merged commit 0c292c9 into rust-lang:master Feb 8, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 8, 2022
@scottmcm scottmcm deleted the generic-iter-process branch February 8, 2022 17:55
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0c292c9): comparison url.

Summary: This benchmark run did not return any relevant results. 6 results were found to be statistically significant but too small to be relevant.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 18, 2022
Let `try_collect` take advantage of `try_fold` overrides

No public API changes.

With this change, `try_collect` (rust-lang#94047) is no longer going through the `impl Iterator for &mut impl Iterator`, and thus will be able to use `try_fold` overrides instead of being forced through `next` for every element.

Here's the test added, to see that it fails before this PR (once a new enough nightly is out): https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=462f2896f2fed2c238ee63ca1a7e7c56

This might as well go to the same person as my last `try_process` PR  (rust-lang#93572), so
r? `@yaahc`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 18, 2022
Let `try_collect` take advantage of `try_fold` overrides

No public API changes.

With this change, `try_collect` (rust-lang#94047) is no longer going through the `impl Iterator for &mut impl Iterator`, and thus will be able to use `try_fold` overrides instead of being forced through `next` for every element.

Here's the test added, to see that it fails before this PR (once a new enough nightly is out): https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=462f2896f2fed2c238ee63ca1a7e7c56

This might as well go to the same person as my last `try_process` PR  (rust-lang#93572), so
r? ``@yaahc``
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants