Skip to content

Commit

Permalink
New Lint: [thread_local_initializer_can_be_made_const]
Browse files Browse the repository at this point in the history
Adds a new lint to suggest using `const` on `thread_local!`
initializers that can be evaluated at compile time.

Impl details:

The lint relies on the expansion of `thread_local!`. For non
const-labelled initializers, `thread_local!` produces a function
called `__init` that lazily initializes the value. We check the function
and decide whether the body can be const. If so, we lint that the
initializer value can be made const.

changelog: new lint [`thread_local_initializer_can_be_made_const`
  • Loading branch information
m-rph committed Dec 28, 2023
1 parent 677f8d8 commit 4bf9066
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5611,6 +5611,7 @@ Released 2018-09-13
[`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
[`test_attr_in_doctest`]: https://rust-lang.github.io/rust-clippy/master/index.html#test_attr_in_doctest
[`tests_outside_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#tests_outside_test_module
[`thread_local_initializer_can_be_made_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#thread_local_initializer_can_be_made_const
[`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some
[`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display
[`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::tabs_in_doc_comments::TABS_IN_DOC_COMMENTS_INFO,
crate::temporary_assignment::TEMPORARY_ASSIGNMENT_INFO,
crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO,
crate::thread_local_initializer_can_be_made_const::THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST_INFO,
crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO,
crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO,
crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO,
Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ mod swap_ptr_to_ref;
mod tabs_in_doc_comments;
mod temporary_assignment;
mod tests_outside_test_module;
mod thread_local_initializer_can_be_made_const;
mod to_digit_is_some;
mod trailing_empty_array;
mod trait_bounds;
Expand Down Expand Up @@ -1080,6 +1081,9 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences));
store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions));
store.register_late_pass(|_| Box::new(unconditional_recursion::UnconditionalRecursion));
store.register_late_pass(move |_| {
Box::new(thread_local_initializer_can_be_made_const::ThreadLocalInitializerCanBeMadeConst::new(msrv()))
});
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
102 changes: 102 additions & 0 deletions clippy_lints/src/thread_local_initializer_can_be_made_const.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
use clippy_config::msrvs::Msrv;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::fn_has_unsatisfiable_preds;
use clippy_utils::qualify_min_const_fn::is_min_const_fn;
use clippy_utils::source::snippet;
use rustc_errors::Applicability;
use rustc_hir::{intravisit, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::impl_lint_pass;
use rustc_span::sym::thread_local_macro;

declare_clippy_lint! {
/// ### What it does
/// Suggests to use `const` in `thread_local!` macro if possible.
/// ### Why is this bad?
///
/// The `thread_local!` macro wraps static declarations and makes them thread-local.
/// It supports using a `const` keyword that may be used for declarations that can
/// be evaluated as a constant expression. This can enable a more efficient thread
/// local implementation that can avoid lazy initialization. For types that do not
/// need to be dropped, this can enable an even more efficient implementation that
/// does not need to track any additional state.
///
/// https://doc.rust-lang.org/std/macro.thread_local.html
///
/// ### Example
/// ```no_run
/// // example code where clippy issues a warning
/// thread_local! {
/// static BUF: String = String::new();
/// }
/// ```
/// Use instead:
/// ```no_run
/// // example code which does not raise clippy warning
/// thread_local! {
/// static BUF: String = const { String::new() };
/// }
/// ```
#[clippy::version = "1.76.0"]
pub THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST,
perf,
"suggest using `const` in `thread_local!` macro"
}

pub struct ThreadLocalInitializerCanBeMadeConst {
msrv: Msrv,
}

impl ThreadLocalInitializerCanBeMadeConst {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
}
}

impl_lint_pass!(ThreadLocalInitializerCanBeMadeConst => [THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST]);

