diff --git a/src/tools/clippy/clippy_config/src/msrvs.rs b/src/tools/clippy/clippy_config/src/msrvs.rs index 72d5b9aff28df..0c6368d33f48b 100644 --- a/src/tools/clippy/clippy_config/src/msrvs.rs +++ b/src/tools/clippy/clippy_config/src/msrvs.rs @@ -21,6 +21,7 @@ msrv_aliases! { 1,68,0 { PATH_MAIN_SEPARATOR_STR } 1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS } 1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE } + 1,59,0 { THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST } 1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY } 1,55,0 { SEEK_REWIND } 1,54,0 { INTO_KEYS } diff --git a/src/tools/clippy/clippy_lints/src/indexing_slicing.rs b/src/tools/clippy/clippy_lints/src/indexing_slicing.rs index 391db0b0df726..35fcd8cdd3547 100644 --- a/src/tools/clippy/clippy_lints/src/indexing_slicing.rs +++ b/src/tools/clippy/clippy_lints/src/indexing_slicing.rs @@ -174,6 +174,7 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing { // only `usize` index is legal in rust array index // leave other type to rustc if let Constant::Int(off) = constant + && off <= usize::MAX as u128 && let ty::Uint(utype) = cx.typeck_results().expr_ty(index).kind() && *utype == ty::UintTy::Usize && let ty::Array(_, s) = ty.kind() diff --git a/src/tools/clippy/clippy_lints/src/thread_local_initializer_can_be_made_const.rs b/src/tools/clippy/clippy_lints/src/thread_local_initializer_can_be_made_const.rs index 9fee4c062007f..1af3733ebfa4b 100644 --- a/src/tools/clippy/clippy_lints/src/thread_local_initializer_can_be_made_const.rs +++ b/src/tools/clippy/clippy_lints/src/thread_local_initializer_can_be_made_const.rs @@ -1,12 +1,11 @@ -use clippy_config::msrvs::Msrv; +use clippy_config::msrvs::{self, 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 { + 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,32 @@ 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 self.msrv.meets(msrvs::THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST) + && 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/src/tools/clippy/clippy_lints/src/unconditional_recursion.rs b/src/tools/clippy/clippy_lints/src/unconditional_recursion.rs index 209035804e43e..224ec475c5107 100644 --- a/src/tools/clippy/clippy_lints/src/unconditional_recursion.rs +++ b/src/tools/clippy/clippy_lints/src/unconditional_recursion.rs @@ -69,14 +69,6 @@ fn span_error(cx: &LateContext<'_>, method_span: Span, expr: &Expr<'_>) { ); } -fn get_ty_def_id(ty: Ty<'_>) -> Option { - match ty.peel_refs().kind() { - ty::Adt(adt, _) => Some(adt.did()), - ty::Foreign(def_id) => Some(*def_id), - _ => None, - } -} - fn get_hir_ty_def_id<'tcx>(tcx: TyCtxt<'tcx>, hir_ty: rustc_hir::Ty<'tcx>) -> Option { let TyKind::Path(qpath) = hir_ty.kind else { return None }; match qpath { @@ -131,21 +123,49 @@ fn get_impl_trait_def_id(cx: &LateContext<'_>, method_def_id: LocalDefId) -> Opt } } -#[allow(clippy::unnecessary_def_path)] +/// When we have `x == y` where `x = &T` and `y = &T`, then that resolves to +/// `<&T as PartialEq<&T>>::eq`, which is not the same as `>::eq`, +/// however we still would want to treat it the same, because we know that it's a blanket impl +/// that simply delegates to the `PartialEq` impl with one reference removed. +/// +/// Still, we can't just do `lty.peel_refs() == rty.peel_refs()` because when we have `x = &T` and +/// `y = &&T`, this is not necessarily the same as `>::eq` +/// +/// So to avoid these FNs and FPs, we keep removing a layer of references from *both* sides +/// until both sides match the expected LHS and RHS type (or they don't). +fn matches_ty<'tcx>( + mut left: Ty<'tcx>, + mut right: Ty<'tcx>, + expected_left: Ty<'tcx>, + expected_right: Ty<'tcx>, +) -> bool { + while let (&ty::Ref(_, lty, _), &ty::Ref(_, rty, _)) = (left.kind(), right.kind()) { + if lty == expected_left && rty == expected_right { + return true; + } + left = lty; + right = rty; + } + false +} + fn check_partial_eq(cx: &LateContext<'_>, method_span: Span, method_def_id: LocalDefId, name: Ident, expr: &Expr<'_>) { - let args = cx - .tcx - .instantiate_bound_regions_with_erased(cx.tcx.fn_sig(method_def_id).skip_binder()) - .inputs(); + let Some(sig) = cx + .typeck_results() + .liberated_fn_sigs() + .get(cx.tcx.local_def_id_to_hir_id(method_def_id)) + else { + return; + }; + // That has two arguments. - if let [self_arg, other_arg] = args - && let Some(self_arg) = get_ty_def_id(*self_arg) - && let Some(other_arg) = get_ty_def_id(*other_arg) + if let [self_arg, other_arg] = sig.inputs() + && let &ty::Ref(_, self_arg, _) = self_arg.kind() + && let &ty::Ref(_, other_arg, _) = other_arg.kind() // The two arguments are of the same type. - && self_arg == other_arg && let Some(trait_def_id) = get_impl_trait_def_id(cx, method_def_id) // The trait is `PartialEq`. - && Some(trait_def_id) == get_trait_def_id(cx, &["core", "cmp", "PartialEq"]) + && cx.tcx.is_diagnostic_item(sym::PartialEq, trait_def_id) { let to_check_op = if name.name == sym::eq { BinOpKind::Eq @@ -154,31 +174,19 @@ fn check_partial_eq(cx: &LateContext<'_>, method_span: Span, method_def_id: Loca }; let is_bad = match expr.kind { ExprKind::Binary(op, left, right) if op.node == to_check_op => { - // Then we check if the left-hand element is of the same type as `self`. - if let Some(left_ty) = cx.typeck_results().expr_ty_opt(left) - && let Some(left_id) = get_ty_def_id(left_ty) - && self_arg == left_id - && let Some(right_ty) = cx.typeck_results().expr_ty_opt(right) - && let Some(right_id) = get_ty_def_id(right_ty) - && other_arg == right_id - { - true - } else { - false - } + // Then we check if the LHS matches self_arg and RHS matches other_arg + let left_ty = cx.typeck_results().expr_ty_adjusted(left); + let right_ty = cx.typeck_results().expr_ty_adjusted(right); + matches_ty(left_ty, right_ty, self_arg, other_arg) }, - ExprKind::MethodCall(segment, receiver, &[_arg], _) if segment.ident.name == name.name => { - if let Some(ty) = cx.typeck_results().expr_ty_opt(receiver) - && let Some(ty_id) = get_ty_def_id(ty) - && self_arg != ty_id - { - // Since this called on a different type, the lint should not be - // triggered here. - return; - } + ExprKind::MethodCall(segment, receiver, [arg], _) if segment.ident.name == name.name => { + let receiver_ty = cx.typeck_results().expr_ty_adjusted(receiver); + let arg_ty = cx.typeck_results().expr_ty_adjusted(arg); + if let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) && let Some(trait_id) = cx.tcx.trait_of_item(fn_id) && trait_id == trait_def_id + && matches_ty(receiver_ty, arg_ty, self_arg, other_arg) { true } else { diff --git a/src/tools/clippy/tests/ui/crashes/ice-12253.rs b/src/tools/clippy/tests/ui/crashes/ice-12253.rs new file mode 100644 index 0000000000000..41f50035144a7 --- /dev/null +++ b/src/tools/clippy/tests/ui/crashes/ice-12253.rs @@ -0,0 +1,5 @@ +#[allow(overflowing_literals, unconditional_panic, clippy::no_effect)] +fn main() { + let arr: [i32; 5] = [0; 5]; + arr[0xfffffe7ffffffffffffffffffffffff]; +} diff --git a/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.fixed b/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.fixed index bbde25b0a88b5..a6ed59d49c54f 100644 --- a/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.fixed +++ b/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.fixed @@ -27,4 +27,18 @@ 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` + } +} + +#[clippy::msrv = "1.58"] +fn f() { + thread_local! { + static TLS: i32 = 1; + } } diff --git a/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.rs b/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.rs index 3d7aacf2f094f..3f0159c58065c 100644 --- a/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.rs +++ b/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.rs @@ -27,4 +27,18 @@ 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` + } +} + +#[clippy::msrv = "1.58"] +fn f() { + thread_local! { + static TLS: i32 = 1; + } } diff --git a/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.stderr b/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.stderr index b35bd306b5230..2cb51850c4855 100644 --- a/src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.stderr +++ b/src/tools/clippy/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 = 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` + --> $DIR/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` + --> $DIR/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 diff --git a/src/tools/clippy/tests/ui/unconditional_recursion.rs b/src/tools/clippy/tests/ui/unconditional_recursion.rs index 7b898a6e0e784..263fdf26d4d2f 100644 --- a/src/tools/clippy/tests/ui/unconditional_recursion.rs +++ b/src/tools/clippy/tests/ui/unconditional_recursion.rs @@ -288,4 +288,63 @@ impl PartialEq for S15<'_> { } } +mod issue12154 { + struct MyBox(T); + + impl std::ops::Deref for MyBox { + type Target = T; + fn deref(&self) -> &T { + &self.0 + } + } + + impl PartialEq for MyBox { + fn eq(&self, other: &Self) -> bool { + (**self).eq(&**other) + } + } + + // Not necessarily related to the issue but another FP from the http crate that was fixed with it: + // https://docs.rs/http/latest/src/http/header/name.rs.html#1424 + // We used to simply peel refs from the LHS and RHS, so we couldn't differentiate + // between `PartialEq for &T` and `PartialEq<&T> for T` impls. + #[derive(PartialEq)] + struct HeaderName; + impl<'a> PartialEq<&'a HeaderName> for HeaderName { + fn eq(&self, other: &&'a HeaderName) -> bool { + *self == **other + } + } + + impl<'a> PartialEq for &'a HeaderName { + fn eq(&self, other: &HeaderName) -> bool { + *other == *self + } + } + + // Issue #12181 but also fixed by the same PR + struct Foo; + + impl Foo { + fn as_str(&self) -> &str { + "Foo" + } + } + + impl PartialEq for Foo { + fn eq(&self, other: &Self) -> bool { + self.as_str().eq(other.as_str()) + } + } + + impl PartialEq for Foo + where + for<'a> &'a str: PartialEq, + { + fn eq(&self, other: &T) -> bool { + (&self.as_str()).eq(other) + } + } +} + fn main() {}