Unnamed repository; edit this file 'description' to name the repository.
| -rw-r--r-- | crates/hir-ty/src/infer/expr.rs | 13 | ||||
| -rw-r--r-- | crates/hir-ty/src/mir/borrowck.rs | 118 | ||||
| -rw-r--r-- | crates/hir/src/diagnostics.rs | 7 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 29 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/missing_match_arms.rs | 2 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/missing_unsafe.rs | 5 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/moved_out_of_ref.rs | 154 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/mutability_errors.rs | 1 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/lib.rs | 2 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/tests.rs | 14 | ||||
| -rw-r--r-- | crates/test-utils/src/minicore.rs | 17 |
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 3261c22313..b800c2e32a 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 a6af4e75d4..412390d3fa 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 f81f8b0b01..10893b62bf 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 4204744614..bc925552f3 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 b5e619e2a0..3f13b97a44 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 d73f4e7721..2026b6fcef 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 0000000000..99243a5ab8 --- /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 d60007b48a..9d6a862cb1 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 25d3568950..048dedf6bd 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 413689561b..b5cd4e0d68 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 22cef04983..6d6c9af7f0 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 } |