Add a try_collect() helper method to Iterator by a-lafrance · Pull Request #94041 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation19 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Implement Iterator::try_collect() as a helper around Iterator::collect() as discussed here.
First time contributor so definitely open to any feedback about my implementation! Specifically wondering if I should open a tracking issue for the unstable feature I introduced.
As the main participant in the internals discussion: r? @scottmcm
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @scottmcm (or someone else) soon.
Please see the contribution instructions for more information.
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Updated the try_collect() implementation to accept Try-not-FromIterator types like ControlFlow. Also modified the docs to get the main points of the method across better.
@rustbot label -S-waiting-on-author +S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
This comment has been minimized.
This comment has been minimized.
Status: Blocked on something else such as an RFC or other implementation work.
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
| let u: Vec<Result<i32, ()>> = vec![Ok(1), Ok(2), Ok(3)]; |
| let v = u.into_iter().try_collect::<Vec>(); |
| assert_eq!(v, Ok(vec![1, 2, 3])); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is already possible with .collect(). Thanks to impl<A, E, V> FromIterator<Result<A, E>> for Result<V, E> where V: FromIterator<A>.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but the main benefits of try_collect() are increasing the discoverability of this technique and allowing Try-but-not-FromIterator types (e.g. ControlFlow) to be fallibly collected as well. See my comment on the tracking issue here for a more thorough description of the differences between try_collect() and collect().
Tweaked try_collect() to accept more Try types
Updated feature attribute for tracking issue
The ICE in CI is caused by the use of intra-doc links involving core (the ones that are [`ControlFlow`]: core::ops::ControlFlow` and [`FromIterator`]: core::iter::FromIterator). There is more information in #73445. Obviously it shouldn't ICE and fall apart. I can't explain why that happens.
I'm pretty sure you can just reference them without qualification (like [`FromIterator`], no need for a reference) since they are in scope.
You can test documentation with ./x.py doc --stage 0 library/test. You can add the --open flag to open a web browser to inspect it.
Thanks for the info @ehuss! Just pushed a fix for the erring links, so @scottmcm I think it should be ready for a final review.
@rustbot label -S-waiting-on-author +S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
Status: Blocked on something else such as an RFC or other implementation work.
label
Thank you, ehuss! And for the PR + updates, @a-lafrance!
This looks good to go into unstable by me. Lots more discussion to be had on the tracking issue, to be sure.
@bors r+
📌 Commit 47d5196 has been approved by scottmcm
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
bors added a commit to rust-lang-ci/rust that referenced this pull request
…askrgr
Rollup of 9 pull requests
Successful merges:
- rust-lang#93337 (Update tracking issue numbers for inline assembly sub-features)
- rust-lang#93758 (Improve comments about type folding/visiting.)
- rust-lang#93780 (Generate list instead of div items in sidebar)
- rust-lang#93976 (Add MAIN_SEPARATOR_STR)
- rust-lang#94011 (Even more let_else adoptions)
- rust-lang#94041 (Add a
try_collect()helper method toIterator) - rust-lang#94043 (Fix ICE when using Box<T, A> with pointer sized A)
- rust-lang#94082 (Remove CFG_PLATFORM)
- rust-lang#94085 (Clippy: Don't lint
needless_borrowin method receiver positions)
Failed merges:
r? @ghost
@rustbot modify labels: rollup
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.