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

Improve wording of static_mut_ref #120964

Closed
RalfJung opened this issue Feb 12, 2024 · 6 comments · Fixed by #121034
Closed

Improve wording of static_mut_ref #120964

RalfJung opened this issue Feb 12, 2024 · 6 comments · Fixed by #121034
Assignees
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Feb 12, 2024

This lint currently prints

   = note: reference of mutable static is a hard error from 2024 edition
   = note: mutable statics can be written to by multiple threads: aliasing violations or data races will cause undefined behavior

The first line seems grammatically not entirely right to me. Shouldn't it say "in the 2024 edition"? And should it be "reference of" or "reference to"?

The second line doesn't quite capture the problem IMO. The issue is that the reference has lifetime 'static, so

  • a mutable reference supposedly lives forever, so creating more than one is very dangerous and they can accidentally be used in overlapping ways
  • a shared reference supposedly lives forever, so if there is ever also a mutable reference created that is very dangerous as they can accidentally be used in overlapping ways

Cc @obeis

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 12, 2024
@obeis
Copy link
Contributor

obeis commented Feb 12, 2024

@rustbot claim

@RalfJung
Copy link
Member Author

Also, the lint should probably be named static_mut_refs. Policy for lints (AFAIK) is for them to have a plural name.

@saethlin saethlin added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 13, 2024
@obeis
Copy link
Contributor

obeis commented Feb 13, 2024

Renaming the lint is not straightforward because of this PR #120676; I am not sure how it can be done.

@RalfJung
Copy link
Member Author

What does that PR have to do with it?

When renaming a lint, remember to register it as a renamed lint. (Though I'm not sure if that will help with whatever issue you were running into.)

@obeis
Copy link
Contributor

obeis commented Feb 13, 2024

I need to allow both lints static_mut_ref and static_mut_refs because Bootstrap will use version 1.77 beta, and static_mut_ref already exists, as I understand.

What do you mean by register it as a renamed lint?

@RalfJung
Copy link
Member Author

RalfJung commented Feb 13, 2024

I need to allow both lints static_mut_ref and static_mut_refs because Bootstrap will use version 1.77 beta, and static_mut_ref already exists, as I understand.

We have the bootstrap cfg-flag to handle such situations. So you can do something like

#[cfg_attr(bootstrap, allow(static_mut_ref))]
#[cfg_attr(not(bootstrap), allow(static_mut_refs))]

What do you mean by register it as a renamed lint?

// Register renamed and removed lints.
store.register_renamed("single_use_lifetime", "single_use_lifetimes");
store.register_renamed("elided_lifetime_in_path", "elided_lifetimes_in_paths");
store.register_renamed("bare_trait_object", "bare_trait_objects");
store.register_renamed("unstable_name_collision", "unstable_name_collisions");
store.register_renamed("unused_doc_comment", "unused_doc_comments");
store.register_renamed("async_idents", "keyword_idents");
store.register_renamed("exceeding_bitshifts", "arithmetic_overflow");
store.register_renamed("redundant_semicolon", "redundant_semicolons");
store.register_renamed("overlapping_patterns", "overlapping_range_endpoints");
store.register_renamed("disjoint_capture_migration", "rust_2021_incompatible_closure_captures");
store.register_renamed("or_patterns_back_compat", "rust_2021_incompatible_or_patterns");
store.register_renamed("non_fmt_panic", "non_fmt_panics");
store.register_renamed("unused_tuple_struct_fields", "dead_code");

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 15, 2024
oli-obk added a commit to oli-obk/rust that referenced this issue Feb 15, 2024
CKingX added a commit to CKingX/rust that referenced this issue Feb 18, 2024
@bors bors closed this as completed in bcb3545 Feb 18, 2024
GuillaumeGomez pushed a commit to GuillaumeGomez/rust that referenced this issue Feb 21, 2024
bjorn3 pushed a commit to bjorn3/rust that referenced this issue Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants