Auto merge of #129618 - Jarcho:backports, r=Mark-Simulacrum

[beta] Clippy backports r? `@Mark-Simulacrum` Backports: - rust-lang/rust-clippy#12892 - rust-lang/rust-clippy#13168 - rust-lang/rust-clippy#13290 Both 12892 and 13290 are fixes for stable regressions. The first is a little large, but mostly just reverts the change that caused the regression. 13168 is to handle the `Error` trait moving to core in 1.81.
rust-lang · Aug 28, 2024 · 4a765c0 · 4a765c0
2 parents b5fd9f6 + 1f4f9d8
commit 4a765c0
Show file tree

Hide file tree

Showing 11 changed files with 165 additions and 31 deletions.
diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs
@@ -1034,7 +1034,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
             allow_comparison_to_zero,
         ))
     });
-    store.register_late_pass(|_| Box::<std_instead_of_core::StdReexports>::default());
+    store.register_late_pass(move |_| Box::new(std_instead_of_core::StdReexports::new(conf)));
     store.register_late_pass(move |_| Box::new(instant_subtraction::InstantSubtraction::new(msrv())));
     store.register_late_pass(|_| Box::new(partialeq_to_none::PartialeqToNone));
     store.register_late_pass(move |_| Box::new(manual_clamp::ManualClamp::new(msrv())));

diff --git a/src/tools/clippy/clippy_lints/src/needless_borrows_for_generic_args.rs b/src/tools/clippy/clippy_lints/src/needless_borrows_for_generic_args.rs
@@ -1,6 +1,6 @@
 use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::mir::PossibleBorrowerMap;
+use clippy_utils::mir::{enclosing_mir, expr_local, local_assignments, used_exactly_once, PossibleBorrowerMap};
 use clippy_utils::source::snippet_with_context;
 use clippy_utils::ty::{implements_trait, is_copy};
 use clippy_utils::{expr_use_ctxt, peel_n_hir_expr_refs, DefinedTy, ExprUseNode};
@@ -11,6 +11,7 @@ use rustc_hir::{Body, Expr, ExprKind, Mutability, Path, QPath};
 use rustc_index::bit_set::BitSet;
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::mir::{Rvalue, StatementKind};
 use rustc_middle::ty::{
     self, ClauseKind, EarlyBinder, FnSig, GenericArg, GenericArgKind, ParamTy, ProjectionPredicate, Ty,
 };
@@ -107,6 +108,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowsForGenericArgs<'tcx> {
             }
             && let count = needless_borrow_count(
                 cx,
+                &mut self.possible_borrowers,
                 fn_id,
                 cx.typeck_results().node_args(hir_id),
                 i,
@@ -155,9 +157,14 @@ fn path_has_args(p: &QPath<'_>) -> bool {
 /// The following constraints will be checked:
 /// * The borrowed expression meets all the generic type's constraints.
 /// * The generic type appears only once in the functions signature.
-/// * The borrowed value is Copy itself OR not a variable (created by a function call)
+/// * The borrowed value is:
+///   - `Copy` itself, or
+///   - the only use of a mutable reference, or
+///   - not a variable (created by a function call)
+#[expect(clippy::too_many_arguments)]
 fn needless_borrow_count<'tcx>(
     cx: &LateContext<'tcx>,
+    possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
     fn_id: DefId,
     callee_args: ty::GenericArgsRef<'tcx>,
     arg_index: usize,
@@ -232,9 +239,9 @@ fn needless_borrow_count<'tcx>(
 
         let referent_ty = cx.typeck_results().expr_ty(referent);
 
-        if (!is_copy(cx, referent_ty) && !referent_ty.is_ref())
-            && let ExprKind::AddrOf(_, _, inner) = reference.kind
-            && !matches!(inner.kind, ExprKind::Call(..) | ExprKind::MethodCall(..))
+        if !(is_copy(cx, referent_ty)
+            || referent_ty.is_ref() && referent_used_exactly_once(cx, possible_borrowers, reference)
+            || matches!(referent.kind, ExprKind::Call(..) | ExprKind::MethodCall(..)))
         {
             return false;
         }
@@ -337,6 +344,37 @@ fn is_mixed_projection_predicate<'tcx>(
     }
 }
 
+fn referent_used_exactly_once<'tcx>(
+    cx: &LateContext<'tcx>,
+    possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
+    reference: &Expr<'tcx>,
+) -> bool {
+    if let Some(mir) = enclosing_mir(cx.tcx, reference.hir_id)
+        && let Some(local) = expr_local(cx.tcx, reference)
+        && let [location] = *local_assignments(mir, local).as_slice()
+        && let block_data = &mir.basic_blocks[location.block]
+        && let Some(statement) = block_data.statements.get(location.statement_index)
+        && let StatementKind::Assign(box (_, Rvalue::Ref(_, _, place))) = statement.kind
+        && !place.is_indirect_first_projection()
+    {
+        let body_owner_local_def_id = cx.tcx.hir().enclosing_body_owner(reference.hir_id);
+        if possible_borrowers
+            .last()
+            .map_or(true, |&(local_def_id, _)| local_def_id != body_owner_local_def_id)
+        {
+            possible_borrowers.push((body_owner_local_def_id, PossibleBorrowerMap::new(cx, mir)));
+        }
+        let possible_borrower = &mut possible_borrowers.last_mut().unwrap().1;
+        // If `only_borrowers` were used here, the `copyable_iterator::warn` test would fail. The reason is
+        // that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible borrower of
+        // itself. See the comment in that method for an explanation as to why.
+        possible_borrower.bounded_borrowers(&[local], &[local, place.local], place.local, location)
+            && used_exactly_once(mir, place.local).unwrap_or(false)
+    } else {
+        false
+    }
+}
+
 // Iteratively replaces `param_ty` with `new_ty` in `args`, and similarly for each resulting
 // projected type that is a type parameter. Returns `false` if replacing the types would have an
 // effect on the function signature beyond substituting `new_ty` for `param_ty`.

diff --git a/src/tools/clippy/clippy_lints/src/non_copy_const.rs b/src/tools/clippy/clippy_lints/src/non_copy_const.rs
@@ -309,7 +309,7 @@ impl<'tcx> NonCopyConst<'tcx> {
 
 impl<'tcx> LateLintPass<'tcx> for NonCopyConst<'tcx> {
     fn check_crate(&mut self, cx: &LateContext<'tcx>) {
-        self.interior_mut = InteriorMut::new(cx, &self.ignore_interior_mutability);
+        self.interior_mut = InteriorMut::without_pointers(cx, &self.ignore_interior_mutability);
     }
 
     fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx Item<'_>) {

diff --git a/src/tools/clippy/clippy_lints/src/std_instead_of_core.rs b/src/tools/clippy/clippy_lints/src/std_instead_of_core.rs
@@ -1,11 +1,15 @@
+use clippy_config::msrvs::Msrv;
+use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::is_from_proc_macro;
+use rustc_attr::{StabilityLevel, StableSince};
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
 use rustc_hir::def_id::DefId;
 use rustc_hir::{HirId, Path, PathSegment};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
+use rustc_semver::RustcVersion;
 use rustc_session::impl_lint_pass;
 use rustc_span::symbol::kw;
 use rustc_span::{sym, Span};
@@ -66,6 +70,10 @@ declare_clippy_lint! {
     /// imported from core to ensure disabling `alloc` does not cause the crate to fail to compile. This lint
     /// is also useful for crates migrating to become `no_std` compatible.
     ///
+    /// ### Known problems
+    /// The lint is only partially aware of the required MSRV for items that were originally in `std` but moved
+    /// to `core`.
+    ///
     /// ### Example
     /// ```no_run
     /// # extern crate alloc;
@@ -81,20 +89,30 @@ declare_clippy_lint! {
     "type is imported from alloc when available in core"
 }
 
-#[derive(Default)]
 pub struct StdReexports {
     // Paths which can be either a module or a macro (e.g. `std::env`) will cause this check to happen
     // twice. First for the mod, second for the macro. This is used to avoid the lint reporting for the macro
     // when the path could be also be used to access the module.
     prev_span: Span,
+    msrv: Msrv,
+}
+
+impl StdReexports {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self {
+            prev_span: Span::default(),
+            msrv: conf.msrv.clone(),
+        }
+    }
 }
+
 impl_lint_pass!(StdReexports => [STD_INSTEAD_OF_CORE, STD_INSTEAD_OF_ALLOC, ALLOC_INSTEAD_OF_CORE]);
 
 impl<'tcx> LateLintPass<'tcx> for StdReexports {
     fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) {
         if let Res::Def(_, def_id) = path.res
             && let Some(first_segment) = get_first_segment(path)
-            && is_stable(cx, def_id)
+            && is_stable(cx, def_id, &self.msrv)
             && !in_external_macro(cx.sess(), path.span)
             && !is_from_proc_macro(cx, &first_segment.ident)
         {
@@ -131,6 +149,8 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports {
             }
         }
     }
+
+    extract_msrv_attr!(LateContext);
 }
 
 /// Returns the first named segment of a [`Path`].
