Open jyn514 opened 3 years ago
Note the lint currently has a few false positives: https://github.com/rust-lang/rust-clippy/issues/5812, https://github.com/rust-lang/rust-clippy/issues/3825, https://github.com/rust-lang/rust-clippy/issues/3962. I think these are being fixed in https://github.com/rust-lang/rust-clippy/pull/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
.
https://github.com/rust-lang/rust-clippy/pull/6110 doesn't fix https://github.com/rust-lang/rust-clippy/issues/5812. My PR (6110) is for unfrozen enums with frozen variants as described in https://github.com/rust-lang/rust-clippy/issues/3962. I bundled two fixes because I consider https://github.com/rust-lang/rust-clippy/issues/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 https://github.com/rust-lang/rust-clippy/issues/5812 is still open.
In tokio 0.2, there was a footgun: if you created a
const Runtime
, then each time you calledruntime.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: https://github.com/rust-lang/rust-clippy/issues/6088Fortunately, this was fixed by
const_item_mutation
in https://github.com/rust-lang/rust/pull/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-3Runtime
can now only be constructed at runtime: as a local variable or in aonce_cell
, orlazy_static
. The code for that looks like this:rustc will compile this without a warning. Clippy, however, says that this is almost certainly not what you want:
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