Skip to content

Missing parenthesis in static_mut_refs diagnostic suggestion #131977

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
CharlyCst opened this issue Oct 20, 2024 · 3 comments · Fixed by #132095 or #132756
Closed

Missing parenthesis in static_mut_refs diagnostic suggestion #131977

CharlyCst opened this issue Oct 20, 2024 · 3 comments · Fixed by #132095 or #132756
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@CharlyCst
Copy link

CharlyCst commented Oct 20, 2024

Code

static mut TEST: usize = 0;

fn main() {
    let _ = unsafe { (&TEST) as *const usize };
}

Current output

   Compiling playground v0.0.1 (/playground)
warning: creating a shared reference to mutable static is discouraged
 --> src/main.rs:4:22
  |
4 |     let _ = unsafe { (&TEST) as *const usize };
  |                      ^^^^^^^ shared reference to mutable static
  |
  = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
  = note: shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives
  = note: `#[warn(static_mut_refs)]` on by default
help: use `&raw const` instead to create a raw pointer
  |
4 |     let _ = unsafe { &raw const TEST) as *const usize };
  |                      ~~~~~~~~~~

warning: `playground` (bin "playground") generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.73s
     Running `target/debug/playground`

Desired output

   Compiling playground v0.0.1 (/playground)
warning: creating a shared reference to mutable static is discouraged
 --> src/main.rs:4:22
  |
4 |     let _ = unsafe { (&TEST) as *const usize };
  |                      ^^^^^^^ shared reference to mutable static
  |
  = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
  = note: shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives
  = note: `#[warn(static_mut_refs)]` on by default
help: use `&raw const` instead to create a raw pointer
  |
4 |     let _ = unsafe { (&raw const TEST) as *const usize };
  |                       ~~~~~~~~~~

warning: `playground` (bin "playground") generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.73s
     Running `target/debug/playground`

Rationale and extra context

The suggestion removes the left parenthesis, but keeps the right parenthesis. Of course the proposed change does not compile.
Original code: (&TEST) suggestion: &raw const TEST)

Other cases

No response

Rust Version

rustc 1.84.0-nightly (da93539 2024-10-19)
binary: rustc
commit-hash: da93539
commit-date: 2024-10-19
host: aarch64-apple-darwin
release: 1.84.0-nightly
LLVM version: 19.1.1

Anything else?

Link to rust playground

@CharlyCst CharlyCst added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 20, 2024
@jieyouxu jieyouxu added A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Oct 21, 2024
@indegrowl
Copy link

Pretty straightforward! I'll work on it. It'll be my first PR.

@gechelberger
Copy link

This is actually more complicated than I assumed.

The offending code is operating on the HIR where parens have already been elided into the token tree heirarchy.

The suggested replacement of &raw const or &raw mut is dropped into the span err_span.with_hi(ex.span.lo()) from here.

The cleanest way to solve it would be to narrow the referenced span when lowering from the AST to the HIR so that the 'hir::Expr' node's Span doesn't point to or include the parens - only the contents of them - since the HIR isn't supposed to know about parens anyways.

I doubt that this would change the correctness of other lints and errors operating on the HIR, but it almost certainly breaks a lot of the ui compiletests which would then have different spans highlighted in their stderr contents.

Are there other lints/errors that have run into similar issues to reference?

@gechelberger
Copy link

For reference, I commented out this line which replaces the HIR Span with the AST Paren Span to get a sense for how significant of an effect it would have:

