Make `std::os::fd` public. #98368

sunfishcode · 2022-06-22T03:02:56Z

std::os::fd defines types like OwnedFd and RawFd and is common
between Unix and non-Unix platforms that share a basic file-descriptor
concept. Rust currently uses this internally to simplify its own code,
but it would be useful for external users in the same way, so make it
public.

This means that OwnedFd etc. will all appear in three places, for
example on unix platforms:

std::os::fd::OwnedFd
std::os::unix::io::OwnedFd
std::os::unix::prelude::OwnedFd

r? @joshtriplett

rust-highfive · 2022-06-22T03:02:59Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

joshtriplett · 2022-06-23T03:45:02Z

@rfcbot merge

dtolnay · 2022-06-29T19:33:14Z

Would it be possible to get a listing of everything that is being proposed to be stabilized at a path inside std::os::fd (and on what platforms) by this PR? Currently the summary just says "types like OwnedFd and RawFd" which isn't specific enough for me to approve the stabilization.

@rfcbot concern what are the contents

sunfishcode · 2022-06-29T19:49:31Z

@dtolnay The contents of the fd module in this PR are:

types: OwnedFd, BorrowedFd, RawFd
traits: AsFd, AsRawFd, IntoRawFd, FromRawFd

The platforms this PR adds the fd module to are Unix-family platforms (cfg(unix)) and WASI (cfg(target_os = "wasi")).

In the future, it's possible that fortanix_sgx and/or solid may want to add this module as well, as they also have similar types and traits.

dtolnay · 2022-06-29T20:28:24Z

Great!

@rfcbot resolve what are the contents
@rfcbot reviewed

BurntSushi · 2022-06-29T23:11:35Z

Is this insta-stable?

The other thing I'm confused about here is why things like RawFd are going in a platform independent module? They aren't used on Windows, for example, right?

dtolnay · 2022-06-29T23:17:07Z

I think the idea is that we already have std::os::unix, and std::os::wasi, and in the future perhaps std::os::fortanix_sgx or std::os::solid, all of which contain identical fd-related wrappers and traits. If someone is writing code for "anything with an fd", they can't easily do it. So std::os::fd serves as std::os::[anything with an fd].

BurntSushi · 2022-06-29T23:23:35Z

@dtolnay Ah I see thanks, I think that makes sense.

@sunfishcode Can we add that guidance to the docs of std::os::fd? That is, call out that RawFd (and company) aren't purely platform independent, but that they are used on so many platforms that it can be convenient to just import it from std::os::fd.

I do admit though that this feels a little strange to me to be honest. I like the practicality behind this, but I think you still have to gate std::os::fd imports with conditional compilation in order for it to be correct, right? If so, I'd expect that to be in the docs too, because the import doesn't really hint that you need any conditional compilation.

rustbot · 2022-06-30T02:08:59Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

sunfishcode · 2022-06-30T02:15:09Z

Is this insta-stable?

Yes, I guess it currently is. Would it be better here to use #[unstable(...)]? And I guess open a tracking issue?

@sunfishcode Can we add that guidance to the docs of std::os::fd? That is, call out that RawFd (and company) aren't purely platform independent, but that they are used on so many platforms that it can be convenient to just import it from std::os::fd.

I've clarified the comment to explicitly mention both Unix and WASI.

I do admit though that this feels a little strange to me to be honest. I like the practicality behind this, but I think you still have to gate std::os::fd imports with conditional compilation in order for it to be correct, right? If so, I'd expect that to be in the docs too, because the import doesn't really hint that you need any conditional compilation.

It does already have conditional compilation; the pub mod fd is gated on #[cfg(any(unix, target_os = "wasi", doc))]. Is that what you're referring to?

BurntSushi · 2022-06-30T02:21:18Z

It does already have conditional compilation; the pub mod fd is gated on #[cfg(any(unix, target_os = "wasi", doc))]. Is that what you're referring to?

Hmmm, I suppose, yes. I was more referring to consumers of this API. The import path doesn't really make it clear (like other import paths do) that conditional compilation is needed. It's just a departure from what we've done, I think.

The fact that pub mod fd is gated on any(unix, target_os = "wasi", doc) does make it better. But still overall feels a little weird to me.

BurntSushi · 2022-06-30T02:22:17Z

Yes, I guess it currently is. Would it be better here to use #[unstable(...)]? And I guess open a tracking issue?

