Make `BorrowedFd::borrow_raw` a const fn. #96232

sunfishcode · 2022-04-20T00:26:28Z

Making BorrowedFd::borrow_raw a const fn allows it to be used to
create a constant BorrowedFd<'static> holding constants such as
AT_FDCWD. This will allow rustix::fs::cwd to become a const fn.

For consistency, make similar changes to BorrowedHandle::borrow_raw
and BorrowedSocket::borrow_raw.

r? @joshtriplett

Making `BorrowedFd::borrow_raw` a const fn allows it to be used to create a constant `BorrowedFd<'static>` holding constants such as `AT_FDCWD`. This will allow [`rustix::fs::cwd`] to become a const fn. For consistency, make similar changes to `BorrowedHandle::borrow_raw` and `BorrowedSocket::borrow_raw`. [`rustix::fs::cwd`]: https://docs.rs/rustix/latest/rustix/fs/fn.cwd.html

rust-highfive · 2022-04-20T00:26:31Z

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 r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. 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-04-20T00:27:03Z

r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs

This corresponds to rust-lang/rust#96232. Also, update the documentation and remove the assert from `BorrowedHandle::borrow_raw` to match what's in std: `BorrowedHandle` can hold NULL pointers.

sunfishcode · 2022-05-10T17:28:41Z

I marked this T-libs-api because it changes changes BorrowedFd::borrow_raw, which is currently an unstable API function, to be a const fn.

As context, this #87074 is proposing to stabilize this API, though that can happen with or without this PR, since adding const to a function is a backwards-compatible change.

joshtriplett · 2022-05-10T18:22:34Z

Seems reasonable to me.

@bors r+

bors · 2022-05-10T18:22:35Z

📌 Commit 0a1ce82 has been approved by joshtriplett

…onst-fns, r=joshtriplett Make `BorrowedFd::borrow_raw` a const fn. Making `BorrowedFd::borrow_raw` a const fn allows it to be used to create a constant `BorrowedFd<'static>` holding constants such as `AT_FDCWD`. This will allow [`rustix::fs::cwd`] to become a const fn. For consistency, make similar changes to `BorrowedHandle::borrow_raw` and `BorrowedSocket::borrow_raw`. [`rustix::fs::cwd`]: https://docs.rs/rustix/latest/rustix/fs/fn.cwd.html r? `@joshtriplett`

klensy · 2022-05-10T19:44:39Z

library/std/src/os/windows/io/socket.rs

-    pub unsafe fn borrow_raw(socket: RawSocket) -> Self {
-        debug_assert_ne!(socket, c::INVALID_SOCKET as RawSocket);
+    pub const unsafe fn borrow_raw(socket: RawSocket) -> Self {
+        assert!(socket != c::INVALID_SOCKET as RawSocket);


was debug_assert_ne, but became assert, so will be called at not debug builds. Why this change?

This makes it consistent with the unix version of borrowed_fd, which does a plain assert. Which is there because the previous unix implementation code FileDesc::new used a plain assert.

Also, I think it makes sense; this is a new API, and we're expecting to migrate mature codebases to use it, so it makes sense to be extra careful that old and new code agree on when and where INVALID_SOCKET can appear. Also, OwnedSocket uses rustc_layout_scalar_valid_range_* layout tricks that assume that INVALID_SOCKET can't appear, so it could easily produce some dramatic UB if the invariant is violated.

Make sense.

This corresponds to rust-lang/rust#96232. Also, update the documentation and remove the assert from `BorrowedHandle::borrow_raw` to match what's in std: `BorrowedHandle` can hold NULL pointers.

bors · 2022-05-10T21:19:22Z

⌛ Testing commit 0a1ce82 with merge ecd4495...

bors · 2022-05-10T23:44:15Z

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing ecd4495 to master...

rust-timer · 2022-05-11T01:31:16Z

Finished benchmarking commit (ecd4495): comparison url.

Summary:

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regression found

	Regressions 😿 (primary)	Regressions 😿 (secondary)	Improvements 🎉 (primary)	Improvements 🎉 (secondary)	All 😿 🎉 (primary)
count¹	0	1	0	0	0
mean²	N/A	0.2%	N/A	N/A	N/A
max	N/A	0.2%	N/A	N/A	N/A

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

number of relevant changes ↩
the arithmetic mean of the percent change ↩

rust-highfive assigned joshtriplett Apr 20, 2022

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

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

rust-highfive assigned m-ou-se Apr 20, 2022

rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Apr 20, 2022

rust-highfive unassigned joshtriplett Apr 20, 2022

rustbot removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Apr 20, 2022

sunfishcode mentioned this pull request May 7, 2022

Make BorrowedFd::borrow_raw a const fn. sunfishcode/io-lifetimes#29

Merged

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 May 10, 2022

matthiaskrgr mentioned this pull request May 10, 2022

Rollup of 6 pull requests #96919

Closed

klensy reviewed May 10, 2022

View reviewed changes

bors added the merged-by-bors This PR was explicitly merged by bors. label May 10, 2022

bors merged commit ecd4495 into rust-lang:master May 10, 2022

rustbot added this to the 1.62.0 milestone May 10, 2022

bors mentioned this pull request May 11, 2022

Implement stabilization of #[feature(io_safety)]. #95118

Merged

sunfishcode deleted the sunfishcode/io-safety-const-fns branch May 11, 2022 03:58

sunfishcode mentioned this pull request May 25, 2022

Make views require dedicated unsafe marker traits. sunfishcode/io-lifetimes#32

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `BorrowedFd::borrow_raw` a const fn. #96232

Make `BorrowedFd::borrow_raw` a const fn. #96232

sunfishcode commented Apr 20, 2022

rust-highfive commented Apr 20, 2022

sunfishcode commented Apr 20, 2022

sunfishcode commented May 10, 2022

joshtriplett commented May 10, 2022

bors commented May 10, 2022

klensy May 10, 2022

sunfishcode May 10, 2022

klensy May 11, 2022

bors commented May 10, 2022

bors commented May 10, 2022

rust-timer commented May 11, 2022

Make BorrowedFd::borrow_raw a const fn. #96232

Make BorrowedFd::borrow_raw a const fn. #96232

Conversation

sunfishcode commented Apr 20, 2022

rust-highfive commented Apr 20, 2022

sunfishcode commented Apr 20, 2022

sunfishcode commented May 10, 2022

joshtriplett commented May 10, 2022

bors commented May 10, 2022

klensy May 10, 2022

Choose a reason for hiding this comment

sunfishcode May 10, 2022

Choose a reason for hiding this comment

klensy May 11, 2022

Choose a reason for hiding this comment

bors commented May 10, 2022

bors commented May 10, 2022

rust-timer commented May 11, 2022

Footnotes

Make `BorrowedFd::borrow_raw` a const fn. #96232

Make `BorrowedFd::borrow_raw` a const fn. #96232