Get rid of some `#![allow(static_mut_refs)]` #121303

GrigorenkoPV · 2024-02-19T15:58:12Z

No description provided.

rustbot · 2024-02-19T15:58:19Z

rustbot has assigned @ChrisDenton.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-02-19T15:58:21Z

The Miri subtree was changed

cc @rust-lang/miri

oli-obk · 2024-02-19T16:23:30Z

r=me on the compiler changes

r? @RalfJung on the miri test changes. They lgtm, but maybe there are subtle interactions that should have been handled differently

RalfJung · 2024-02-22T20:32:17Z

Miri changes LGTM, thanks!
@bors r=oli-obk,RalfJung

bors · 2024-02-22T20:32:20Z

📌 Commit 859c122 has been approved by oli-obk,RalfJung

It is now in the queue for this repository.

bors · 2024-02-23T03:37:42Z

⌛ Testing commit 859c122 with merge daa4264...

…obk,RalfJung Get rid of some `#![allow(static_mut_refs)]`

bors · 2024-02-23T03:43:40Z

💔 Test failed - checks-actions

RalfJung · 2024-02-23T07:26:08Z

Some of the allow that you removed do seem to be necessary on Windows builds.

GrigorenkoPV · 2024-02-23T15:13:16Z

Hopefully this is fixed now.

I have also rebased on top of master and squashed all the changes in one commit.

Will probably look into the ways of setting up a win-msvc toolchain on linux, with some terrible wine hacking, because the experience I have just had with a Windows VM was not very pleasant. But at least more responsive than a CI failure.

RalfJung · 2024-02-23T16:39:14Z

The stuff @saethlin did for mir-opt tests should let us do check-builds for MSVC targets on a Linux host, without any Windows tools around.

RalfJung · 2024-02-23T16:40:58Z

library/panic_unwind/src/seh.rs

@@ -220,7 +220,7 @@ extern "C" {
 // This is fine since the MSVC runtime uses string comparison on the type name
 // to match TypeDescriptors rather than pointer equality.
 static mut TYPE_DESCRIPTOR: _TypeDescriptor = _TypeDescriptor {
-    pVFTable: unsafe { &TYPE_INFO_VTABLE } as *const _ as *const _,


Strange, did this one not trigger the warning? It doesn't seem to be inside any of the allow.

Yes, it said something about external statics iirc

(If you mean why I left the unsafe block in place)

Strange, did this one not trigger the warning? It doesn't seem to be inside any of the allow.

Ah, I guess this is because it is not a mut static:
https://github.com/rust-lang/rust/pull/121303/files#diff-1308e363d41cbb1f3f93d560e000263610240cc456596918347038d303dac7b6L214

Ah I see, you just made them all use addr_of for good measure. Makes sense!

Ah I see, you just made them all use addr_of for good measure. Makes sense!

I think there are many more places where & ... as *const ... is used. Would it make sense to implement a lint (probably allow-by-default) suggesting to replace those with addr_of!? Or maybe at least for me to go through the libs and change those manually?

UPD: Apparently (and honestly unsurprisingly) there is already a clippy lint for that: borrow_as_ptr.

Yeah those should all be addr_of ideally.

RalfJung · 2024-02-23T16:59:05Z

library/std/src/panicking.rs

@@ -337,9 +337,6 @@ pub mod panic_count {
 #[doc(hidden)]
 #[cfg(not(feature = "panic_immediate_abort"))]
 #[unstable(feature = "update_panic_count", issue = "none")]
-// FIXME: Use `SyncUnsafeCell` instead of allowing `static_mut_refs` lint
-#[cfg_attr(bootstrap, allow(static_mut_ref))]
-#[cfg_attr(not(bootstrap), allow(static_mut_refs))]


Looks like no fix was needed for these. This does make me wonder why they were ever added... 🤷

RalfJung · 2024-02-23T16:59:26Z

@bors r=oli-obk,RalfJung rollup=iffy

bors · 2024-02-23T16:59:28Z

📌 Commit 58c8c08 has been approved by oli-obk,RalfJung

It is now in the queue for this repository.

saethlin · 2024-02-23T17:24:35Z

The stuff @saethlin did for mir-opt tests should let us do check-builds for MSVC targets on a Linux host, without any Windows tools around.

To be clear, x check library --target=aarch64-pc-windows-msvc just works. You should be able to set various targets for x check without worrying about what host you're on, though this is of course not tested in all the various ways that bootstrap can be used. (actually doing cross-builds still doesn't work, which is what I often end up needing 🙃 )

RalfJung · 2024-02-23T17:56:50Z

Ah, great. :)

We should add that to PR CI for some targets... currently e.g. macOS is only covered by the Miri tests I think.

EDIT: filed an issue: #121519

bors · 2024-02-23T18:52:50Z

⌛ Testing commit 58c8c08 with merge 2dbd623...

bors · 2024-02-23T20:52:52Z

☀️ Test successful - checks-actions
Approved by: oli-obk,RalfJung
Pushing 2dbd623 to master...

rust-timer · 2024-02-23T22:07:53Z

Finished benchmarking commit (2dbd623): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

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)	-	-	0
Improvements ✅ (primary)	-3.4%	[-3.4%, -3.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.4%	[-3.4%, -3.4%]	1

Cycles

Results

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)	5.3%	[5.2%, 5.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 650.563s -> 650.231s (-0.05%)
Artifact size: 310.99 MiB -> 310.96 MiB (-0.01%)

Use `addr_of!` As per rust-lang#121303 (comment)

Rollup merge of rust-lang#121556 - GrigorenkoPV:addr_of, r=Nilstrieb Use `addr_of!` As per rust-lang#121303 (comment)

rustbot assigned ChrisDenton Feb 19, 2024

rustbot added 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. labels Feb 19, 2024

rustbot assigned RalfJung and unassigned ChrisDenton Feb 19, 2024

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 Feb 22, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2024

Auto merge of rust-lang#121303 - GrigorenkoPV:static_mut_refs, r=oli-…

daa4264

…obk,RalfJung Get rid of some `#![allow(static_mut_refs)]`

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 23, 2024

Get rid of some #[allow(static_mut_refs)]

58c8c08

GrigorenkoPV force-pushed the static_mut_refs branch from 859c122 to 58c8c08 Compare February 23, 2024 15:06

RalfJung reviewed Feb 23, 2024

View reviewed changes

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 Feb 23, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 23, 2024

bors merged commit 2dbd623 into rust-lang:master Feb 23, 2024
12 checks passed

rustbot added this to the 1.78.0 milestone Feb 23, 2024

GrigorenkoPV deleted the static_mut_refs branch February 23, 2024 23:58

GrigorenkoPV mentioned this pull request Feb 24, 2024

Use addr_of! #121556

Merged

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 24, 2024

Rollup merge of rust-lang#121556 - GrigorenkoPV:addr_of, r=Nilstrieb

0acf061

Use `addr_of!` As per rust-lang#121303 (comment)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 24, 2024

Rollup merge of rust-lang#121556 - GrigorenkoPV:addr_of, r=Nilstrieb

4c40153

Use `addr_of!` As per rust-lang#121303 (comment)

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 25, 2024

Unrolled build for rust-lang#121556

9c66df7

Rollup merge of rust-lang#121556 - GrigorenkoPV:addr_of, r=Nilstrieb Use `addr_of!` As per rust-lang#121303 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get rid of some `#![allow(static_mut_refs)]` #121303

Get rid of some `#![allow(static_mut_refs)]` #121303

GrigorenkoPV commented Feb 19, 2024

rustbot commented Feb 19, 2024

rustbot commented Feb 19, 2024

oli-obk commented Feb 19, 2024

RalfJung commented Feb 22, 2024

bors commented Feb 22, 2024

bors commented Feb 23, 2024

This comment has been minimized.

bors commented Feb 23, 2024

RalfJung commented Feb 23, 2024 •

edited

Loading

GrigorenkoPV commented Feb 23, 2024

RalfJung commented Feb 23, 2024

RalfJung Feb 23, 2024 •

edited

Loading

GrigorenkoPV Feb 23, 2024 •

edited

Loading

GrigorenkoPV Feb 23, 2024

RalfJung Feb 23, 2024

GrigorenkoPV Feb 23, 2024 •

edited

Loading

RalfJung Feb 23, 2024

RalfJung Feb 23, 2024

RalfJung commented Feb 23, 2024

bors commented Feb 23, 2024

saethlin commented Feb 23, 2024 •

edited

Loading

RalfJung commented Feb 23, 2024 •

edited

Loading

bors commented Feb 23, 2024

bors commented Feb 23, 2024

rust-timer commented Feb 23, 2024

Get rid of some #![allow(static_mut_refs)] #121303

Get rid of some #![allow(static_mut_refs)] #121303

Conversation

GrigorenkoPV commented Feb 19, 2024

rustbot commented Feb 19, 2024

rustbot commented Feb 19, 2024

oli-obk commented Feb 19, 2024

RalfJung commented Feb 22, 2024

bors commented Feb 22, 2024

bors commented Feb 23, 2024

This comment has been minimized.

bors commented Feb 23, 2024

RalfJung commented Feb 23, 2024 • edited Loading

GrigorenkoPV commented Feb 23, 2024

RalfJung commented Feb 23, 2024

RalfJung Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

GrigorenkoPV Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GrigorenkoPV Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung commented Feb 23, 2024

bors commented Feb 23, 2024

saethlin commented Feb 23, 2024 • edited Loading

RalfJung commented Feb 23, 2024 • edited Loading

bors commented Feb 23, 2024

bors commented Feb 23, 2024

rust-timer commented Feb 23, 2024

Overall result: no relevant changes - no action needed

Get rid of some `#![allow(static_mut_refs)]` #121303

Get rid of some `#![allow(static_mut_refs)]` #121303

RalfJung commented Feb 23, 2024 •

edited

Loading

RalfJung Feb 23, 2024 •

edited

Loading

GrigorenkoPV Feb 23, 2024 •

edited

Loading

GrigorenkoPV Feb 23, 2024 •

edited

Loading

saethlin commented Feb 23, 2024 •

edited

Loading

RalfJung commented Feb 23, 2024 •

edited

Loading