I'm not sure of the underlying mechanism, but basically, if we don't have to do this as insta-stable, then we probably shouldn't. The typical insta-stable things are trait impls, and I believe that's because there just isn't any alternative.

sunfishcode · 2022-06-30T02:46:04Z

Yes, I guess it currently is. Would it be better here to use #[unstable(...)]? And I guess open a tracking issue?

I'm not sure of the underlying mechanism, but basically, if we don't have to do this as insta-stable, then we probably shouldn't. The typical insta-stable things are trait impls, and I believe that's because there just isn't any alternative.

I've now filed a tracking issue #98699, and updated the PR to use a #[unstable(...)] attribute. This was a little tricky, because the fd module is already used internally in a way that doesn't want it to be #[unstable(...)], so it appears this requires having two separate modules.

dtolnay · 2022-06-30T03:07:29Z

@rfcbot fcp cancel

rfcbot · 2022-06-30T03:07:31Z

@dtolnay proposal cancelled.

sunfishcode · 2022-09-27T23:56:50Z

Fixed the WIndows build issue.

r? @joshtriplett

joshtriplett · 2022-09-28T07:53:31Z

@bors r+ rollup

bors · 2022-09-28T07:53:33Z

📌 Commit a7f3ba9 has been approved by joshtriplett

It is now in the queue for this repository.

…r=joshtriplett Make `std::os::fd` public. `std::os::fd` defines types like `OwnedFd` and `RawFd` and is common between Unix and non-Unix platforms that share a basic file-descriptor concept. Rust currently uses this internally to simplify its own code, but it would be useful for external users in the same way, so make it public. This means that `OwnedFd` etc. will all appear in three places, for example on unix platforms: - `std::os::fd::OwnedFd` - `std::os::unix::io::OwnedFd` - `std::os::unix::prelude::OwnedFd` r? `@joshtriplett`

…r=joshtriplett Make `std::os::fd` public. `std::os::fd` defines types like `OwnedFd` and `RawFd` and is common between Unix and non-Unix platforms that share a basic file-descriptor concept. Rust currently uses this internally to simplify its own code, but it would be useful for external users in the same way, so make it public. This means that `OwnedFd` etc. will all appear in three places, for example on unix platforms: - `std::os::fd::OwnedFd` - `std::os::unix::io::OwnedFd` - `std::os::unix::prelude::OwnedFd` r? ``@joshtriplett``

…r=joshtriplett Make `std::os::fd` public. `std::os::fd` defines types like `OwnedFd` and `RawFd` and is common between Unix and non-Unix platforms that share a basic file-descriptor concept. Rust currently uses this internally to simplify its own code, but it would be useful for external users in the same way, so make it public. This means that `OwnedFd` etc. will all appear in three places, for example on unix platforms: - `std::os::fd::OwnedFd` - `std::os::unix::io::OwnedFd` - `std::os::unix::prelude::OwnedFd` r? ```@joshtriplett```

…r=joshtriplett Make `std::os::fd` public. `std::os::fd` defines types like `OwnedFd` and `RawFd` and is common between Unix and non-Unix platforms that share a basic file-descriptor concept. Rust currently uses this internally to simplify its own code, but it would be useful for external users in the same way, so make it public. This means that `OwnedFd` etc. will all appear in three places, for example on unix platforms: - `std::os::fd::OwnedFd` - `std::os::unix::io::OwnedFd` - `std::os::unix::prelude::OwnedFd` r? ````@joshtriplett````

Rollup of 8 pull requests Successful merges: - rust-lang#98368 (Make `std::os::fd` public.) - rust-lang#102085 (Code refactoring smart_resolve_report_errors) - rust-lang#102351 (Improve E0585 help) - rust-lang#102368 (Add a niche to `Duration`, unix `SystemTime`, and non-apple `Instant`) - rust-lang#102393 (Add regression test for issue 94923) - rust-lang#102399 (Account for use of index-based lifetime names in print of binder) - rust-lang#102416 (remove FIXME, improve documentation) - rust-lang#102433 (env::temp_dir: fix a typo) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

library/std/src/os/fd/mod.rs

@bjorn3

As @bjorn3 pointed out [here], I used the wrong stability attribute in rust-lang#98368 when making `std::os::fd` public. I set it to Rust 1.63, which was when io-safety was stabilized, but it should be Rust 1.66, which was when `std::os::fd` was stabilized. [here]: rust-lang#98368 (comment)

