Skip to content

std::io::Take should have into_inner() method #23755

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

Jump to bottom
Closed
lilyball opened this issue Mar 26, 2015 · 14 comments
Closed

std::io::Take should have into_inner() method #23755

lilyball opened this issue Mar 26, 2015 · 14 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@lilyball
Copy link
Contributor

It seems like a reasonably obvious thing to do and would be trivial to implement. The only alternative right now is to always use .by_ref() before calling .take(), and that's not always viable (e.g. because the io::Take needs to be kept around without borrowing the owner of the underlying reader).

An example usage would be implementing a message-based protocol on top of a stream protocol, where the messages either have fixed-length frames or have length indicators, I might want to call .take() to be able to work with a single message and then .into_inner() when I'm done to recover the underlying reader.
@lilyball lilyball added A-libs C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Mar 26, 2015
@huonw huonw added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 8, 2016
@huonw
Copy link
Member

huonw commented Jan 8, 2016

triage: I-nominated

A lot of library types have .into_inner methods, but I'm not sure we've ever explicitly decided that we should do it for any adaptor type like this (e.g. should the various iterator adaptors get .into_inners?), especially ones like this where &mut/.by_ref work for some/most(?) cases.

@alexcrichton
Copy link
Member

triage: P-medium

@sfackler indicated that he was interested in making a PR for this and limiting it to the I/O adaptors for now.

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated labels Jan 21, 2016
@frewsxcv
Copy link
Member

@sfackler Are you still working on this? Or can I take this?

@brson brson added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. P-low Low priority and removed P-medium Medium priority labels Aug 25, 2016
@brson
Copy link
Contributor

brson commented Aug 25, 2016

@frewsxcv you got it.

@frewsxcv frewsxcv mentioned this issue Aug 26, 2016
Merged
frewsxcv added a commit to frewsxcv/rust that referenced this issue Aug 26, 2016
@frewsxcv
Introduce into_inner method on std::io::Take.
ced1252 
rust-lang#23755
@frewsxcv
Copy link
Member

Opened a PR: #36019.

bors added a commit that referenced this issue Sep 13, 2016
@bors
Auto merge of #36019 - frewsxcv:take-into-inner, r=alexcrichton
09905b1 
Introduce `into_inner` method on `std::io::Take`.

#23755
@martinhath
Copy link
Contributor

Shouldn't this be closed?

@frewsxcv
Copy link
Member

frewsxcv commented Oct 2, 2016

Tracking issues (like this one) stay upon until the feature has stabilized. Right now, std::io::Take::into_inner is still unstable.

@sfackler
Copy link
Member

sfackler commented Oct 2, 2016

Looks like the #[unstable] tag isn't pointing at it though.

@sfackler sfackler added B-unstable Blocker: Implemented in the nightly compiler and unstable. and removed A-io E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. P-low Low priority labels Oct 2, 2016
@frewsxcv
Copy link
Member

frewsxcv commented Oct 2, 2016

Yep, was just about to ping the #rust-libs channel to add the label since I can't ;)

@frewsxcv
Copy link
Member

frewsxcv commented Oct 3, 2016

Misread what you wrote the first time I read your comment. Added the issue number to the #[unstable] in #36916.

@alexcrichton
Copy link
Member

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Nov 1, 2016 •
edited
Loading

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

No concerns currently listed.

Once these reviewers reach consensus, 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.

@aturon aturon mentioned this issue Nov 7, 2016
Closed
@rfcbot
Copy link
Collaborator

rfcbot commented Nov 12, 2016

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

psst @alexcrichton, I wasn't able to add the final-comment-period label, please do so.

@alexcrichton alexcrichton added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Nov 12, 2016
@rfcbot
Copy link
Collaborator

rfcbot commented Nov 22, 2016

The final comment period is now complete.

@aturon aturon mentioned this issue Dec 14, 2016
Merged
bors added a commit that referenced this issue Dec 18, 2016
@bors
Auto merge of #38369 - aturon:stab-1.15, r=alexcrichton
1f965cc 
Library stabilizations/deprecations for 1.15 release

Stabilized:

- `std::iter::Iterator::{min_by, max_by}`
- `std::os::*::fs::FileExt`
- `std::sync::atomic::Atomic*::{get_mut, into_inner}`
- `std::vec::IntoIter::{as_slice, as_mut_slice}`
- `std::sync::mpsc::Receiver::try_iter`
- `std::os::unix::process::CommandExt::before_exec`
- `std::rc::Rc::{strong_count, weak_count}`
- `std::sync::Arc::{strong_count, weak_count}`
- `std::char::{encode_utf8, encode_utf16}`
- `std::cell::Ref::clone`
- `std::io::Take::into_inner`

Deprecated:

- `std::rc::Rc::{would_unwrap, is_unique}`
- `std::cell::RefCell::borrow_state`

Closes #23755
Closes #27733
Closes #27746
Closes #27784
Closes #28356
Closes #31398
Closes #34931
Closes #35601
Closes #35603
Closes #35918
Closes #36105
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@lilyball @alexcrichton @brson @frewsxcv @martinhath @huonw @sfackler @rust-highfive @rfcbot