impl<'tcx> LateLintPass<'tcx> for ThreadLocalInitializerCanBeMadeConst {
fn check_fn(
&mut self,
cx: &LateContext<'tcx>,
fn_kind: rustc_hir::intravisit::FnKind<'tcx>,
_: &'tcx rustc_hir::FnDecl<'tcx>,
body: &'tcx rustc_hir::Body<'tcx>,
span: rustc_span::Span,
defid: rustc_span::def_id::LocalDefId,
) {
if in_external_macro(cx.sess(), span)
&& let Some(callee) = span.source_callee()
&& let Some(macro_def_id) = callee.macro_def_id
&& cx.tcx.is_diagnostic_item(thread_local_macro, macro_def_id)
&& let intravisit::FnKind::ItemFn(..) = fn_kind
// Building MIR for `fn`s with unsatisfiable preds results in ICE.
&& !fn_has_unsatisfiable_preds(cx, defid.to_def_id())
&& let mir = cx.tcx.optimized_mir(defid.to_def_id())
&& let Ok(()) = is_min_const_fn(cx.tcx, mir, &self.msrv)
// this is the `__init` function emitted by the `thread_local!` macro
// when the `const` keyword is not used. We avoid checking the `__init` directly
// as that is not a public API.
// 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(ret_expr) = block.expr
&& let initializer_snippet = snippet(cx, ret_expr.span, "thread_local! { ... }")
&& initializer_snippet != "thread_local! { ... }"
{
span_lint_and_sugg(
cx,
THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST,
ret_expr.span,
"initializer for `thread_local` value can be made `const`",
"replace with",
format!("const {} {initializer_snippet} {}", "{", "}"),
Applicability::MachineApplicable,
);
}
}

extract_msrv_attr!(LateContext);
}
30 changes: 30 additions & 0 deletions tests/ui/thread_local_initializer_can_be_made_const.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![warn(clippy::thread_local_initializer_can_be_made_const)]

use std::cell::RefCell;

fn main() {
// lint and suggest const
thread_local! {
static BUF_1: RefCell<String> = const { RefCell::new(String::new()) };
}
//~^^ ERROR: initializer for `thread_local` value can be made `const`

// don't lint
thread_local! {
static BUF_2: RefCell<String> = const { RefCell::new(String::new()) };
}

thread_local! {
static SIMPLE:i32 = const { 1 };
}
//~^^ ERROR: initializer for `thread_local` value can be made `const`

// lint and suggest const for all non const items
thread_local! {
static BUF_3_CAN_BE_MADE_CONST: RefCell<String> = const { RefCell::new(String::new()) };
static CONST_MIXED_WITH:i32 = const { 1 };
static BUF_4_CAN_BE_MADE_CONST: RefCell<String> = const { RefCell::new(String::new()) };
}
//~^^^^ ERROR: initializer for `thread_local` value can be made `const`
//~^^^ ERROR: initializer for `thread_local` value can be made `const`
}
30 changes: 30 additions & 0 deletions tests/ui/thread_local_initializer_can_be_made_const.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![warn(clippy::thread_local_initializer_can_be_made_const)]

use std::cell::RefCell;

fn main() {
// lint and suggest const
thread_local! {
static BUF_1: RefCell<String> = RefCell::new(String::new());
}
//~^^ ERROR: initializer for `thread_local` value can be made `const`

// don't lint
thread_local! {
static BUF_2: RefCell<String> = const { RefCell::new(String::new()) };
}

thread_local! {
static SIMPLE:i32 = 1;
}
//~^^ ERROR: initializer for `thread_local` value can be made `const`

// lint and suggest const for all non const items
thread_local! {
static BUF_3_CAN_BE_MADE_CONST: RefCell<String> = RefCell::new(String::new());
static CONST_MIXED_WITH:i32 = const { 1 };
static BUF_4_CAN_BE_MADE_CONST: RefCell<String> = RefCell::new(String::new());
}
//~^^^^ ERROR: initializer for `thread_local` value can be made `const`
//~^^^ ERROR: initializer for `thread_local` value can be made `const`
}
29 changes: 29 additions & 0 deletions tests/ui/thread_local_initializer_can_be_made_const.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error: initializer for `thread_local` value can be made `const`
--> $DIR/thread_local_initializer_can_be_made_const.rs:8:41
|
LL | static BUF_1: RefCell<String> = RefCell::new(String::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `const { RefCell::new(String::new()) }`
|
= note: `-D clippy::thread-local-initializer-can-be-made-const` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::thread_local_initializer_can_be_made_const)]`

error: initializer for `thread_local` value can be made `const`
--> $DIR/thread_local_initializer_can_be_made_const.rs:18:29
|
LL | static SIMPLE:i32 = 1;
| ^ help: replace with: `const { 1 }`

error: initializer for `thread_local` value can be made `const`
--> $DIR/thread_local_initializer_can_be_made_const.rs:24:59
|
LL | static BUF_3_CAN_BE_MADE_CONST: RefCell<String> = RefCell::new(String::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `const { RefCell::new(String::new()) }`

error: initializer for `thread_local` value can be made `const`
--> $DIR/thread_local_initializer_can_be_made_const.rs:26:59
|
LL | static BUF_4_CAN_BE_MADE_CONST: RefCell<String> = RefCell::new(String::new());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `const { RefCell::new(String::new()) }`

error: aborting due to 4 previous errors

0 comments on commit 4bf9066

Please sign in to comment.