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

static_mut_refs: Automatic edition migration #123061

Closed
ehuss opened this issue Mar 25, 2024 · 6 comments
Closed

static_mut_refs: Automatic edition migration #123061

ehuss opened this issue Mar 25, 2024 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2024 Area: The 2024 edition T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor

ehuss commented Mar 25, 2024

#117556 implemented the static_mut_refs change for the 2024 Edition. However, the migration code is marked as maybe-incorrect, which prevents automatic migration. Is it possible to have an automatic migration for this change? If the addr_of! change is unlikely to work (see #123059), what is the scope of manual migration? How many crates will be affected by this change?

If this has a very widespread impact, we should be very certain that we want to move forward without that migration.

@ehuss ehuss added A-diagnostics Area: Messages for errors, warnings, and lints T-lang Relevant to the language team, which will review and decide on the PR/issue. A-edition-2024 Area: The 2024 edition labels Mar 25, 2024
@traviscross
Copy link
Contributor

@traviscross
Copy link
Contributor

traviscross commented Mar 25, 2024

Here's what @scottmcm earlier had to say about this:

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.

@RalfJung
Copy link
Member

Yeah, this is a case of "please use this opportunity to carefully review your code, based on today's understanding of how references work". There's no sensible automatic migration.

@est31
Copy link
Member

est31 commented Mar 26, 2024

Yeah indeed edition changes are required to have automatic migrations implemented, but this is not a change where such automatic migration is easy to do (unlike say Trait to dyn Trait). It's similar to discussions about deprecating other UB-prone code like forbidding mem::uninitialized in new editions or such.

Maybe that policy should be evolved into: if the API/pattern is UB-prone and easy to misuse, it is okay to not offer automatic edition migrations.

@ryanavella
Copy link

Forgive me if this has been suggested before, but is there room for a --allow-unsound flag for cargo fix?

I'm imagining that something like this:

static mut X: i32 = 0;
let _y = unsafe { &mut X };

Could be transformed into this:

static mut X: i32 = 0;
let _y = unsafe {
    // TODO: this was rewritten by `cargo fix`, and needs to be manually audited.
    // See [https://github.com/rust-lang/rust/issues/114447] for more information.
    &mut *std::ptr::addr_of_mut!(X)
};

@traviscross
Copy link
Contributor

We're not necessarily going to add any specific migration lints here as we want people to carefully review any code that is affected by this. So let's go ahead and close this as we'll be willing to mark this as ready for Rust 2024 without any lints here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2024 Area: The 2024 edition T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants