Auto merge of #12026 - PartiallyTyped:12015, r=llogiq

New Lint: [`thread_local_initializer_can_be_made_const`]

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`]

fixes: #12015

Author: bors

Diff for: 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 for: 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 for: 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 for: 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 for: 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<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`
+}

Diff for: 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<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`
+}

Diff for: 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<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
+