Uplift declare_interior_mutable_const and borrow_interior_mutable_const from clippy to rustc

[jyn514]

In tokio 0.2, there was a footgun: if you created a const Runtime, then each time you called runtime.block_on() it would make a new runtime. This lead to confusing errors at runtime like 'reactor gone' which gave no hint of what actually went wrong: rust-lang/rust-clippy#6088

Fortunately, this was fixed by const_item_mutation in #75573. Unfortunately, Runtime::block_on will now take &self instead of &mut self starting in tokio 0.3: https://tokio.rs/blog/2020-10-tokio-0-3

Runtime can now only be constructed at runtime: as a local variable or in a once_cell, or lazy_static. The code for that looks like this:

use once_cell::sync::Lazy;
use tokio::runtime::Runtime;

const RUNTIME: Lazy<Runtime> = Lazy::new(|| {
    Runtime::new().unwrap()
});

fn main() {
    RUNTIME.block_on(async {});
}

rustc will compile this without a warning. Clippy, however, says that this is almost certainly not what you want:

error: a `const` item should never be interior mutable
 --> src/main.rs:4:1
  |
4 |   const RUNTIME: Lazy<Runtime> = Lazy::new(|| {
  |   ^----
  |   |
  |  _make this a static item (maybe with lazy_static)
  | |
5 | |     Runtime::new().unwrap()
6 | | });
  | |___^
  |
  = note: `#[deny(clippy::declare_interior_mutable_const)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const

error: a `const` item with interior mutability should not be borrowed
 --> src/main.rs:9:5
  |
9 |     RUNTIME.block_on(async {});
  |     ^^^^^^^
  |
  = note: `#[deny(clippy::borrow_interior_mutable_const)]` on by default
  = help: assign this const to a local or static variable, and use the variable here
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const

To avoid having this footgun, the lint should be uplifted from clippy to rustc (possibly as warn-by-default), so that users will notice it even if they don't use clippy.

cc @Darksonn

[jyn514]

Note the lint currently has a few false positives: rust-lang/rust-clippy#5812, rust-lang/rust-clippy#3825, rust-lang/rust-clippy#3962. I think these are being fixed in rust-lang/rust-clippy#6110, so uplifting this should wait until after that PR is merged.

cc @rust-lang/clippy

[flip1995]

I don't think this lint is ready to get uplifted, because it has too many FPs. I will do a review round in Clippy tomorrow and also take a look at rust-lang/rust-clippy#6110. If my opinion about that changes with rust-lang/rust-clippy#6110 merged, I'll write that here.

That being said, a FP free version of this lint would definitely be great in rustc.

[rail-rain]

rust-lang/rust-clippy#6110 doesn't fix rust-lang/rust-clippy#5812. My PR (6110) is for unfrozen enums with frozen variants as described in rust-lang/rust-clippy#3962. I bundled two fixes because I consider rust-lang/rust-clippy#3825 is the same as 3962 except, in the 3825 case, the constant defined in an outer crate; and the fact that the unfrozen structure in 3825 is Bytes is a coincidence. The proper solution for constants of Bytes (5812) might be to avoid linting "a const of an unfrozen type with a piece of code that never modify any of the type's interior mutable items", which I'm not sure if it's even achievable.

[somewheve]

use std::sync::atomic::AtomicBool;
use std::sync::atomic::Ordering::SeqCst;

pub const A: AtomicBool = AtomicBool::new(true);


fn main() {
    A.store(false, SeqCst);
    println!("Hello, world!");
}

C:\Users\Administrator\CLionProjects\xx>cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `target\debug\xx.exe`
Hello, world!

C:\Users\Administrator\CLionProjects\xx>cargo clippy
warning: a `const` item should never be interior mutable
 --> src\main.rs:4:1
  |
4 | pub const A: AtomicBool = AtomicBool::new(true);
  | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  | |
  | make this a static item (maybe with lazy_static)
  |
  = note: `#[warn(clippy::declare_interior_mutable_const)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const

warning: a `const` item with interior mutability should not be borrowed
 --> src\main.rs:8:5
  |
8 |     A.store(false, SeqCst);
  |     ^
  |
  = note: `#[warn(clippy::borrow_interior_mutable_const)]` on by default
  = help: assign this const to a local or static variable, and use the variable here
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const

warning: 2 warnings emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.02s

the lint should be uplifted from clippy to rustc too . wrong behavior just build success without any warning

[atsuzaki]

Someone ran into this footgun and filed an issue recently (#114939) so I'd like to take the opportunity to bump this.

What's the status on false positives? Seems like rust-lang/rust-clippy#5812 is still open.