Auto merge of #14789 - HKalbasi:mir, r=HKalbasi

Add `moved-out-of-ref` diagnostic

This is marked as experimental, since spans are terrible, there are some false positives due #14754, and it doesn't play well with unknown types. But I hope we can soon lift it.

11 files changed, 352 insertions, 10 deletions

diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs
index 3261c22313b..b800c2e32a9 100644
--- a/crates/hir-ty/src/infer/expr.rs
+++ b/crates/hir-ty/src/infer/expr.rs
@@ -934,7 +934,18 @@ impl<'a> InferenceContext<'a> {
         match fn_x {
             FnTrait::FnOnce => (),
             FnTrait::FnMut => {
-                if !matches!(derefed_callee.kind(Interner), TyKind::Ref(Mutability::Mut, _, _)) {
+                if let TyKind::Ref(Mutability::Mut, _, inner) = derefed_callee.kind(Interner) {
+                    if adjustments
+                        .last()
+                        .map(|x| matches!(x.kind, Adjust::Borrow(_)))
+                        .unwrap_or(true)
+                    {
+                        // prefer reborrow to move
+                        adjustments
+                            .push(Adjustment { kind: Adjust::Deref(None), target: inner.clone() });
+                        adjustments.push(Adjustment::borrow(Mutability::Mut, inner.clone()))
+                    }
+                } else {
                     adjustments.push(Adjustment::borrow(Mutability::Mut, derefed_callee.clone()));
                 }
             }
diff --git a/crates/hir-ty/src/mir/borrowck.rs b/crates/hir-ty/src/mir/borrowck.rs
index a6af4e75d45..412390d3fa1 100644
--- a/crates/hir-ty/src/mir/borrowck.rs
+++ b/crates/hir-ty/src/mir/borrowck.rs
@@ -5,12 +5,14 @@
 
 use std::iter;
 
-use hir_def::DefWithBodyId;
+use hir_def::{DefWithBodyId, HasModule};
 use la_arena::ArenaMap;
 use stdx::never;
 use triomphe::Arc;
 
-use crate::{db::HirDatabase, ClosureId};
+use crate::{
+    db::HirDatabase, mir::Operand, utils::ClosureSubst, ClosureId, Interner, Ty, TyExt, TypeFlags,
+};
 
 use super::{
     BasicBlockId, BorrowKind, LocalId, MirBody, MirLowerError, MirSpan, Place, ProjectionElem,
@@ -25,9 +27,16 @@ pub enum MutabilityReason {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
+pub struct MovedOutOfRef {
+    pub ty: Ty,
+    pub span: MirSpan,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct BorrowckResult {
     pub mir_body: Arc<MirBody>,
     pub mutability_of_locals: ArenaMap<LocalId, MutabilityReason>,
+    pub moved_out_of_ref: Vec<MovedOutOfRef>,
 }
 
 fn all_mir_bodies(
@@ -68,12 +77,115 @@ pub fn borrowck_query(
     let r = all_mir_bodies(db, def)
         .map(|body| {
             let body = body?;
-            Ok(BorrowckResult { mutability_of_locals: mutability_of_locals(&body), mir_body: body })
+            Ok(BorrowckResult {
+                mutability_of_locals: mutability_of_locals(&body),
+                moved_out_of_ref: moved_out_of_ref(db, &body),
+                mir_body: body,
+            })
         })
         .collect::<Result<Vec<_>, MirLowerError>>()?;
     Ok(r.into())
 }
 
+fn moved_out_of_ref(db: &dyn HirDatabase, body: &MirBody) -> Vec<MovedOutOfRef> {
+    let mut result = vec![];
+    let mut for_operand = |op: &Operand, span: MirSpan| match op {
+        Operand::Copy(p) | Operand::Move(p) => {
+            let mut ty: Ty = body.locals[p.local].ty.clone();
+            let mut is_dereference_of_ref = false;
+            for proj in &p.projection {
+                if *proj == ProjectionElem::Deref && ty.as_reference().is_some() {
+                    is_dereference_of_ref = true;
+                }
+                ty = proj.projected_ty(
+                    ty,
+                    db,
+                    |c, subst, f| {
+                        let (def, _) = db.lookup_intern_closure(c.into());
+                        let infer = db.infer(def);
+                        let (captures, _) = infer.closure_info(&c);
+                        let parent_subst = ClosureSubst(subst).parent_subst();
+                        captures
+                            .get(f)
+                            .expect("broken closure field")
+                            .ty
+                            .clone()
+                            .substitute(Interner, parent_subst)
+                    },
+                    body.owner.module(db.upcast()).krate(),
+                );
+            }
+            if is_dereference_of_ref
+                && !ty.clone().is_copy(db, body.owner)
+                && !ty.data(Interner).flags.intersects(TypeFlags::HAS_ERROR)
+            {
+                result.push(MovedOutOfRef { span, ty });
+            }
+        }
+        Operand::Constant(_) | Operand::Static(_) => (),
+    };
+    for (_, block) in body.basic_blocks.iter() {
+        for statement in &block.statements {
+            match &statement.kind {
+                StatementKind::Assign(_, r) => match r {
+                    Rvalue::ShallowInitBoxWithAlloc(_) => (),
+                    Rvalue::ShallowInitBox(o, _)
+                    | Rvalue::UnaryOp(_, o)
+                    | Rvalue::Cast(_, o, _)
+                    | Rvalue::Repeat(o, _)
+                    | Rvalue::Use(o) => for_operand(o, statement.span),
+                    Rvalue::CopyForDeref(_)
+                    | Rvalue::Discriminant(_)
+                    | Rvalue::Len(_)
+                    | Rvalue::Ref(_, _) => (),
+                    Rvalue::CheckedBinaryOp(_, o1, o2) => {
+                        for_operand(o1, statement.span);
+                        for_operand(o2, statement.span);
+                    }
+                    Rvalue::Aggregate(_, ops) => {
+                        for op in ops {
+                            for_operand(op, statement.span);
+                        }
+                    }
+                },
+                StatementKind::Deinit(_)
+                | StatementKind::StorageLive(_)
+                | StatementKind::StorageDead(_)
+                | StatementKind::Nop => (),
+            }
+        }
+        match &block.terminator {
+            Some(terminator) => match &terminator.kind {
+                TerminatorKind::SwitchInt { discr, .. } => for_operand(discr, terminator.span),
+                TerminatorKind::FalseEdge { .. }
+                | TerminatorKind::FalseUnwind { .. }
+                | TerminatorKind::Goto { .. }
+                | TerminatorKind::Resume
+                | TerminatorKind::GeneratorDrop
+                | TerminatorKind::Abort
+                | TerminatorKind::Return
+                | TerminatorKind::Unreachable
+                | TerminatorKind::Drop { .. } => (),
+                TerminatorKind::DropAndReplace { value, .. } => {
+                    for_operand(value, terminator.span);
+                }
+                TerminatorKind::Call { func, args, .. } => {
+                    for_operand(func, terminator.span);
+                    args.iter().for_each(|x| for_operand(x, terminator.span));
+                }
+                TerminatorKind::Assert { cond, .. } => {
+                    for_operand(cond, terminator.span);
+                }
+                TerminatorKind::Yield { value, .. } => {
+                    for_operand(value, terminator.span);
+                }
+            },
+            None => (),
+        }
+    }
+    result
+}
+
 fn is_place_direct(lvalue: &Place) -> bool {
     !lvalue.projection.iter().any(|x| *x == ProjectionElem::Deref)
 }
diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
index f81f8b0b011..10893b62bfb 100644
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs
@@ -46,6 +46,7 @@ diagnostics![
     MissingFields,
     MissingMatchArms,
     MissingUnsafe,
+    MovedOutOfRef,
     NeedMut,
     NoSuchField,
     PrivateAssocItem,
@@ -252,3 +253,9 @@ pub struct NeedMut {
 pub struct UnusedMut {
     pub local: Local,
 }
+
+#[derive(Debug)]
+pub struct MovedOutOfRef {
+    pub ty: Type,
+    pub span: InFile<SyntaxNodePtr>,
+}
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 42047446144..bc925552f34 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -90,10 +90,11 @@ pub use crate::{
         AnyDiagnostic, BreakOutsideOfLoop, ExpectedFunction, InactiveCode, IncoherentImpl,
         IncorrectCase, InvalidDeriveTarget, MacroDefError, MacroError, MacroExpansionParseError,
         MalformedDerive, MismatchedArgCount, MissingFields, MissingMatchArms, MissingUnsafe,
-        NeedMut, NoSuchField, PrivateAssocItem, PrivateField, ReplaceFilterMapNextWithFindMap,
-        TypeMismatch, UndeclaredLabel, UnimplementedBuiltinMacro, UnreachableLabel,
-        UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall,
-        UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro, UnusedMut,
+        MovedOutOfRef, NeedMut, NoSuchField, PrivateAssocItem, PrivateField,
+        ReplaceFilterMapNextWithFindMap, TypeMismatch, UndeclaredLabel, UnimplementedBuiltinMacro,
+        UnreachableLabel, UnresolvedExternCrate, UnresolvedField, UnresolvedImport,
+        UnresolvedMacroCall, UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro,
+        UnusedMut,
     },
     has_source::HasSource,
     semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits},
@@ -1575,6 +1576,26 @@ impl DefWithBody {
         if let Ok(borrowck_results) = db.borrowck(self.into()) {
             for borrowck_result in borrowck_results.iter() {
                 let mir_body = &borrowck_result.mir_body;
+                for moof in &borrowck_result.moved_out_of_ref {
+                    let span: InFile<SyntaxNodePtr> = match moof.span {
+                        mir::MirSpan::ExprId(e) => match source_map.expr_syntax(e) {
+                            Ok(s) => s.map(|x| x.into()),
+                            Err(_) => continue,
+                        },
+                        mir::MirSpan::PatId(p) => match source_map.pat_syntax(p) {
+                            Ok(s) => s.map(|x| match x {
+                                Either::Left(e) => e.into(),
+                                Either::Right(e) => e.into(),
+                            }),
+                            Err(_) => continue,
+                        },
+                        mir::MirSpan::Unknown => continue,
+                    };
+                    acc.push(
+                        MovedOutOfRef { ty: Type::new_for_crate(krate, moof.ty.clone()), span }
+                            .into(),
+                    )
+                }
                 let mol = &borrowck_result.mutability_of_locals;
                 for (binding_id, _) in hir_body.bindings.iter() {
                     let Some(&local) = mir_body.binding_locals.get(binding_id) else {
diff --git a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs
index b5e619e2a03..3f13b97a447 100644
--- a/crates/ide-diagnostics/src/handlers/missing_match_arms.rs
+++ b/crates/ide-diagnostics/src/handlers/missing_match_arms.rs
@@ -1027,6 +1027,7 @@ fn main() {
 
             check_diagnostics(
                 r#"
+//- minicore: copy
 fn main() {
     match &false {
         &true => {}
@@ -1041,6 +1042,7 @@ fn main() {
             cov_mark::check_count!(validate_match_bailed_out, 1);
             check_diagnostics(
                 r#"
+//- minicore: copy
 fn main() {
     match (&false,) {
         //^^^^^^^^^ error: missing match arm: `(&false,)` not covered
diff --git a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs
index d73f4e7721c..2026b6fcef5 100644
--- a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs
+++ b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs
@@ -142,6 +142,8 @@ fn main() {
     fn missing_unsafe_diagnostic_with_static_mut() {
         check_diagnostics(
             r#"
+//- minicore: copy
+
 struct Ty {
     a: u8,
 }
@@ -256,6 +258,7 @@ fn main() {
     fn add_unsafe_block_when_accessing_mutable_static() {
         check_fix(
             r#"
+//- minicore: copy
 struct Ty {
     a: u8,
 }
@@ -374,6 +377,7 @@ fn main() {
     fn unsafe_expr_as_right_hand_side_of_assignment() {
         check_fix(
             r#"
+//- minicore: copy
 static mut STATIC_MUT: u8 = 0;
 
 fn main() {
@@ -396,6 +400,7 @@ fn main() {
     fn unsafe_expr_in_binary_plus() {
         check_fix(
             r#"
+//- minicore: copy
 static mut STATIC_MUT: u8 = 0;
 
 fn main() {
diff --git a/crates/ide-diagnostics/src/handlers/moved_out_of_ref.rs b/crates/ide-diagnostics/src/handlers/moved_out_of_ref.rs
new file mode 100644
index 00000000000..99243a5ab86
--- /dev/null
+++ b/crates/ide-diagnostics/src/handlers/moved_out_of_ref.rs
@@ -0,0 +1,154 @@
+use crate::{Diagnostic, DiagnosticsContext};
+use hir::HirDisplay;
+
+// Diagnostic: moved-out-of-ref
+//
+// This diagnostic is triggered on moving non copy things out of references.
+pub(crate) fn moved_out_of_ref(ctx: &DiagnosticsContext<'_>, d: &hir::MovedOutOfRef) -> Diagnostic {
+    Diagnostic::new(
+        "moved-out-of-ref",
+        format!("cannot move `{}` out of reference", d.ty.display(ctx.sema.db)),
+        ctx.sema.diagnostics_display_range(d.span.clone()).range,
+    )
+    .experimental() // spans are broken, and I'm not sure how precise we can detect copy types
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::check_diagnostics;
+
+    // FIXME: spans are broken
+
+    #[test]
+    fn move_by_explicit_deref() {
+        check_diagnostics(
+            r#"
+struct X;
+fn main() {
+    let a = &X;
+    let b = *a;
+      //^ error: cannot move `X` out of reference
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn move_out_of_field() {
+        check_diagnostics(
+            r#"
+//- minicore: copy
+struct X;
+struct Y(X, i32);
+fn main() {
+    let a = &Y(X, 5);
+    let b = a.0;
+      //^ error: cannot move `X` out of reference
+    let y = a.1;
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn move_out_of_static() {
+        check_diagnostics(
+            r#"
+//- minicore: copy
+struct X;
+fn main() {
+    static S: X = X;
+    let s = S;
+      //^ error: cannot move `X` out of reference
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn generic_types() {
+        check_diagnostics(
+            r#"
+//- minicore: derive, copy
+
+#[derive(Copy)]
+struct X<T>(T);
+struct Y;
+
+fn consume<T>(_: X<T>) {
+
+}
+
+fn main() {
+    let a = &X(Y);
+    consume(*a);
+  //^^^^^^^^^^^ error: cannot move `X<Y>` out of reference
+    let a = &X(5);
+    consume(*a);
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn no_false_positive_simple() {
+        check_diagnostics(
+            r#"
+//- minicore: copy
+fn f(_: i32) {}
+fn main() {
+    let x = &2;
+    f(*x);
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn no_false_positive_unknown_type() {
+        check_diagnostics(
+            r#"
+//- minicore: derive, copy
+fn f(x: &Unknown) -> Unknown {
+    *x
+}
+
+#[derive(Copy)]
+struct X<T>(T);
+
+struct Y<T>(T);
+
+fn g(x: &X<Unknown>) -> X<Unknown> {
+    *x
+}
+
+fn h(x: &Y<Unknown>) -> Y<Unknown> {
+    // FIXME: we should show error for this, as `Y` is not copy
+    // regardless of its generic parameter.
+    *x
+}
+
+"#,
+        );
+    }
+
+    #[test]
+    fn no_false_positive_dyn_fn() {
+        check_diagnostics(
+            r#"
+//- minicore: copy, fn
+fn f(x: &mut &mut dyn Fn()) {
+    x();
+}
+
+struct X<'a> {
+    field: &'a mut dyn Fn(),
+}
+
+fn f(x: &mut X<'_>) {
+    (x.field)();
+}
+"#,
+        );
+    }
+}
diff --git a/crates/ide-diagnostics/src/handlers/mutability_errors.rs b/crates/ide-diagnostics/src/handlers/mutability_errors.rs
index d60007b48a6..9d6a862cb11 100644
--- a/crates/ide-diagnostics/src/handlers/mutability_errors.rs
+++ b/crates/ide-diagnostics/src/handlers/mutability_errors.rs
@@ -340,6 +340,7 @@ fn main() {
     fn regression_14310() {
         check_diagnostics(
             r#"
+            //- minicore: copy, builtin_impls
             fn clone(mut i: &!) -> ! {
                    //^^^^^ 💡 weak: variable does not need to be mutable
                 *i
diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs
index 25d3568950d..048dedf6bd1 100644
--- a/crates/ide-diagnostics/src/lib.rs
+++ b/crates/ide-diagnostics/src/lib.rs
@@ -38,6 +38,7 @@ mod handlers {
     pub(crate) mod missing_fields;
     pub(crate) mod missing_match_arms;
     pub(crate) mod missing_unsafe;
+    pub(crate) mod moved_out_of_ref;
     pub(crate) mod mutability_errors;
     pub(crate) mod no_such_field;
     pub(crate) mod private_assoc_item;
@@ -283,6 +284,7 @@ pub fn diagnostics(
             AnyDiagnostic::MissingFields(d) => handlers::missing_fields::missing_fields(&ctx, &d),
             AnyDiagnostic::MissingMatchArms(d) => handlers::missing_match_arms::missing_match_arms(&ctx, &d),
             AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&ctx, &d),
+            AnyDiagnostic::MovedOutOfRef(d) => handlers::moved_out_of_ref::moved_out_of_ref(&ctx, &d),
             AnyDiagnostic::NeedMut(d) => handlers::mutability_errors::need_mut(&ctx, &d),
             AnyDiagnostic::NoSuchField(d) => handlers::no_such_field::no_such_field(&ctx, &d),
             AnyDiagnostic::PrivateAssocItem(d) => handlers::private_assoc_item::private_assoc_item(&ctx, &d),
diff --git a/crates/ide-diagnostics/src/tests.rs b/crates/ide-diagnostics/src/tests.rs
index 413689561bc..b5cd4e0d689 100644
--- a/crates/ide-diagnostics/src/tests.rs
+++ b/crates/ide-diagnostics/src/tests.rs
@@ -121,6 +121,15 @@ pub(crate) fn check_diagnostics_with_config(config: DiagnosticsConfig, ra_fixtur
             })
             .collect::<Vec<_>>();
         actual.sort_by_key(|(range, _)| range.start());
+        if expected.is_empty() {
+            // makes minicore smoke test debugable
+            for (e, _) in &actual {
+                eprintln!(
+                    "Code in range {e:?} = {}",
+                    &db.file_text(file_id)[usize::from(e.start())..usize::from(e.end())]
+                )
+            }
+        }
         assert_eq!(expected, actual);
     }
 }
@@ -156,6 +165,11 @@ fn minicore_smoke_test() {
 
     // Checks that there is no diagnostic in minicore for each flag.
     for flag in MiniCore::available_flags() {
+        if flag == "clone" {
+            // Clone without copy has `moved-out-of-ref`, so ignoring.
+            // FIXME: Maybe we should merge copy and clone in a single flag?
+            continue;
+        }
         eprintln!("Checking minicore flag {flag}");
         check(MiniCore::from_flags([flag]));
     }
diff --git a/crates/test-utils/src/minicore.rs b/crates/test-utils/src/minicore.rs
index 22cef049838..6d6c9af7f04 100644
--- a/crates/test-utils/src/minicore.rs
+++ b/crates/test-utils/src/minicore.rs
@@ -111,6 +111,7 @@ pub mod marker {
         impl<T: ?Sized> Copy for *const T {}
         impl<T: ?Sized> Copy for *mut T {}
         impl<T: ?Sized> Copy for &T {}
+        impl Copy for ! {}
     }
     // endregion:copy
 
@@ -246,6 +247,12 @@ pub mod clone {
         f32 f64
         bool char
     }
+
+    impl Clone for ! {
+        fn clone(&self) {
+            *self
+        }
+    }
     // endregion:builtin_impls
 
     // region:derive
@@ -319,8 +326,8 @@ pub mod mem {
     pub fn drop<T>(_x: T) {}
     pub const fn replace<T>(dest: &mut T, src: T) -> T {
         unsafe {
-            let result = *dest;
-            *dest = src;
+            let result = crate::ptr::read(dest);
+            crate::ptr::write(dest, src);
             result
         }
     }
@@ -339,6 +346,12 @@ pub mod ptr {
     pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
         unsafe { drop_in_place(to_drop) }
     }
+    pub const unsafe fn read<T>(src: *const T) -> T {
+        *src
+    }
+    pub const unsafe fn write<T>(dst: *mut T, src: T) {
+        *dst = src;
+    }
     // endregion:drop
 }