It is definitely not the solution on its own.

  • It does solve this issue.
  • It fails 95 cases in tests/ui
    • Some of these are still semantically correct with the only differences being the locations of the highlighted error spans.
    • It causes semantic problems for calls
      • method calls: (a.unwrap)()
      • closures: (|| {})(|| { let b = 1; })
    • It causes a reciprocal syntax problem with expressions that directly operate outside the parens contents
      • indexing: (a as [Foo; 3]).0 => (a as [Foo; 3][0] instead of (a as [Foo; 3])[0]
    • possibly other issues

ui-tests.txt

@jieyouxu jieyouxu added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Oct 24, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 7, 2024
…iser

Fix rust-lang#131977 parens mangled in shared mut static lint suggestion

Resolves rust-lang#131977 for static mut references after discussion with
Esteban & Jieyou on [t-compiler/help](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/linting.20with.20parens.20in.20the.20HIR).

This doesn't do anything to change the underlying issue if there are other expressions that generate lint suggestions which need to be applied within parentheses.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 7, 2024
…iser

Fix rust-lang#131977 parens mangled in shared mut static lint suggestion

Resolves rust-lang#131977 for static mut references after discussion with
Esteban & Jieyou on [t-compiler/help](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/linting.20with.20parens.20in.20the.20HIR).

This doesn't do anything to change the underlying issue if there are other expressions that generate lint suggestions which need to be applied within parentheses.
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 7, 2024
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#130586 (Set "symbol name" in raw-dylib import libraries to the decorated name)
 - rust-lang#131913 (Add `{ignore,needs}-{rustc,std}-debug-assertions` directive support)
 - rust-lang#132095 (Fix rust-lang#131977 parens mangled in shared mut static lint suggestion)
 - rust-lang#132131 ([StableMIR] API to retrieve definitions from crates)
 - rust-lang#132696 (Compile `test_num_f128` conditionally on `reliable_f128_math` config)
 - rust-lang#132738 (Initialize channel `Block`s directly on the heap)
 - rust-lang#132739 (Fix `librustdoc/scrape_examples.rs` formatting)
 - rust-lang#132740 (Update test for LLVM 20's new vector splat syntax)

r? `@ghost`
`@rustbot` modify labels: rollup
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Nov 8, 2024
…iser

Fix rust-lang#131977 parens mangled in shared mut static lint suggestion

Resolves rust-lang#131977 for static mut references after discussion with
Esteban & Jieyou on [t-compiler/help](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/linting.20with.20parens.20in.20the.20HIR).

This doesn't do anything to change the underlying issue if there are other expressions that generate lint suggestions which need to be applied within parentheses.
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Nov 8, 2024
…iser

Fix rust-lang#131977 parens mangled in shared mut static lint suggestion

Resolves rust-lang#131977 for static mut references after discussion with
Esteban & Jieyou on [t-compiler/help](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/linting.20with.20parens.20in.20the.20HIR).

This doesn't do anything to change the underlying issue if there are other expressions that generate lint suggestions which need to be applied within parentheses.
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 8, 2024
…kingjubilee

Rollup of 10 pull requests

Successful merges:

 - rust-lang#130586 (Set "symbol name" in raw-dylib import libraries to the decorated name)
 - rust-lang#131913 (Add `{ignore,needs}-{rustc,std}-debug-assertions` directive support)
 - rust-lang#132095 (Fix rust-lang#131977 parens mangled in shared mut static lint suggestion)
 - rust-lang#132131 ([StableMIR] API to retrieve definitions from crates)
 - rust-lang#132639 (core: move intrinsics.rs into intrinsics folder)
 - rust-lang#132696 (Compile `test_num_f128` conditionally on `reliable_f128_math` config)
 - rust-lang#132737 (bootstrap: Print better message if lock pid isn't available)
 - rust-lang#132739 (Fix `librustdoc/scrape_examples.rs` formatting)
 - rust-lang#132740 (Update test for LLVM 20's new vector splat syntax)
 - rust-lang#132741 (Update mips64 data layout to match LLVM 20 change)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in cc0ec04 Nov 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 8, 2024
Rollup merge of rust-lang#132095 - gechelberger:fix-131977, r=wesleywiser

Fix rust-lang#131977 parens mangled in shared mut static lint suggestion

Resolves rust-lang#131977 for static mut references after discussion with
Esteban & Jieyou on [t-compiler/help](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/linting.20with.20parens.20in.20the.20HIR).

This doesn't do anything to change the underlying issue if there are other expressions that generate lint suggestions which need to be applied within parentheses.
mati865 pushed a commit to mati865/rust that referenced this issue Nov 12, 2024
…iser

Fix rust-lang#131977 parens mangled in shared mut static lint suggestion

Resolves rust-lang#131977 for static mut references after discussion with
Esteban & Jieyou on [t-compiler/help](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/linting.20with.20parens.20in.20the.20HIR).

This doesn't do anything to change the underlying issue if there are other expressions that generate lint suggestions which need to be applied within parentheses.
mati865 pushed a commit to mati865/rust that referenced this issue Nov 12, 2024
…kingjubilee

Rollup of 10 pull requests

Successful merges:

 - rust-lang#130586 (Set "symbol name" in raw-dylib import libraries to the decorated name)
 - rust-lang#131913 (Add `{ignore,needs}-{rustc,std}-debug-assertions` directive support)
 - rust-lang#132095 (Fix rust-lang#131977 parens mangled in shared mut static lint suggestion)
 - rust-lang#132131 ([StableMIR] API to retrieve definitions from crates)
 - rust-lang#132639 (core: move intrinsics.rs into intrinsics folder)
 - rust-lang#132696 (Compile `test_num_f128` conditionally on `reliable_f128_math` config)
 - rust-lang#132737 (bootstrap: Print better message if lock pid isn't available)
 - rust-lang#132739 (Fix `librustdoc/scrape_examples.rs` formatting)
 - rust-lang#132740 (Update test for LLVM 20's new vector splat syntax)
 - rust-lang#132741 (Update mips64 data layout to match LLVM 20 change)

r? `@ghost`
`@rustbot` modify labels: rollup
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
4 participants