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

Tracking issue for release notes of #125834: treat &raw (const|mut) UNSAFE_STATIC implied deref as safe #129578

Closed
2 of 4 tasks
rustbot opened this issue Aug 25, 2024 · 5 comments
Labels
I-release-nominated Nominated for the release team. relnotes Marks issues that should be documented in the release notes of the next release. relnotes-tracking-issue Marks issues tracking what text to put in release notes. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Milestone

Comments

@rustbot
Copy link
Collaborator

rustbot commented Aug 25, 2024

This issue tracks the release notes text for #125834.

  • Issue is nominated for the responsible team (and T-release nomination is removed).
  • Proposed text is drafted by team responsible for underlying change.
  • Issue is nominated for release team review of clarity for wider audience.
  • Release team includes text in release notes/blog posts.

Release notes text:

The section title will be de-duplicated by the release team with other release notes issues.
Prefer to use the standard titles from previous releases.
More than one section can be included if needed.

# Language
- [`addr_of(_mut)!` macros and the newly stabilized `&raw (const|mut)` are now safe to use with all static items](https://github.com/rust-lang/rust/pull/125834)

Release blog section (if any, leave blank if no section is expected):

@rustbot rustbot added I-release-nominated Nominated for the release team. relnotes Marks issues that should be documented in the release notes of the next release. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 25, 2024
@Mark-Simulacrum Mark-Simulacrum added I-compiler-nominated Nominated for discussion during a compiler team meeting. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. I-release-nominated Nominated for the release team. labels Aug 25, 2024
@saethlin saethlin added the T-release Relevant to the release subteam, which will review and decide on the PR/issue. label Aug 25, 2024
@workingjubilee
Copy link
Member

workingjubilee commented Aug 26, 2024

Hi. I feel my PR title was very "inside baseball" terminology, and what should go into the compatibility notes might be more like one of these:

  • &raw (const|mut) is now safe to use with an UNSAFE_STATIC operand
  • Taking the address of an UNSAFE_STATIC is now safe

Possible release blog text by author of PR if you wish to note it in the release blog (whether this is appropriate or not is between you and T-compiler imo). You may wish to work it in with the raw_ref_op stabilization text as otherwise there probably will be some conceptual duplication, since these are both hitting stable together. Also feel free to trim it down, the most important part is, IMO, the "hey sprinkle around #[allow(unused_unsafe)]" part.

Taking the address of an UNSAFE_STATIC is now safe

This code is now allowed:

static mut STATIC_MUT: Type = Type::new();
extern "C" {
    static EXTERN_STATIC: Type;
}
fn main() {
     let static_mut_ptr = std::ptr::addr_of_mut!(STATIC_MUT);
     let extern_static_ptr = &raw const EXTERN_STATIC;
}

In an expression context, STATIC_MUT and EXTERN_STATIC are place expressions. Previously, the compiler's safety checks were not aware that the raw ref operator did not actually affect the operand's place, treating it as a possible read or write to a pointer. No unsafety is actually present, however, as it just creates a pointer.

This may cause problems where some unsafe blocks are now reported as unused if you deny the unused_unsafe lint, but they are now only useful on older versions. Annotate these unsafe blocks with #[allow(unused_unsafe)] if you wish to support multiple versions of Rust, as in this example diff:

 static mut STATIC_MUT: Type = Type::new();
 fn main() {
+    #[allow(unused_unsafe)]
     let static_mut_ptr = unsafe { std::ptr::addr_of_mut!(STATIC_MUT) };
 }

A future version of Rust is expected to generalize this to other expressions which would be safe in this position, not just statics.

@Mark-Simulacrum Mark-Simulacrum added this to the 1.82.0 milestone Aug 27, 2024
@RalfJung
Copy link
Member

A future version of Rust is expected to generalize this to all instances of &raw const or &raw mut followed by a place expression, not just statics.

That's not right, &raw const (*ptr).field will have to remain unsafe.

Only &raw const *ptr can be made safe, and &raw const reference.field.

@wesleywiser
Copy link
Member

T-compiler reviewed during the weekly triage meeting. We decided to go with a modification of @workingjubilee's first suggestion, tweaking to highlight this also affects addr_of(_mut)! and attempting to remove the UNSAFE_STATIC reference since we don't think that shorthand is commonly understood by the wider Rust community.

I think if there's room in the blog post and T-release wants to include it, the above content elaborating on the changes would be a great addition. We've had some churn around statics recently so highlighting the correct way to write some of this code, especially since it uses newer library functionality, would be good.

Thank you @workingjubilee and @RalfJung for drafting that!

@wesleywiser wesleywiser added I-release-nominated Nominated for the release team. and removed I-compiler-nominated Nominated for discussion during a compiler team meeting. labels Aug 29, 2024
@workingjubilee
Copy link
Member

workingjubilee commented Aug 29, 2024

A future version of Rust is expected to generalize this to all instances of &raw const or &raw mut followed by a place expression, not just statics.

That's not right, &raw const (*ptr).field will have to remain unsafe.

Only &raw const *ptr can be made safe, and &raw const reference.field.

Ah okay! Then I think what I said there could instead be just

"A future version of Rust is expected to generalize this to other expressions where it's actually-safe, not just ones with statics as operands."

@workingjubilee
Copy link
Member

workingjubilee commented Aug 30, 2024

Also I should note that I only included the "we'll generalize this later" remark, fwiw, because "huh, wouldn't that apply to other expressions?" was like basically the third thing everyone said while considering the PR I made, and because compiler-errors already has the PR up. :^)

@Mark-Simulacrum Mark-Simulacrum added the relnotes-tracking-issue Marks issues tracking what text to put in release notes. label Sep 6, 2024
@Urgau Urgau added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-release Relevant to the release subteam, which will review and decide on the PR/issue. labels Oct 7, 2024
@cuviper cuviper closed this as completed Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-release-nominated Nominated for the release team. relnotes Marks issues that should be documented in the release notes of the next release. relnotes-tracking-issue Marks issues tracking what text to put in release notes. 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

8 participants