Disallow reference to static mut and adding static_mut_ref lint

[obeis]

Closes #114447

r? @scottmcm

[est31]

I wonder if you could also implement the error part for edition 2024, similar to how it's done here. Asking because then it might not be done in a lint pass any more? E.g. maybe_lint_bare_trait is called during the hir::Ty -> rustc_middle::ty::Ty lowering.

[rustbot]

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

[est31]

Some additional points:

• could you add a test that ptr::addr_of_mut still works?
• could you add a test that uses dot notation so there is no explicit syntax including &? Maybe like static mut foo: [u8; 3] = [1, 2, 3]; let _ = FOO.len();?
• this seems like a good use for test revisions where in one revision you enable the lint, and in the other revision you enable edition 2024. then that can cut down on duplication.
• the messaging around the lint/error is that all uses are illegal, but it's only the references and mutable references that are. In fact, the lint/error could suggest adopting addr_of_mut.

[est31]

the messaging around the lint/error is that all uses are illegal, but it's only the references and mutable references that are.

Note that I'm not arguing here that we should never make all of static mut deprecated (maybe a good idea, maybe not, in any case it's right now it's too early because SyncUnsafeCell is still unstable). But there is an inconsistency between what the lint does and its name on one hand, and what its messaging is on the other.

[scottmcm]

Thanks for making a PR for this! Unfortunately, I'm not personally familiar with the lint system, so while I'm happy to look at the tests to see what it's doing (est31's suggestions in #117556 (comment) sound good), I'm not a good reviewer for it technically.

r? compiler

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[obeis]

cc @RalfJung because it was your idea and unsafe is your area

[est31]

@bors r=davidtwco

[bors]

📌 Commit a4a52d5 has been approved by davidtwco

It is now in the queue for this repository.

[bors]

⌛ Testing commit a4a52d5 with merge 5088709...

[bors]

💔 Test failed - checks-actions

[est31]

@bors r=davidtwco

[bors]

📌 Commit a8aa687 has been approved by davidtwco

It is now in the queue for this repository.

[bors]

⌛ Testing commit a8aa687 with merge 092c722...

[bors]

💥 Test timed out

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[fmease]

Hmm, let's retry that?
@bors retry

[ehuss]

Some questions about this:

• Is there an intended migration path for this, or is the user expected to rewrite their code if it is encountered?
• The current suggestion of adding addr_of_mut! does not compile. At a minimum, I would think it needs to be qualified with core::ptr::. Can that be fixed?
• Is there an explanation of why that is a "maybe incorrect" suggestion?
• Was there a crater run done to see how much of an impact this will have?
• Was there an RFC for this, or any public discussion about the lang team's decision? It seems unusual to make language changes without an RFC.

[scottmcm]

As mentioned in #114447 (comment), you can always change &BLAH to &*ptr::addr_of!(BLAH), which will compile, but still be just as UB-prone.

The problem with taking references to static mut is that it's most often insta-UB, so there's no local change that can fix that, and yes, larger rework will typically be needed.

---

For more reading, there's been discussion and slow movement for at least 5 years towards limiting static mut. See #53639 for example.