mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-23 15:23:46 +00:00
Auto merge of #12276 - PartiallyTyped:hide-thread-local, r=blyxyas
Fix FP in `threadlocal!` when falling back to `os_local` **Issue**: The lint always triggers for some targets. **Why**: The lint assumes an `__init` function is not present for const initializers. This does not hold for all targets. Some targets always have an initializer function. **Fix**: If an initializer function exists, we check that it is not a `const` fn. Lint overview before the patch: 1. Outside `threadlocal!`, then exit early. 2. In `threadlocal!` and `__init` does not exist => is const already, exit early. 3. In `threadlocal!` and `__init` exists and `__init` body can be `const` => we lint. Lint overview after the patch: 1. Outside `threadlocal!`, then exit early. 2. In `threadlocal!` and `__init` does not exist => is const already, exit early. 3. In `threadlocal!` and `__init` exists and is `const fn` => is const already, exit early. 4. In `threadlocal!` and `__init` exists and is not `const fn` and `__init` body can be `const` => we lint. The patch adds step 3. changelog: Fixed fp in [`thread_local_initializer_can_be_made_const`] where fallback implementation of `threadlocal!` was always linted. ## Verifying the changes For each of these platforms [ mac, x86_64-windows-gnu, x86_64-windows-msvc ]: 1. switched to master, ran bless. Found a failure for windows-gnu. 2. switched to patch, ran bless. Success for all three platforms. ## How platform affects the lint: The compiler performs a [conditional compilation depending on platform features](https://doc.rust-lang.org/src/std/sys/common/thread_local/mod.rs.html). The `os_local` fallback includes a call to `__init`, without step 3, we lint in all cases. ```rust cfg_if::cfg_if! { if #[cfg(any(all(target_family = "wasm", not(target_feature = "atomics")), target_os = "uefi"))] { #[doc(hidden)] mod static_local; #[doc(hidden)] pub use static_local::{Key, thread_local_inner}; } else if #[cfg(target_thread_local)] { #[doc(hidden)] mod fast_local; #[doc(hidden)] pub use fast_local::{Key, thread_local_inner}; } else { #[doc(hidden)] mod os_local; #[doc(hidden)] pub use os_local::{Key, thread_local_inner}; } } ``` r? `@llogiq`
This commit is contained in:
commit
225d3771d7
@ -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} }}"),
|
||||
|
@ -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`
|
||||
}
|
||||
}
|
||||
|
@ -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`
|
||||
}
|
||||
}
|
||||
|
@ -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
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user