static_mut_refs lint fires on `assert_eq`

[RalfJung]

Code

static mut S: i32 = 0;

fn main() {
    unsafe {
        assert!(S == 0);
        assert_eq!(S, 0);
    }
}

Current output

warning: creating a shared reference to mutable static is discouraged
 --> src/main.rs:6:20
  |
6 |         assert_eq!(S, 0);
  |                    ^ 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

Desired output

(none)

Rationale and extra context

The two macro invocations do the same thing, but in the first case we recognize that the reference created for == is a "short-lived reference" and suppress the warning. In the second case, we do show a warning -- I suspect it has to do with the formatting? Indeed println!("{}", S); also triggers a warning. Maybe formatting macros where we know that the reference does not outlive the macro could be recognized by the lint?

An alternative would be to suggest using { S } in these cases which will also avoid the warning.

Other cases

No response

Rust Version

current nightly

Anything else?

No response

[GrigorenkoPV]

I suspect it has to do with the formatting?

Kinda. It has everything to do with the expansions, but it is dictated by assert_eq!'s need to bind the expressions it recieves, because it wants to both compare them and print them in case of failure.

So this code:

static mut S: i32 = 0;

fn assert() {
    unsafe { assert!(S == 0) }
}

fn assert_eq() {
    unsafe { assert_eq!(S, 0) }
}

fn main() {
    assert();
    assert_eq();
}

expands to

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;

static mut S: i32 = 0;

fn assert() {
    unsafe {
        if !(S == 0) {
            ::core::panicking::panic("assertion failed: S == 0")
        }
    }
}

fn assert_eq() {
    unsafe {
        match (&S, &0) {
            (left_val, right_val) => {
                if !(*left_val == *right_val) {
                    let kind = ::core::panicking::AssertKind::Eq;
                    ::core::panicking::assert_failed(
                        kind,
                        &*left_val,
                        &*right_val,
                        ::core::option::Option::None,
                    );
                }
            }
        }
    }
}

fn main() {
    assert();
    assert_eq();
}

according to cargo expand.

We can probably just slap #[allow] onto the code that assert_eq! produces, but idk if that's a good idea.

[GrigorenkoPV]

Also, I've just realized that having a const left_val or const right_val declared breaks assert_eq!.

static S: i32 = 0;

const left_val: u32 = 1;

fn main() {
    unsafe { assert_eq!(S, 0) }
}

gives

error[E0308]: mismatched types
 --> src/main.rs:6:14
  |
3 | const left_val: u32 = 1;
  | ------------------- constant defined here
...
6 |     unsafe { assert_eq!(S, 0) }
  |              ^^^^^^^^^^^^^^^^
  |              |
  |              expected `&i32`, found `u32`
  |              this expression has type `(&i32, &{integer})`
  |              `left_val` is interpreted as a constant, not a new binding
  |              help: introduce a new binding instead: `other_left_val`
  |
  = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0614]: type `u32` cannot be dereferenced
 --> src/main.rs:6:14
  |
6 |     unsafe { assert_eq!(S, 0) }
  |              ^^^^^^^^^^^^^^^^
  |
  = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)

The error message, admittedly, is not very helpful.

Thanlfully, you can't use this to make assert_eq pass/fail when it shouldn't. The worst you can acheive is a cryptic error message from the compiler. I think.

See also: https://sabrinajewson.org/blog/truly-hygienic-let

[RalfJung]

Also, I've just realized that having a const left_val or const right_val declared breaks assert_eq!.

That's a totally different issue though. :)

[RalfJung]

We can probably just slap #[allow] onto the code that assert_eq! produces, but idk if that's a good idea.

Seems reasonable to me. I guess the one potential problem is a static mut of custom type where then this will invoke Debug::fmt with a reference to the static and the fmt code could also directly access the static... but it's just a lint, some false negatives are fine in particular if they seriously cut down on false positives.

[GrigorenkoPV]

Also, I've just realized that having a const left_val or const right_val declared breaks assert_eq!.

That's a totally different issue though. :)

True, I've opened #131446 for it.

[GrigorenkoPV]

An alternative would be to suggest using { S } in these cases which will also avoid the warning.

This only works for Copy types, because &{ S } reads the value from S, copies it into a temporary and takes a ref to that.