Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

thread_local_initializer_can_be_made_const false positive: panicking Cell and RefCell initialization #12637

Closed
dtolnay opened this issue Apr 5, 2024 · 1 comment · Fixed by #12685
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@dtolnay
Copy link
Member

dtolnay commented Apr 5, 2024

Summary

The set methods on LocalKey<Cell<T>> and LocalKey<RefCell<T>> are guaranteed to bypass the thread_local's initialization expression. See rust-lang/rust#92122. Thus, = panic!() is a useful idiom for forcing the use of set on each thread before it accesses the thread local in any other manner.

Clippy suggests replacing this with = const { panic!() }, which does not compile.

Lint Name

thread_local_initializer_can_be_made_const

Reproducer

use std::cell::Cell;

thread_local! {
    static STATE: Cell<usize> = panic!();
}

fn main() {
    STATE.set(9);
    println!("{}", STATE.get());
}
warning: initializer for `thread_local` value can be made `const`
 --> src/main.rs:4:33
  |
4 |     static STATE: Cell<usize> = panic!();
  |                                 ^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#thread_local_initializer_can_be_made_const
  = note: `#[warn(clippy::thread_local_initializer_can_be_made_const)]` on by default
  = note: this warning originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

The suggested change makes the code not compile.

error[E0080]: evaluation of constant value failed
 --> src/main.rs:4:41
  |
4 |     static STATE: Cell<usize> = const { panic!() };
  |                                         ^^^^^^^^ the evaluated program panicked at 'explicit panic', src/main.rs:4:41
  |
  = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)

note: erroneous constant encountered
 --> src/main.rs:3:1
  |
3 | / thread_local! {
4 | |     static STATE: Cell<usize> = const { panic!() };
5 | | }
  | |_^
  |
  = note: this note originates in the macro `$crate::thread::local_impl::thread_local_inner` which comes from the expansion of the macro `thread_local` (in Nightly builds, run with -Z macro-backtrace for more info)

Version

rustc 1.79.0-nightly (385fa9d84 2024-04-04)
binary: rustc
commit-hash: 385fa9d845dd326c6bbfd58c22244215e431948a
commit-date: 2024-04-04
host: x86_64-unknown-linux-gnu
release: 1.79.0-nightly
LLVM version: 18.1.2

Additional Labels

@rustbot label +I-suggestion-causes-error

@dtolnay dtolnay added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Apr 5, 2024
@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Apr 5, 2024
@m-rph
Copy link
Contributor

m-rph commented Apr 16, 2024

@rustbot claim.

I think we can simply add a check to ensure whether the inner construct is not something that panics.

@dtolnay do you know of other similar patterns?

@m-rph m-rph added I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied I-false-positive Issue: The lint was triggered on code it shouldn't have and removed I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied I-false-positive Issue: The lint was triggered on code it shouldn't have labels Apr 17, 2024
m-rph added a commit to m-rph/rust-clippy that referenced this issue Apr 17, 2024
…hable initializers

This commit introduces a check to ensure that the lint won't trigger when the initializer is
unreachable, such as:

```
thread_local! {
    static STATE: Cell<usize> = panic!();
}
```

This is achieved by looking at the unpeeled initializer expression and ensuring that the parent
macro is not `panic!()`, `todo!()`, `unreachable!()`, `unimplemented!()`.

fixes rust-lang#12637

changelog: [`threadlocal_initializer_can_be_made_const`] will no longer trigger on `unreachable` macros.
m-rph pushed a commit to m-rph/rust-clippy that referenced this issue Apr 17, 2024
…hable initializers

This commit introduces a check to ensure that the lint won't trigger when the initializer is
unreachable, such as:

```
thread_local! {
    static STATE: Cell<usize> = panic!();
}
```

This is achieved by looking at the unpeeled initializer expression and ensuring that the parent
macro is not `panic!()`, `todo!()`, `unreachable!()`, `unimplemented!()`.

fixes rust-lang#12637

changelog: [`threadlocal_initializer_can_be_made_const`] will no longer trigger on `unreachable` macros.
blyxyas pushed a commit to m-rph/rust-clippy that referenced this issue Apr 19, 2024
…hable initializers

This commit introduces a check to ensure that the lint won't trigger when the initializer is
unreachable, such as:

```
thread_local! {
    static STATE: Cell<usize> = panic!();
}
```

This is achieved by looking at the unpeeled initializer expression and ensuring that the parent
macro is not `panic!()`, `todo!()`, `unreachable!()`, `unimplemented!()`.

fixes rust-lang#12637

changelog: [`threadlocal_initializer_can_be_made_const`] will no longer trigger on `unreachable` macros.
bors added a commit that referenced this issue Apr 19, 2024
Threadlocal_initializer_can_be_made_const will not trigger for unreachable initializers

This commit introduces a check to ensure that the lint won't trigger when the initializer is unreachable, such as:

```
thread_local! {
    static STATE: Cell<usize> = panic!();
}
```

This is achieved by looking at the unpeeled initializer expression and ensuring that the parent macro is not `panic!()`, `todo!()`, `unreachable!()`, `unimplemented!()`.

fixes #12637

changelog: [`threadlocal_initializer_can_be_made_const`] will no longer trigger on `unreachable` macros.
bors added a commit that referenced this issue Apr 20, 2024
Threadlocal_initializer_can_be_made_const will not trigger for unreachable initializers

This commit introduces a check to ensure that the lint won't trigger when the initializer is unreachable, such as:

```
thread_local! {
    static STATE: Cell<usize> = panic!();
}
```

This is achieved by looking at the unpeeled initializer expression and ensuring that the parent macro is not `panic!()`, `todo!()`, `unreachable!()`, `unimplemented!()`.

fixes #12637

changelog: [`threadlocal_initializer_can_be_made_const`] will no longer trigger on `unreachable` macros.
@bors bors closed this as completed in 206b1a1 Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
3 participants