…stable-version, r=m-ou-se Fix the stability attributes for `std::os::fd`. As `@bjorn3` pointed out [here], I used the wrong stability attribute in rust-lang#98368 when making `std::os::fd` public. I set it to Rust 1.63, which was when io-safety was stabilized, but it should be Rust 1.66, which was when `std::os::fd` was stabilized. [here]: rust-lang#98368 (comment)

@bjorn3

As @bjorn3 pointed out [here], I used the wrong stability attribute in #98368 when making `std::os::fd` public. I set it to Rust 1.63, which was when io-safety was stabilized, but it should be Rust 1.66, which was when `std::os::fd` was stabilized. [here]: rust-lang/rust#98368 (comment)

…rsion, r=m-ou-se Fix the stability attributes for `std::os::fd`. As `@bjorn3` pointed out [here], I used the wrong stability attribute in #98368 when making `std::os::fd` public. I set it to Rust 1.63, which was when io-safety was stabilized, but it should be Rust 1.66, which was when `std::os::fd` was stabilized. [here]: rust-lang/rust#98368 (comment)

rust-highfive assigned joshtriplett Jun 22, 2022

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 22, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 22, 2022

This comment was marked as outdated.

Sign in to view

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jun 23, 2022

sunfishcode mentioned this pull request Jun 30, 2022

Tracking Issue for std::os::fd #98699

Closed

3 tasks

rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jun 30, 2022

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 1, 2022

Fix compilation of the doc tests on Windows.

a7f3ba9

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 28, 2022

Dylan-DPC mentioned this pull request Sep 28, 2022

Rollup of 6 pull requests #102407

Closed

Dylan-DPC mentioned this pull request Sep 28, 2022

Rollup of 6 pull requests #102413

Closed

JohnTitor mentioned this pull request Sep 29, 2022

Rollup of 11 pull requests #102448

Closed

JohnTitor mentioned this pull request Sep 29, 2022

Rollup of 10 pull requests #102449

Closed

JohnTitor mentioned this pull request Sep 29, 2022

Rollup of 8 pull requests #102450

Merged

bors merged commit 7cd4780 into rust-lang:master Sep 29, 2022

rustbot added this to the 1.66.0 milestone Sep 29, 2022

bjorn3 reviewed Jan 6, 2023

View reviewed changes

library/std/src/os/fd/mod.rs Show resolved Hide resolved

sunfishcode deleted the sunfishcode/std-os-fd branch January 13, 2023 00:48

sunfishcode mentioned this pull request Jan 14, 2023

Fix the stability attributes for std::os::fd. #106867

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `std::os::fd` public. #98368

Make `std::os::fd` public. #98368

sunfishcode commented Jun 22, 2022

rust-highfive commented Jun 22, 2022

joshtriplett commented Jun 23, 2022

This comment was marked as outdated.

dtolnay commented Jun 29, 2022

sunfishcode commented Jun 29, 2022

dtolnay commented Jun 29, 2022

BurntSushi commented Jun 29, 2022

dtolnay commented Jun 29, 2022

BurntSushi commented Jun 29, 2022

rustbot commented Jun 30, 2022

sunfishcode commented Jun 30, 2022

BurntSushi commented Jun 30, 2022

BurntSushi commented Jun 30, 2022

sunfishcode commented Jun 30, 2022

dtolnay commented Jun 30, 2022

rfcbot commented Jun 30, 2022

sunfishcode commented Sep 27, 2022

joshtriplett commented Sep 28, 2022

bors commented Sep 28, 2022

Make std::os::fd public. #98368

Make std::os::fd public. #98368

Conversation

sunfishcode commented Jun 22, 2022

rust-highfive commented Jun 22, 2022

joshtriplett commented Jun 23, 2022

This comment was marked as outdated.

dtolnay commented Jun 29, 2022

sunfishcode commented Jun 29, 2022

dtolnay commented Jun 29, 2022

BurntSushi commented Jun 29, 2022

dtolnay commented Jun 29, 2022

BurntSushi commented Jun 29, 2022

rustbot commented Jun 30, 2022

sunfishcode commented Jun 30, 2022

BurntSushi commented Jun 30, 2022

BurntSushi commented Jun 30, 2022

sunfishcode commented Jun 30, 2022

dtolnay commented Jun 30, 2022

rfcbot commented Jun 30, 2022

sunfishcode commented Sep 27, 2022

joshtriplett commented Sep 28, 2022

bors commented Sep 28, 2022

Make `std::os::fd` public. #98368

Make `std::os::fd` public. #98368