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 documentation about BorrowedFd::to_owned. #93354

Merged

Conversation

sunfishcode
Copy link
Member

Following up on #88564, this adds documentation explaining why
BorrowedFd::to_owned returns another BorrowedFd rather than an
OwnedFd. And similar for BorrowedHandle and BorrowedSocket.

r? @joshtriplett

Following up on rust-lang#88564, this adds documentation explaining why
`BorrowedFd::to_owned` returns another `BorrowedFd` rather than an
`OwnedFd`. And similar for `BorrowedHandle` and `BorrowedSocket`.
@sunfishcode sunfishcode added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 27, 2022
@joshtriplett
Copy link
Member

What happened with the idea of just omitting the Copy and Clone impls to avoid confusion?

@sunfishcode
Copy link
Member Author

@joshtriplett I added this comment to #88564. It turns out, there are places where having BorrowedFd be Copy is convenient.

@sunfishcode
Copy link
Member Author

Also, if #93869 is accepted, we should expect using BorrowedFd by value to be even more common, because users will be able to use BorrowedFd instead of &BorrowedFd in some common situations.

@sunfishcode
Copy link
Member Author

Also, if #93869 is accepted, we should expect using BorrowedFd by value to be even more common, because users will be able to use BorrowedFd instead of &BorrowedFd in some common situations.

#93869 is now replaced by #93888, but this concern still applies. The common pattern for AsFd APIs is this:

pub fn library_func<Fd: AsFd>(fd: Fd) {
    ...
}

so user code that does something simple like this:

fn user_code(fd: BorrowedFd) {
    library_func(fd);
    library_func(fd);
}

ends up depending on BorrowedFd implementing Copy.

@joshtriplett
Copy link
Member

This seems like the best of a bunch of bad options here. We can't make it dup, and even if we could that would also be surprising. It's confusing that to_owned doesn't return OwnedFd, but it can't.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 2, 2022

📌 Commit 47aaf79 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 Mar 2, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…rrowedfd-toowned, r=joshtriplett

Add documentation about `BorrowedFd::to_owned`.

Following up on rust-lang#88564, this adds documentation explaining why
`BorrowedFd::to_owned` returns another `BorrowedFd` rather than an
`OwnedFd`. And similar for `BorrowedHandle` and `BorrowedSocket`.

r? `@joshtriplett`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…rrowedfd-toowned, r=joshtriplett

Add documentation about `BorrowedFd::to_owned`.

Following up on rust-lang#88564, this adds documentation explaining why
`BorrowedFd::to_owned` returns another `BorrowedFd` rather than an
`OwnedFd`. And similar for `BorrowedHandle` and `BorrowedSocket`.

r? ``@joshtriplett``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…rrowedfd-toowned, r=joshtriplett

Add documentation about `BorrowedFd::to_owned`.

Following up on rust-lang#88564, this adds documentation explaining why
`BorrowedFd::to_owned` returns another `BorrowedFd` rather than an
`OwnedFd`. And similar for `BorrowedHandle` and `BorrowedSocket`.

r? ```@joshtriplett```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…rrowedfd-toowned, r=joshtriplett

Add documentation about `BorrowedFd::to_owned`.

Following up on rust-lang#88564, this adds documentation explaining why
`BorrowedFd::to_owned` returns another `BorrowedFd` rather than an
`OwnedFd`. And similar for `BorrowedHandle` and `BorrowedSocket`.

r? ````@joshtriplett````
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2022
Rollup of 9 pull requests

Successful merges:

 - rust-lang#92061 (update char signess for openbsd)
 - rust-lang#93072 (Compatible variants suggestion with desugaring)
 - rust-lang#93354 (Add documentation about `BorrowedFd::to_owned`.)
 - rust-lang#93663 (Rename `BorrowedFd::borrow_raw_fd` to `BorrowedFd::borrow_raw`.)
 - rust-lang#94375 (Adt copy suggestions)
 - rust-lang#94433 (Improve allowness of the unexpected_cfgs lint)
 - rust-lang#94499 (Documentation was missed when demoting Windows XP to no_std only)
 - rust-lang#94505 (Restore the local filter on mono item sorting)
 - rust-lang#94529 (Unused doc comments blocks)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bc1a890 into rust-lang:master Mar 3, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 3, 2022
@sunfishcode sunfishcode deleted the sunfishcode/document-borrowedfd-toowned branch March 3, 2022 13:55
sunfishcode added a commit to sunfishcode/io-lifetimes that referenced this pull request Mar 3, 2022
This corresponds to rust-lang/rust#93354, which was recently merged.
sunfishcode added a commit to sunfishcode/io-lifetimes that referenced this pull request Mar 3, 2022
This corresponds to rust-lang/rust#93354, which was recently merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants