Skip to content

add owned locked stdio handles #86799

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
Merged
merged 2 commits into from
Jul 3, 2021
Merged

add owned locked stdio handles #86799

merged 2 commits into from
Jul 3, 2021

Conversation

@tlyu
Copy link
Contributor

@tlyu tlyu commented Jul 2, 2021 •
edited
Loading

Add stderr_locked, stdin_locked, and stdout_locked free functions
to obtain owned locked stdio handles in a single step. Also add
into_lock methods to consume a stdio handle and return an owned
lock. These methods will make it easier to use locked stdio
handles without having to deal with lifetime problems or keeping
bindings to the unlocked handles around.

Fixes #85383; enables #86412.

r? @joshtriplett
@rustbot label +A-io +C-enhancement +D-newcomer-roadblock +T-libs-api
@tlyu
add owned locked stdio handles
b3db5cd 
Add stderr_locked, stdin_locked, and stdout_locked free functions to obtain owned locked stdio handles in a single step. Also add into_lock methods to consume a stdio handle and return an owned lock. These methods will make it easier to use locked stdio handles without having to deal with lifetime problems or keeping bindings to the unlocked handles around.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2021
@tlyu tlyu mentioned this pull request Jul 2, 2021
Closed
@rustbot rustbot added A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 2, 2021
@joshtriplett
Copy link
Member

joshtriplett commented Jul 2, 2021

Looks good, and thank you for adding detailed tests!

I don't think we need the type aliases to hide the lifetimes; people won't need to write them.

Also, as a minor nit, since we have the free functions for the common case, I think we should use the more grammatically flowing into_locked.

r=me with both of those addressed.
@tlyu
stdio_locked: updates based on feedback
c58ceb7 
Rename methods to `into_locked`. Remove type aliases for owned locks.
@tlyu
Copy link
Contributor Author

tlyu commented Jul 2, 2021

Looks good, and thank you for adding detailed tests!

Thanks for the review!

I don't think we need the type aliases to hide the lifetimes; people won't need to write them.

Also, as a minor nit, since we have the free functions for the common case, I think we should use the more grammatically flowing into_locked.

Both done.

r=me with both of those addressed.

I think I don't have enough privileges on bors to do that?
@joshtriplett
Copy link
Member

joshtriplett commented Jul 2, 2021

@bors r+
@bors
Copy link
Collaborator

bors commented Jul 2, 2021

📌 Commit c58ceb7 has been approved by joshtriplett
@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 Jul 2, 2021
@bors
Copy link
Collaborator

bors commented Jul 3, 2021

⌛ Testing commit c58ceb7 with merge a8b8558...
@bors
Copy link
Collaborator

bors commented Jul 3, 2021

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing a8b8558 to master...
@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 3, 2021
@bors bors merged commit a8b8558 into rust-lang:master Jul 3, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 3, 2021
@joshtriplett
Copy link
Member

joshtriplett commented Jul 3, 2021

@tlyu Sorry, I missed a detail: you need to file a tracking issue (use the library tracking issue template when filing), and then fill in the tracking issue number in the feature gate usages, rather than "none". Could you please file a follow-up PR for that?
This was referenced Jul 3, 2021
Closed
Merged
@tlyu tlyu deleted the stdio-locked branch July 3, 2021 18:48
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 12, 2021
@JohnTitor
Rollup merge of rust-lang#86846 - tlyu:stdio-locked-tracking, r=josht…
bcacfe7 
…riplett stdio_locked: add tracking issue Add the tracking issue number rust-lang#86845 to the stability attributes for the implementation in rust-lang#86799. r? `@joshtriplett` `@rustbot` label +A-io +C-cleanup +T-libs-api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. 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-api Relevant to the library API team, which will review and decide on the PR/issue.

5 participants

@tlyu @joshtriplett @bors @rust-highfive @rustbot