Footgun with const declarations of type Atomic*

[asajeffrey]

I just got caught by a footgun where I'd declared a global counter as:

const COUNTER: AtomicUsize = AtomicUsize::new(0);

and incremented it using COUNTER.fetch_add(1, Ordering::SeqCst). It took me a while to realize why I was always getting 0! There are checks for const declarations not having interior mutability to catch these footguns, but they don't apply to atomics.

[sfackler]

Clippy has a lint for this.

[Centril]

@sfackler I assume this refers to https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const ?

@asajeffrey Are you happy with that or would you like the language team to consider upstreaming the lint?

[asajeffrey]

The thing about that line is that you have to know to enable it, and pretty much the only way to learn that is the hard way. It would be nice if this was enabled by default, but that's probably a breaking change.

[Centril]

cc @oli-obk (who is on vacation, but they'll be back one day)

[mati865]

This lint is enabled by default (even more, it will stop compilation with the error), candidate for #53224

[emilio]

I just got beaten by this too, I think relying on clippy for it is not great.

[SimonSapin]

There are checks for const declarations not having interior mutability to catch these footguns, but they don't apply to atomics.

What checks are those? Why do they not apply?

[tgnottingham]

This does seem like the kind of thing the compiler should catch, which is the subject of #40543.

@asajeffrey The clippy lints do apply to atomics. You may have gotten bitten by rust-lang/rust-clippy#4612 or similar.

use std::sync::atomic::{AtomicUsize, Ordering};

const COUNTER: AtomicUsize = AtomicUsize::new(0);

fn main() {
    COUNTER.fetch_add(1, Ordering::SeqCst);
}

Clippy produces:

error: a `const` item should never be interior mutable
 --> src/main.rs:3:1
  |
3 | const COUNTER: AtomicUsize = AtomicUsize::new(0);
  | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  | |
  | make this a static item (maybe with lazy_static)
  |
  = 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:6:5
  |
6 |     COUNTER.fetch_add(1, Ordering::SeqCst);
  |     ^^^^^^^
  |
  = 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

[NovaDenizen]

This is still an issue. #53224 has been closed. 1-48-0-nightly still produces no warnings or errors.

[tesuji]

Here: #75573
Actually the above PR is orthogonal, and it won't fix this issue.
It will require uplift the lint from clippy.
If anybody is willing to do the work, then open an MCP issue and ping language team for sign off
to make progress on this.