about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-03-16 15:54:04 +0000
committerbors <bors@rust-lang.org>2024-03-16 15:54:04 +0000
commit28fd269a50a79280e7576986e11081d987072c10 (patch)
tree415a0243e9adde106d9b6ab644addf5d620aee1b
parent2bcca41ad81146744295fada789a2699c72f69ac (diff)
parent046eb84794947d4abfebc0ae329c4f951fef880c (diff)
downloadrust-28fd269a50a79280e7576986e11081d987072c10.tar.gz
rust-28fd269a50a79280e7576986e11081d987072c10.zip
Auto merge of #122510 - flip1995:clippy_backport, r=Mark-Simulacrum
[beta] Clippy backports

Backports included in this PR:

- https://github.com/rust-lang/rust-clippy/pull/12276 Fixing the lint on some platforms before hitting stable
- https://github.com/rust-lang/rust-clippy/pull/12405 Respect MSRV before hitting stable
- https://github.com/rust-lang/rust-clippy/pull/12266 Fixing an (unlikely) ICE
- https://github.com/rust-lang/rust-clippy/pull/12177 Fixing FPs before hitting stable

Each backport on its own might not warrant a backport, but the collection of those are nice QoL fixes for Clippy users, before those bugs hit stable.

All of those commits are already on `master`.

r? `@Mark-Simulacrum`
-rw-r--r--src/tools/clippy/clippy_config/src/msrvs.rs1
-rw-r--r--src/tools/clippy/clippy_lints/src/indexing_slicing.rs1
-rw-r--r--src/tools/clippy/clippy_lints/src/thread_local_initializer_can_be_made_const.rs67
-rw-r--r--src/tools/clippy/clippy_lints/src/unconditional_recursion.rs86
-rw-r--r--src/tools/clippy/tests/ui/crashes/ice-12253.rs5
-rw-r--r--src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.fixed14
-rw-r--r--src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.rs14
-rw-r--r--src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.stderr14
-rw-r--r--src/tools/clippy/tests/ui/unconditional_recursion.rs59
9 files changed, 200 insertions, 61 deletions
diff --git a/src/tools/clippy/clippy_config/src/msrvs.rs b/src/tools/clippy/clippy_config/src/msrvs.rs
index 72d5b9aff28..0c6368d33f4 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 391db0b0df7..35fcd8cdd35 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 9fee4c06200..1af3733ebfa 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<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,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 209035804e4..224ec475c51 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<DefId> {
-    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<DefId> {
     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 `<T as PartialEq<T>>::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 `<T as PartialEq<T>>::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 00000000000..41f50035144
--- /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 bbde25b0a88..a6ed59d49c5 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 3d7aacf2f09..3f0159c5806 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 b35bd306b52..2cb51850c48 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<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`
+  --> $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 7b898a6e0e7..263fdf26d4d 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>(T);
+
+    impl<T> std::ops::Deref for MyBox<T> {
+        type Target = T;
+        fn deref(&self) -> &T {
+            &self.0
+        }
+    }
+
+    impl<T: PartialEq> PartialEq for MyBox<T> {
+        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<T> 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<HeaderName> 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<T> PartialEq<T> for Foo
+    where
+        for<'a> &'a str: PartialEq<T>,
+    {
+        fn eq(&self, other: &T) -> bool {
+            (&self.as_str()).eq(other)
+        }
+    }
+}
+
 fn main() {}