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

Rollup of 3 pull requests #127757

Merged
merged 31 commits into from
Jul 15, 2024
Merged

Rollup of 3 pull requests #127757

merged 31 commits into from
Jul 15, 2024

Conversation

workingjubilee
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

workingjubilee and others added 30 commits July 14, 2024 16:44
This provides a list of locations to hunt down issues in.
We stick to C types in for socket and address as these are at least nominally BSD-ish and they're used outside of pal/windows in general *nix code
As with USHORT, keep using C types for BSD socket APIs.
…bilee

Windows: Remove some unnecessary type aliases

Back in the olden days, C did not have fixed-width types so these type aliases were at least potentially useful. Nowadays, and especially in Rust, we don't need the aliases and they don't help with anything. Notably the windows bindings we use also don't bother with the aliases. And even when we have used aliases they're often only used once then forgotten about.

The only one that gives me pause is `DWORD` because it's used a fair bit. But it's still used inconsistently and we implicitly assume it's a `u32` anyway (e.g. `as` casting from an `i32`).
…td, r=jhpratt

std: `#![deny(unsafe_op_in_unsafe_fn)]` in platform-independent code

This applies the `unsafe_op_in_unsafe_fn` lint in all places in std that _do not have platform-specific cfg in their code_. For all such places, the lint remains allowed, because they need further work to address the relevant concerns. This list includes:

- `std::backtrace_rs` (internal-only)
- `std::sys` (internal-only)
- `std::os`

Notably this eliminates all "unwrapped" unsafe operations in `std::io` and `std::sync`, which will make them much more auditable in the future. Such has *also* been left for future work. While I made a few safety comments along the way on interfaces I have grown sufficiently familiar with, in most cases I had no context, nor particular confidence the unsafety was correct.

In the cases where I was able to determine the unsafety was correct without having prior context, it was obviously redundant. For example, an unsafe function calling another unsafe function that has the exact same contract, forwarding its caller's requirements just as it forwards its actual call.
…workingjubilee

Make os/windows and pal/windows default to `#![deny(unsafe_op_in_unsafe_fn)]`

This is to prevent regressions in modules that currently pass. I did also fix up a few trivial places where the module contained only one or two simple wrappers. In more complex cases we should try to ensure the `unsafe` blocks are appropriately scoped and have any appropriate safety comments.

This does not fix the windows bits of rust-lang#127747 but it should help prevent regressions until that is done and also make it more obvious specifically which modules need attention.
@rustbot rustbot added O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jul 15, 2024
@workingjubilee
Copy link
Member Author

@bors r+ rollup=never p=3

@bors
Copy link
Contributor

bors commented Jul 15, 2024

📌 Commit 476d399 has been approved by workingjubilee

It is now in the queue for this repository.

@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 15, 2024
@bors
Copy link
Contributor

bors commented Jul 15, 2024

⌛ Testing commit 476d399 with merge d3dd34a...

@bors
Copy link
Contributor

bors commented Jul 15, 2024

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing d3dd34a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 15, 2024
@bors bors merged commit d3dd34a into rust-lang:master Jul 15, 2024
7 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 15, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#127712 Windows: Remove some unnecessary type aliases 05b6f7b04458d108c955eb86977bfdc73fe8e2a2 (link)
#127744 std: #![deny(unsafe_op_in_unsafe_fn)] in platform-indepen… d5d9a1e1f85b2de3b5bbb3fcc85c57b6e582952a (link)
#127750 Make os/windows and pal/windows default to `#![deny(unsafe_… c66f127c5307f48ef9d005420eb6801032ca85e3 (link)

previous master: adeb79d3f5

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d3dd34a): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.7% [0.7%, 0.7%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.7% [-1.2%, -0.2%] 2
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 5.5%, secondary 0.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
13.1% [13.1%, 13.1%] 1
Regressions ❌
(secondary)
5.4% [4.7%, 6.1%] 2
Improvements ✅
(primary)
-2.1% [-2.1%, -2.1%] 1
Improvements ✅
(secondary)
-5.3% [-5.5%, -5.1%] 2
All ❌✅ (primary) 5.5% [-2.1%, 13.1%] 2

Cycles

Results (secondary 2.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.7% [2.0%, 4.2%] 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

Results (primary -0.0%, secondary -0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.1% [0.1%, 0.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.0% [-0.0%, -0.0%] 7
Improvements ✅
(secondary)
-0.0% [-0.0%, -0.0%] 34
All ❌✅ (primary) -0.0% [-0.0%, 0.1%] 8

Bootstrap: 700.232s -> 700.525s (0.04%)
Artifact size: 328.70 MiB -> 328.65 MiB (-0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. O-windows Operating system: Windows rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants