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.

Sign up for GitHub

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

Jump to bottom

RFC: Selfexhausting iter adapter #2370

Closed
wants to merge 5 commits into from
Closed

RFC: Selfexhausting iter adapter #2370

wants to merge 5 commits into from

Conversation

Emerentius
Copy link

@Emerentius Emerentius commented Mar 24, 2018 •
edited
Loading

Rendered

Add an adapter .exhausting() to Iterator which causes the iterator to be driven to its end on drop. Before dropping it will act exactly like the source iterator.

One of two RFCs for splitting the functionality of drain into two orthogonal APIs.
Other RFC: #2369

@Emerentius Emerentius mentioned this pull request Mar 24, 2018
Closed
@Centril Centril added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Mar 24, 2018
scottmcm
scottmcm reviewed Mar 26, 2018
View reviewed changes
text/0000-selfexhausting_iter_adapter.md Outdated
# Implementation

The `exhausting()` method should take the iterator by value.
During iteration, `Exhausting` is a trivial wrapper that acts like `&mut Self`, meaning it implements all the Iterator traits that the contained iter implements and will always do external iteration. On drop, it runs `for _ in self {}`.
Copy link
Member

Choose a reason for hiding this comment

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

Why always external iteration? What would keep it from doing internal iteration with try_fold?

Copy link
Author

@Emerentius Emerentius Mar 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

You're right. I was confused about internal iteration and the requirement for consumption.

@Emerentius
correct mistake about external iteration
71fd7f5 
try_fold makes it possible to use internal iteration
@main--
Copy link

main-- commented Mar 26, 2018

Add exhausting to the FusedIterator trait to side-step the issue of non-fused iterators altogether.

Note that the flag workaround you propose in the drawbacks section is basically a reimplementation of core::iter::Fuse<I>. I think this clearly shows that this API is designed/"wants to" work with fused iterators only, so I'm very much in favor of this alternative.

@Emerentius Emerentius mentioned this pull request Jun 12, 2018
Open
3 tasks
@SimonSapin
Copy link
Contributor

Thank you for writing this RFC! The libs team discussed this today, and we felt that this seems too niche of a functionality and that the motivation seemed too weak, especially with the accompanying non-exhausting-drain RFC being postponed for a future unified design that includes other functionality such as filtering-drain.

So we’ll likely not add this to the standard library at this time:

@rfcbot fcp close

Consider experimenting with this API in a crate, possibly in https://crates.io/crates/itertools.

@rfcbot
Copy link
Collaborator

rfcbot commented Aug 8, 2018 •
edited by dtolnay
Loading

Team member @SimonSapin has proposed to close this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-close This RFC is in PFCP or FCP with a disposition to close it. labels Aug 8, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Aug 9, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Aug 9, 2018
@Emerentius
Copy link
Author

I had forgotten this was still open. It's a good idea to close it for now. I might bring it up again, if it fits together with an improved generalized drain.
I'll cut this short and just close it directly.

@Emerentius Emerentius closed this Aug 9, 2018
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Aug 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottmcm scottmcm scottmcm left review comments

Assignees
No one assigned
Labels
disposition-close This RFC is in PFCP or FCP with a disposition to close it. finished-final-comment-period The final comment period is finished for this RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Emerentius @main-- @SimonSapin @rfcbot @scottmcm @Centril