rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
10.89k stars 1.46k forks source link

Threadlocal_initializer_can_be_made_const will not trigger for unreachable initializers #12685

Closed m-rph closed 3 weeks ago

m-rph commented 3 weeks ago

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.

rustbot commented 3 weeks ago

r? @llogiq

rustbot has assigned @llogiq. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

blyxyas commented 3 weeks ago

I've made a new patch that does a few things: Get the diagnostic item name as a variable and extrapolate that is_panic to be able to reuse the diag name for the todo, unreachable, unimplemented.

blyxyas commented 3 weeks ago

@bors r+

bors commented 3 weeks ago

:pushpin: Commit 206b1a1ac9df38ca5278a6db9954f9c62bd34b17 has been approved by blyxyas

It is now in the queue for this repository.

m-rph commented 3 weeks ago

This pattern seems to happen pretty often. Do you think we could move this to utils and perhaps return an Option<UnreachableKind>?

bors commented 3 weeks ago

:hourglass: Testing commit 206b1a1ac9df38ca5278a6db9954f9c62bd34b17 with merge 33edb1920b0684d418b8e35da9315f6ca870fbb4...

blyxyas commented 3 weeks ago

@bors r-

blyxyas commented 3 weeks ago

Hmmmm... lemme see where else is this pattern used (meow)

bors commented 3 weeks ago

:sunny: Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test Build commit: 33edb1920b0684d418b8e35da9315f6ca870fbb4 (33edb1920b0684d418b8e35da9315f6ca870fbb4)

blyxyas commented 3 weeks ago

Where else is some kind of function like this used? I've tried to Ctrl+Shift+F but there doesn't seem to exist almost anywhere else (and be extrapolatable)

m-rph commented 3 weeks ago

Off the top of my head; unused_io_amount and panic_unimplemented.

In panic_unimplemented knowledge of which macro it is helps. In unused_io_amount it isn't.

blyxyas commented 3 weeks ago

Hmmm... If there are at least two others and you're feeling extra-helpful today, yeah it would help! :heart: But that should be another PR (PRs should be as small as possible). You can assign me to it when you open it :cat: (if you're planning to do it)

blyxyas commented 3 weeks ago

@bors r+

bors commented 3 weeks ago

:pushpin: Commit 206b1a1ac9df38ca5278a6db9954f9c62bd34b17 has been approved by blyxyas

It is now in the queue for this repository.

bors commented 3 weeks ago

:hourglass: Testing commit 206b1a1ac9df38ca5278a6db9954f9c62bd34b17 with merge c2a98cbfa59bf4136272912aef9ac9af22b34496...

bors commented 3 weeks ago

:sunny: Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test Approved by: blyxyas Pushing c2a98cbfa59bf4136272912aef9ac9af22b34496 to master...