about summary refs log tree commit diff
diff options
context:
space:
mode:
authorQuinn Sinclair <me@partiallytyped.dev>2024-01-22 00:16:47 +0100
committerQuinn Sinclair <me@partiallytyped.dev>2024-03-01 00:41:14 +0100
commitbb1ee8746ee0734fac664e78b59be589459c2026 (patch)
tree98e430c68ee92421221abfb894814f584f9518f3
parent3b8da4ac4b63ea37490fb26226e7eaa83a160733 (diff)
downloadrust-bb1ee8746ee0734fac664e78b59be589459c2026.tar.gz
rust-bb1ee8746ee0734fac664e78b59be589459c2026.zip
Fixed FP for `thread_local_initializer_can_be_made_const` for `os_local`
`os_local` impl of `thread_local` — regardless of whether it is const and
unlike other implementations — includes an `fn __init(): EXPR`.

Existing implementation of the lint checked for the presence of said
function and whether the expr can be made const. Because for `os_local`
we always have an `__init()`, it triggers for const implementations.

The solution is to check whether the `__init()` function is already const.
If it is `const`, there is nothing to do. Otherwise, we verify that we can
make it const.

Co-authored-by: Alejandra González <blyxyas@gmail.com>
-rw-r--r--clippy_lints/src/thread_local_initializer_can_be_made_const.rs64
-rw-r--r--tests/ui/thread_local_initializer_can_be_made_const.fixed7
-rw-r--r--tests/ui/thread_local_initializer_can_be_made_const.rs7
-rw-r--r--tests/ui/thread_local_initializer_can_be_made_const.stderr14
4 files changed, 71 insertions, 21 deletions
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
index 9fee4c06200..2dac5ff0c3a 100644
--- a/clippy_lints/src/thread_local_initializer_can_be_made_const.rs
+++ b/clippy_lints/src/thread_local_initializer_can_be_made_const.rs
@@ -1,12 +1,11 @@
 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 clippy_utils::{fn_has_unsatisfiable_preds, peel_blocks};
 use rustc_errors::Applicability;
 use rustc_hir::{intravisit, ExprKind};
-use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_middle::lint::in_external_macro;
+use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
 use rustc_span::sym::thread_local_macro;
 
@@ -57,6 +56,31 @@ impl ThreadLocalInitializerCanBeMadeConst {
 
 impl_lint_pass!(ThreadLocalInitializerCanBeMadeConst => [THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST]);
 
+#[inline]
+fn is_thread_local_initializer(
+    cx: &LateContext<'_>,
+    fn_kind: rustc_hir::intravisit::FnKind<'_>,
+    span: rustc_span::Span,
+) -> Option<bool> {
+    let macro_def_id = span.source_callee()?.macro_def_id?;
+    Some(
+        cx.tcx.is_diagnostic_item(thread_local_macro, macro_def_id)
+            && matches!(fn_kind, intravisit::FnKind::ItemFn(..)),
+    )
+}
+
+#[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.
+    if !fn_has_unsatisfiable_preds(cx, defid)
+        && let mir = cx.tcx.optimized_mir(defid)
+        && let Ok(()) = is_min_const_fn(cx.tcx, mir, msrv)
+    {
+        return true;
+    }
+    false
+}
+
 impl<'tcx> LateLintPass<'tcx> for ThreadLocalInitializerCanBeMadeConst {
     fn check_fn(
         &mut self,
@@ -65,31 +89,31 @@ impl<'tcx> LateLintPass<'tcx> for ThreadLocalInitializerCanBeMadeConst {
         _: &'tcx rustc_hir::FnDecl<'tcx>,
         body: &'tcx rustc_hir::Body<'tcx>,
         span: rustc_span::Span,
-        defid: rustc_span::def_id::LocalDefId,
+        local_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 defid = local_defid.to_def_id();
+        if is_thread_local_initializer(cx, fn_kind, span).unwrap_or(false)
+            // Some implementations of `thread_local!` include an initializer fn.
+            // In the case of a const initializer, the init fn is also const,
+            // so we can skip the lint in that case. This occurs only on some
+            // backends due to conditional compilation:
+            // https://doc.rust-lang.org/src/std/sys/common/thread_local/mod.rs.html
+            // 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(ret_expr) = block.expr
+            && let Some(unpeeled) = block.expr
+            && let ret_expr = peel_blocks(unpeeled)
             && 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,
+                unpeeled.span,
                 "initializer for `thread_local` value can be made `const`",
                 "replace with",
                 format!("const {{ {initializer_snippet} }}"),
diff --git a/tests/ui/thread_local_initializer_can_be_made_const.fixed b/tests/ui/thread_local_initializer_can_be_made_const.fixed
index bbde25b0a88..1a2c5ffc56f 100644
--- a/tests/ui/thread_local_initializer_can_be_made_const.fixed
+++ b/tests/ui/thread_local_initializer_can_be_made_const.fixed
@@ -27,4 +27,11 @@ fn main() {
     }
     //~^^^^ ERROR: initializer for `thread_local` value can be made `const`
     //~^^^ ERROR: initializer for `thread_local` value can be made `const`
+
+    thread_local! {
+        static PEEL_ME: i32 = const { 1 };
+        //~^ ERROR: initializer for `thread_local` value can be made `const`
+        static PEEL_ME_MANY: i32 = const { { let x = 1; x * x } };
+        //~^ 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
index 3d7aacf2f09..7fb00cd1342 100644
--- a/tests/ui/thread_local_initializer_can_be_made_const.rs
+++ b/tests/ui/thread_local_initializer_can_be_made_const.rs
@@ -27,4 +27,11 @@ fn main() {
     }
     //~^^^^ ERROR: initializer for `thread_local` value can be made `const`
     //~^^^ ERROR: initializer for `thread_local` value can be made `const`
+
+    thread_local! {
+        static PEEL_ME: i32 = { 1 };
+        //~^ ERROR: initializer for `thread_local` value can be made `const`
+        static PEEL_ME_MANY: i32 = { let x = 1; x * x };
+        //~^ 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
index 9ee52fbbb31..b4f8bd822b0 100644
--- a/tests/ui/thread_local_initializer_can_be_made_const.stderr
+++ b/tests/ui/thread_local_initializer_can_be_made_const.stderr
@@ -25,5 +25,17 @@ error: initializer for `thread_local` value can be made `const`
 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
+error: initializer for `thread_local` value can be made `const`
+  --> tests/ui/thread_local_initializer_can_be_made_const.rs:32:31
+   |
+LL |         static PEEL_ME: i32 = { 1 };
+   |                               ^^^^^ help: replace with: `const { 1 }`
+
+error: initializer for `thread_local` value can be made `const`
+  --> tests/ui/thread_local_initializer_can_be_made_const.rs:34:36
+   |
+LL |         static PEEL_ME_MANY: i32 = { let x = 1; x * x };
+   |                                    ^^^^^^^^^^^^^^^^^^^^ help: replace with: `const { { let x = 1; x * x } }`
+
+error: aborting due to 6 previous errors