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.

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

add owned locked stdio handles #86799

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

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

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
@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

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.

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

@bors r+

@bors
Copy link
Contributor

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
Contributor

bors commented Jul 3, 2021

⌛ Testing commit c58ceb7 with merge a8b8558...

@bors
Copy link
Contributor

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

@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?

@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
…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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E0716: confusing reference to borrow of Stdin value when there's no obvious reference within StdinLock
5 participants