Uplift declare_interior_mutable_const and borrow_interior_mutable_const from clippy to rustc

@Darksonn

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

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

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.

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.

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

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.

It feels like the clippy lint doesn't need to be perfect to have some parts of it uplifted? I just ran into this with AtomicU8... Maybe we could have a basic set covered in the rustc lint while the clippy analysis is perfected to drop false positives?

Navigation Menu

Repository navigation

Uplift `declare_interior_mutable_const` and `borrow_interior_mutable_const` from clippy to rustc #77983

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Uplift declare_interior_mutable_const and borrow_interior_mutable_const from clippy to rustc #77983

Description

Activity

jyn514 commented on Oct 15, 2020

flip1995 commented on Oct 15, 2020

rail-rain commented on Oct 16, 2020

somewheve commented on Jun 26, 2021

atsuzaki commented on Sep 1, 2023

djc commented on Nov 11, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Uplift `declare_interior_mutable_const` and `borrow_interior_mutable_const` from clippy to rustc #77983