Forbidding creation of constant mutexes, etc

[AngelicosPhosphoros]

I have seen a few cases when people create constant mutexes or OnceLock, or atomics instead of statics then cannot figure out why their code doesn't work. For example, this SO question: rust - How to return a reference to a global variable OnceLock? - Stack Overflow

I think, something need to be done with such cases. Maybe make some deny by default lint against constants with UnsafeCell inside?

[SkiFire13]

There's an open issue for this Produce a warning when using `const` with interior mutability · Issue #40543 · rust-lang/rust · GitHub Also note that there are use cases for having a const Mutex or other types with internal mutability, the problem only arises when you try to actually use the internal mutability.

[AngelicosPhosphoros]

@SkiFire13

the problem only arises when you try to actually use the internal mutability.

This approach has a major issue though: we cannot really track it. For example, this code would be completely opaque:

fn update_data(x: &'static Mutex<i32>){
   let g = x.lock().unwrap();
   *g += 1;
}
...
const MY_ERRONEOUS_MUTEX: Mutex<i32> = Mutex::new(0);

update_data(&MY_ERRONEOUS_MUTEX);

If we outright deny declarations of such constants, people who really need them would be able to just add #[allow(...)] attribute to them.

[Nemo157]

There is an existing warn-by-default clippy lint clippy::declare_interior_mutable_const.

I myself hit a false positive from this lint the other day

const EMPTY: Bytes = Bytes::from_static("');

Bytes uses interior mutability internally for optimization purposes, but doesn't expose that in any of its methods so I had to allow the lint. These sorts of false positives are one of the reasons lints are in clippy instead of being uplifted to rustc. (Everyone should be using clippy too, especially beginners that are likely to run into this sort of issue).

[quaternic]

@AngelicosPhosphoros

const MY_ERRONEOUS_MUTEX: Mutex<i32> = Mutex::new(0);

update_data(&MY_ERRONEOUS_MUTEX);

This code is already creating a temporary value which it then borrows, and that triggers clippy's borrow_interior_mutable_const. I agree it wouldn't make sense to lint differently depending on what the callee in some dependency does, but there is a significant difference between trying to reference the constant and using it as an initializer for new values.

In the case you really do want to pass a reference to a temporary to a function, the fixed code is much clearer, and I don't think it needs a warning:

let temporary_mutex = MY_ERRONEOUS_MUTEX;
update_data(&temporary_mutex);

It's worth comparing to how rustc currently handles code that takes a mutable reference to a const:

const MY_VEC: Vec<i32> = Vec::new();

fn main() {
    // warning: taking a mutable reference to a `const` item
    // note: each usage of a `const` item creates a new temporary
    // note: the mutable reference will refer to this temporary, not the original `const` item
    MY_VEC.push(1);

    // OK
    let mut tmp = MY_VEC;
    tmp.push(2);
}

Would it be reasonable to expand that warning to also cover shared references to types with interior mutability? With slightly adjusted wording to point to static as an option:

warning: taking a reference to a `const` item with interior mutability
note: each usage of a `const` item creates a new temporary
note: the reference will refer to this temporary, not the original `const` item
note: for a single shared instance of the type, consider using a `static` item instead