diff --git a/CHANGELOG.md b/CHANGELOG.md index b9e441029b2..e6c081ca94f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5616,6 +5616,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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 02543ae912a..f6c9ffea9fc 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -652,6 +652,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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dc24243b574..854c111f9f5 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -323,6 +323,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; @@ -1088,6 +1089,9 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { behavior: pub_underscore_fields_behavior, }) }); + 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` } diff --git a/clippy_lints/src/thread_local_initializer_can_be_made_const.rs b/clippy_lints/src/thread_local_initializer_can_be_made_const.rs new file mode 100644 index 00000000000..9fee4c06200 --- /dev/null +++ b/clippy_lints/src/thread_local_initializer_can_be_made_const.rs @@ -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); +} diff --git a/tests/ui/thread_local_initializer_can_be_made_const.fixed b/tests/ui/thread_local_initializer_can_be_made_const.fixed new file mode 100644 index 00000000000..bbde25b0a88 --- /dev/null +++ b/tests/ui/thread_local_initializer_can_be_made_const.fixed @@ -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 = const { RefCell::new(String::new()) }; + } + //~^^ ERROR: initializer for `thread_local` value can be made `const` + + // don't lint + thread_local! { + static BUF_2: RefCell = 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 = const { RefCell::new(String::new()) }; + static CONST_MIXED_WITH:i32 = const { 1 }; + static BUF_4_CAN_BE_MADE_CONST: RefCell = 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` +} diff --git a/tests/ui/thread_local_initializer_can_be_made_const.rs b/tests/ui/thread_local_initializer_can_be_made_const.rs new file mode 100644 index 00000000000..3d7aacf2f09 --- /dev/null +++ b/tests/ui/thread_local_initializer_can_be_made_const.rs @@ -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 = RefCell::new(String::new()); + } + //~^^ ERROR: initializer for `thread_local` value can be made `const` + + // don't lint + thread_local! { + static BUF_2: RefCell = 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 = RefCell::new(String::new()); + static CONST_MIXED_WITH:i32 = const { 1 }; + static BUF_4_CAN_BE_MADE_CONST: RefCell = RefCell::new(String::new()); + } + //~^^^^ ERROR: initializer for `thread_local` value can be made `const` + //~^^^ ERROR: initializer for `thread_local` value can be made `const` +} diff --git a/tests/ui/thread_local_initializer_can_be_made_const.stderr b/tests/ui/thread_local_initializer_can_be_made_const.stderr new file mode 100644 index 00000000000..b35bd306b52 --- /dev/null +++ b/tests/ui/thread_local_initializer_can_be_made_const.stderr @@ -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 = 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 = 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 = RefCell::new(String::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `const { RefCell::new(String::new()) }` + +error: aborting due to 4 previous errors +