Skip to content

Commit

Permalink
Threadlocal_initializer_can_be_made_const will not trigger for unreac…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Quinn Sinclair authored and blyxyas committed Apr 19, 2024
1 parent 2795a60 commit 206b1a1
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 7 deletions.
36 changes: 31 additions & 5 deletions clippy_lints/src/thread_local_initializer_can_be_made_const.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::macros::macro_backtrace;
use clippy_utils::qualify_min_const_fn::is_min_const_fn;
use clippy_utils::source::snippet;
use clippy_utils::{fn_has_unsatisfiable_preds, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::{intravisit, ExprKind};
use rustc_hir::{intravisit, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
use rustc_span::sym::thread_local_macro;
use rustc_span::sym::{self, thread_local_macro};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -69,6 +70,26 @@ fn is_thread_local_initializer(
)
}

fn is_unreachable(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let Some(macro_call) = macro_backtrace(expr.span).next()
&& let Some(diag_name) = cx.tcx.get_diagnostic_name(macro_call.def_id)
{
return (matches!(
diag_name,
sym::core_panic_macro
| sym::std_panic_macro
| sym::core_panic_2015_macro
| sym::std_panic_2015_macro
| sym::core_panic_2021_macro
) && !cx.tcx.hir().is_inside_const_context(expr.hir_id))
|| matches!(
diag_name,
sym::unimplemented_macro | sym::todo_macro | sym::unreachable_macro | sym::unreachable_2015_macro
);
}
false
}

#[inline]
fn initializer_can_be_made_const(cx: &LateContext<'_>, defid: rustc_span::def_id::DefId, msrv: &Msrv) -> bool {
// Building MIR for `fn`s with unsatisfiable preds results in ICE.
Expand Down Expand Up @@ -102,12 +123,17 @@ impl<'tcx> LateLintPass<'tcx> for ThreadLocalInitializerCanBeMadeConst {
// for details on this issue, see:
// https://github.com/rust-lang/rust-clippy/pull/12276
&& !cx.tcx.is_const_fn(defid)
&& initializer_can_be_made_const(cx, defid, &self.msrv)
// we know that the function is const-qualifiable, so now
// we need only to get the initializer expression to span-lint it.
&& let ExprKind::Block(block, _) = body.value.kind
&& let Some(unpeeled) = block.expr
&& let ret_expr = peel_blocks(unpeeled)
// A common pattern around threadlocal! is to make the value unreachable
// to force an initialization before usage
// https://github.com/rust-lang/rust-clippy/issues/12637
// we ensure that this is reachable before we check in mir
&& !is_unreachable(cx, ret_expr)
&& initializer_can_be_made_const(cx, defid, &self.msrv)
// we know that the function is const-qualifiable, so now
// we need only to get the initializer expression to span-lint it.
&& let initializer_snippet = snippet(cx, ret_expr.span, "thread_local! { ... }")
&& initializer_snippet != "thread_local! { ... }"
{
Expand Down
33 changes: 32 additions & 1 deletion tests/ui/thread_local_initializer_can_be_made_const.fixed
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![warn(clippy::thread_local_initializer_can_be_made_const)]

use std::cell::RefCell;
use std::cell::{Cell, RefCell};

fn main() {
// lint and suggest const
Expand Down Expand Up @@ -36,6 +36,37 @@ fn main() {
}
}

fn issue_12637() {
/// 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.
thread_local! {
static STATE_12637_PANIC: Cell<usize> = panic!();
}
STATE_12637_PANIC.set(9);
println!("{}", STATE_12637_PANIC.get());

thread_local! {
static STATE_12637_TODO: Cell<usize> = todo!();
}
STATE_12637_TODO.set(9);
println!("{}", STATE_12637_TODO.get());

thread_local! {
static STATE_12637_UNIMPLEMENTED: Cell<usize> = unimplemented!();
}
STATE_12637_UNIMPLEMENTED.set(9);
println!("{}", STATE_12637_UNIMPLEMENTED.get());

thread_local! {
static STATE_12637_UNREACHABLE: Cell<usize> = unreachable!();
}
STATE_12637_UNREACHABLE.set(9);
println!("{}", STATE_12637_UNREACHABLE.get());
}

#[clippy::msrv = "1.58"]
fn f() {
thread_local! {
Expand Down
33 changes: 32 additions & 1 deletion tests/ui/thread_local_initializer_can_be_made_const.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![warn(clippy::thread_local_initializer_can_be_made_const)]

use std::cell::RefCell;
use std::cell::{Cell, RefCell};

fn main() {
// lint and suggest const
Expand Down Expand Up @@ -36,6 +36,37 @@ fn main() {
}
}

fn issue_12637() {
/// 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.
thread_local! {
static STATE_12637_PANIC: Cell<usize> = panic!();
}
STATE_12637_PANIC.set(9);
println!("{}", STATE_12637_PANIC.get());

thread_local! {
static STATE_12637_TODO: Cell<usize> = todo!();
}
STATE_12637_TODO.set(9);
println!("{}", STATE_12637_TODO.get());

thread_local! {
static STATE_12637_UNIMPLEMENTED: Cell<usize> = unimplemented!();
}
STATE_12637_UNIMPLEMENTED.set(9);
println!("{}", STATE_12637_UNIMPLEMENTED.get());

thread_local! {
static STATE_12637_UNREACHABLE: Cell<usize> = unreachable!();
}
STATE_12637_UNREACHABLE.set(9);
println!("{}", STATE_12637_UNREACHABLE.get());
}

#[clippy::msrv = "1.58"]
fn f() {
thread_local! {
Expand Down

0 comments on commit 206b1a1

Please sign in to comment.