@@ -146,16 +166,29 @@ fn get_first_segment<'tcx>(path: &Path<'tcx>) -> Option<&'tcx PathSegment<'tcx>>
     }
 }
 
-/// Checks if all ancestors of `def_id` are stable, to avoid linting
-/// [unstable moves](https://github.com/rust-lang/rust/pull/95956)
-fn is_stable(cx: &LateContext<'_>, mut def_id: DefId) -> bool {
+/// Checks if all ancestors of `def_id` meet `msrv` to avoid linting [unstable moves](https://github.com/rust-lang/rust/pull/95956)
+/// or now stable moves that were once unstable.
+///
+/// Does not catch individually moved items
+fn is_stable(cx: &LateContext<'_>, mut def_id: DefId, msrv: &Msrv) -> bool {
     loop {
-        if cx
-            .tcx
-            .lookup_stability(def_id)
-            .map_or(false, |stability| stability.is_unstable())
+        if let Some(stability) = cx.tcx.lookup_stability(def_id)
+            && let StabilityLevel::Stable {
+                since,
+                allowed_through_unstable_modules: false,
+            } = stability.level
         {
-            return false;
+            let stable = match since {
+                StableSince::Version(v) => {
+                    msrv.meets(RustcVersion::new(v.major.into(), v.minor.into(), v.patch.into()))
+                },
+                StableSince::Current => msrv.current().is_none(),
+                StableSince::Err => false,
+            };
+
+            if !stable {
+                return false;
+            }
         }
 
         match cx.tcx.opt_parent(def_id) {

diff --git a/src/tools/clippy/tests/ui/declare_interior_mutable_const/others.rs b/src/tools/clippy/tests/ui/declare_interior_mutable_const/others.rs
@@ -3,6 +3,7 @@
 use std::borrow::Cow;
 use std::cell::Cell;
 use std::fmt::Display;
+use std::ptr;
 use std::sync::atomic::AtomicUsize;
 use std::sync::Once;
 
@@ -53,4 +54,20 @@ mod issue_8493 {
     issue_8493!();
 }
 
+#[repr(C, align(8))]
+struct NoAtomic(usize);
+#[repr(C, align(8))]
+struct WithAtomic(AtomicUsize);
+
+const fn with_non_null() -> *const WithAtomic {
+    const NO_ATOMIC: NoAtomic = NoAtomic(0);
+    (&NO_ATOMIC as *const NoAtomic).cast()
+}
+const WITH_ATOMIC: *const WithAtomic = with_non_null();
+
+struct Generic<T>(T);
+impl<T> Generic<T> {
+    const RAW_POINTER: *const Cell<T> = ptr::null();
+}
+
 fn main() {}
diff --git a/src/tools/clippy/tests/ui/declare_interior_mutable_const/others.stderr b/src/tools/clippy/tests/ui/declare_interior_mutable_const/others.stderr
@@ -1,5 +1,5 @@
 error: a `const` item should not be interior mutable
-  --> tests/ui/declare_interior_mutable_const/others.rs:9:1
+  --> tests/ui/declare_interior_mutable_const/others.rs:10:1
    |
 LL | const ATOMIC: AtomicUsize = AtomicUsize::new(5);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -9,23 +9,23 @@ LL | const ATOMIC: AtomicUsize = AtomicUsize::new(5);
    = help: to override `-D warnings` add `#[allow(clippy::declare_interior_mutable_const)]`
 
 error: a `const` item should not be interior mutable
-  --> tests/ui/declare_interior_mutable_const/others.rs:10:1
+  --> tests/ui/declare_interior_mutable_const/others.rs:11:1
    |
 LL | const CELL: Cell<usize> = Cell::new(6);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider making this `Sync` so that it can go in a static item or using a `thread_local`
 
 error: a `const` item should not be interior mutable
-  --> tests/ui/declare_interior_mutable_const/others.rs:11:1
+  --> tests/ui/declare_interior_mutable_const/others.rs:12:1
    |
 LL | const ATOMIC_TUPLE: ([AtomicUsize; 1], Vec<AtomicUsize>, u8) = ([ATOMIC], Vec::new(), 7);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider making this a static item
 
 error: a `const` item should not be interior mutable
-  --> tests/ui/declare_interior_mutable_const/others.rs:16:9
+  --> tests/ui/declare_interior_mutable_const/others.rs:17:9
    |
 LL |         const $name: $ty = $e;
    |         ^^^^^^^^^^^^^^^^^^^^^^
@@ -36,7 +36,7 @@ LL | declare_const!(_ONCE: Once = Once::new());
    = note: this error originates in the macro `declare_const` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: a `const` item should not be interior mutable
-  --> tests/ui/declare_interior_mutable_const/others.rs:44:13
+  --> tests/ui/declare_interior_mutable_const/others.rs:45:13
    |
 LL |             const _BAZ: Cell<usize> = Cell::new(0);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff --git a/src/tools/clippy/tests/ui/needless_borrows_for_generic_args.fixed b/src/tools/clippy/tests/ui/needless_borrows_for_generic_args.fixed
@@ -333,4 +333,12 @@ fn main() {
         f(&y); // Don't lint
         f("".to_owned()); // Lint
     }
+    {
+        fn takes_writer<T: std::io::Write>(_: T) {}
+
+        fn issue_12856(mut buffer: &mut Vec<u8>) {
+            takes_writer(&mut buffer); // Don't lint, would make buffer unavailable later
+            buffer.extend(b"\n");
+        }
+    }
 }
diff --git a/src/tools/clippy/tests/ui/needless_borrows_for_generic_args.rs b/src/tools/clippy/tests/ui/needless_borrows_for_generic_args.rs
@@ -333,4 +333,12 @@ fn main() {
         f(&y); // Don't lint
         f(&"".to_owned()); // Lint
     }
+    {
+        fn takes_writer<T: std::io::Write>(_: T) {}
+
+        fn issue_12856(mut buffer: &mut Vec<u8>) {
+            takes_writer(&mut buffer); // Don't lint, would make buffer unavailable later
+            buffer.extend(b"\n");
+        }
+    }
 }
diff --git a/src/tools/clippy/tests/ui/std_instead_of_core.fixed b/src/tools/clippy/tests/ui/std_instead_of_core.fixed
@@ -75,8 +75,17 @@ mod std_in_proc_macro_derive {
     struct B {}
 }
 
-fn main() {
-    std_instead_of_core();
-    std_instead_of_alloc();
-    alloc_instead_of_core();
+// Some intrinsics are usable on stable but live in an unstable module, but should still suggest
+// replacing std -> core
+fn intrinsic(a: *mut u8, b: *mut u8) {
+    unsafe {
+        core::intrinsics::copy(a, b, 1);
+        //~^ std_instead_of_core
+    }
 }
+
+#[clippy::msrv = "1.76"]
+fn msrv_1_76(_: std::net::IpAddr) {}
+
+#[clippy::msrv = "1.77"]
+fn msrv_1_77(_: core::net::IpAddr) {}
diff --git a/src/tools/clippy/tests/ui/std_instead_of_core.rs b/src/tools/clippy/tests/ui/std_instead_of_core.rs
@@ -75,8 +75,17 @@ mod std_in_proc_macro_derive {
     struct B {}
 }
 
-fn main() {
-    std_instead_of_core();
-    std_instead_of_alloc();
-    alloc_instead_of_core();
+// Some intrinsics are usable on stable but live in an unstable module, but should still suggest
+// replacing std -> core
+fn intrinsic(a: *mut u8, b: *mut u8) {
+    unsafe {
+        std::intrinsics::copy(a, b, 1);
+        //~^ std_instead_of_core
+    }
 }
+
+#[clippy::msrv = "1.76"]
+fn msrv_1_76(_: std::net::IpAddr) {}
+
+#[clippy::msrv = "1.77"]
+fn msrv_1_77(_: std::net::IpAddr) {}
diff --git a/src/tools/clippy/tests/ui/std_instead_of_core.stderr b/src/tools/clippy/tests/ui/std_instead_of_core.stderr
@@ -85,5 +85,17 @@ LL |     use alloc::slice::from_ref;
    = note: `-D clippy::alloc-instead-of-core` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::alloc_instead_of_core)]`
 
-error: aborting due to 13 previous errors
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:82:9
+   |
+LL |         std::intrinsics::copy(a, b, 1);
+   |         ^^^ help: consider importing the item from `core`: `core`
+
+error: used import from `std` instead of `core`
+  --> tests/ui/std_instead_of_core.rs:91:17
+   |
+LL | fn msrv_1_77(_: std::net::IpAddr) {}
+   |                 ^^^ help: consider importing the item from `core`: `core`
+
+error: aborting due to 15 previous errors