diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 66a23eb4125d5..883f711b201a9 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -12,7 +12,7 @@ use rustc_middle::mir::{ FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, VarBindingForm, }; -use rustc_middle::ty::{self, suggest_constraining_type_params, PredicateKind, Ty}; +use rustc_middle::ty::{self, subst::Subst, suggest_constraining_type_params, PredicateKind, Ty}; use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex}; use rustc_span::symbol::sym; use rustc_span::{BytePos, MultiSpan, Span}; @@ -151,6 +151,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { .args_or_use() }) .collect::>(); + let reinits = maybe_reinitialized_locations.len(); if reinits == 1 { err.span_label(reinit_spans[0], "this reinitialization might get skipped"); @@ -276,76 +277,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - if needs_note { - let opt_name = - self.describe_place_with_options(place.as_ref(), IncludingDowncast(true)); - let note_msg = match opt_name { - Some(ref name) => format!("`{}`", name), - None => "value".to_owned(), - }; - - // Try to find predicates on *generic params* that would allow copying `ty` - let tcx = self.infcx.tcx; - let generics = tcx.generics_of(self.mir_def_id()); - if let Some(hir_generics) = tcx - .typeck_root_def_id(self.mir_def_id().to_def_id()) - .as_local() - .and_then(|def_id| tcx.hir().get_generics(def_id)) - { - let predicates: Result, _> = tcx.infer_ctxt().enter(|infcx| { - let mut fulfill_cx = - >::new(infcx.tcx); - - let copy_did = infcx.tcx.lang_items().copy_trait().unwrap(); - let cause = ObligationCause::new( - span, - self.mir_hir_id(), - rustc_infer::traits::ObligationCauseCode::MiscObligation, - ); - fulfill_cx.register_bound( - &infcx, - self.param_env, - // Erase any region vids from the type, which may not be resolved - infcx.tcx.erase_regions(ty), - copy_did, - cause, - ); - // Select all, including ambiguous predicates - let errors = fulfill_cx.select_all_or_error(&infcx); - - // Only emit suggestion if all required predicates are on generic - errors - .into_iter() - .map(|err| match err.obligation.predicate.kind().skip_binder() { - PredicateKind::Trait(predicate) => { - match predicate.self_ty().kind() { - ty::Param(param_ty) => Ok(( - generics.type_param(param_ty, tcx), - predicate.trait_ref.print_only_trait_path().to_string(), - )), - _ => Err(()), - } - } - _ => Err(()), - }) - .collect() - }); - - if let Ok(predicates) = predicates { - suggest_constraining_type_params( - tcx, - hir_generics, - &mut err, - predicates.iter().map(|(param, constraint)| { - (param.name.as_str(), &**constraint, None) - }), - ); - } - } + let opt_name = + self.describe_place_with_options(place.as_ref(), IncludingDowncast(true)); + let note_msg = match opt_name { + Some(ref name) => format!("`{}`", name), + None => "value".to_owned(), + }; + if self.suggest_borrow_fn_like(&mut err, ty, &move_site_vec, ¬e_msg) { + // Suppress the next suggestion since we don't want to put more bounds onto + // something that already has `Fn`-like bounds (or is a closure), so we can't + // restrict anyways. + } else { + self.suggest_adding_copy_bounds(&mut err, ty, span); + } + if needs_note { let span = if let Some(local) = place.as_local() { - let decl = &self.body.local_decls[local]; - Some(decl.source_info.span) + Some(self.body.local_decls[local].source_info.span) } else { None }; @@ -373,6 +321,144 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } + fn suggest_borrow_fn_like( + &self, + err: &mut DiagnosticBuilder<'tcx, ErrorGuaranteed>, + ty: Ty<'tcx>, + move_sites: &[MoveSite], + value_name: &str, + ) -> bool { + let tcx = self.infcx.tcx; + + // Find out if the predicates show that the type is a Fn or FnMut + let find_fn_kind_from_did = |predicates: &[(ty::Predicate<'tcx>, Span)], substs| { + predicates.iter().find_map(|(pred, _)| { + let pred = if let Some(substs) = substs { + pred.subst(tcx, substs).kind().skip_binder() + } else { + pred.kind().skip_binder() + }; + if let ty::PredicateKind::Trait(pred) = pred && pred.self_ty() == ty { + if Some(pred.def_id()) == tcx.lang_items().fn_trait() { + return Some(hir::Mutability::Not); + } else if Some(pred.def_id()) == tcx.lang_items().fn_mut_trait() { + return Some(hir::Mutability::Mut); + } + } + None + }) + }; + + // If the type is opaque/param/closure, and it is Fn or FnMut, let's suggest (mutably) + // borrowing the type, since `&mut F: FnMut` iff `F: FnMut` and similarly for `Fn`. + // These types seem reasonably opaque enough that they could be substituted with their + // borrowed variants in a function body when we see a move error. + let borrow_level = match ty.kind() { + ty::Param(_) => find_fn_kind_from_did( + tcx.explicit_predicates_of(self.mir_def_id().to_def_id()).predicates, + None, + ), + ty::Opaque(did, substs) => { + find_fn_kind_from_did(tcx.explicit_item_bounds(*did), Some(*substs)) + } + ty::Closure(_, substs) => match substs.as_closure().kind() { + ty::ClosureKind::Fn => Some(hir::Mutability::Not), + ty::ClosureKind::FnMut => Some(hir::Mutability::Mut), + _ => None, + }, + _ => None, + }; + + let Some(borrow_level) = borrow_level else { return false; }; + let sugg = move_sites + .iter() + .map(|move_site| { + let move_out = self.move_data.moves[(*move_site).moi]; + let moved_place = &self.move_data.move_paths[move_out.path].place; + let move_spans = self.move_spans(moved_place.as_ref(), move_out.source); + let move_span = move_spans.args_or_use(); + let suggestion = if borrow_level == hir::Mutability::Mut { + "&mut ".to_string() + } else { + "&".to_string() + }; + (move_span.shrink_to_lo(), suggestion) + }) + .collect(); + err.multipart_suggestion_verbose( + &format!( + "consider {}borrowing {value_name}", + if borrow_level == hir::Mutability::Mut { "mutably " } else { "" } + ), + sugg, + Applicability::MaybeIncorrect, + ); + true + } + + fn suggest_adding_copy_bounds( + &self, + err: &mut DiagnosticBuilder<'tcx, ErrorGuaranteed>, + ty: Ty<'tcx>, + span: Span, + ) { + let tcx = self.infcx.tcx; + let generics = tcx.generics_of(self.mir_def_id()); + + let Some(hir_generics) = tcx + .typeck_root_def_id(self.mir_def_id().to_def_id()) + .as_local() + .and_then(|def_id| tcx.hir().get_generics(def_id)) + else { return; }; + // Try to find predicates on *generic params* that would allow copying `ty` + let predicates: Result, _> = tcx.infer_ctxt().enter(|infcx| { + let mut fulfill_cx = >::new(infcx.tcx); + + let copy_did = infcx.tcx.lang_items().copy_trait().unwrap(); + let cause = ObligationCause::new( + span, + self.mir_hir_id(), + rustc_infer::traits::ObligationCauseCode::MiscObligation, + ); + fulfill_cx.register_bound( + &infcx, + self.param_env, + // Erase any region vids from the type, which may not be resolved + infcx.tcx.erase_regions(ty), + copy_did, + cause, + ); + // Select all, including ambiguous predicates + let errors = fulfill_cx.select_all_or_error(&infcx); + + // Only emit suggestion if all required predicates are on generic + errors + .into_iter() + .map(|err| match err.obligation.predicate.kind().skip_binder() { + PredicateKind::Trait(predicate) => match predicate.self_ty().kind() { + ty::Param(param_ty) => Ok(( + generics.type_param(param_ty, tcx), + predicate.trait_ref.print_only_trait_path().to_string(), + )), + _ => Err(()), + }, + _ => Err(()), + }) + .collect() + }); + + if let Ok(predicates) = predicates { + suggest_constraining_type_params( + tcx, + hir_generics, + err, + predicates + .iter() + .map(|(param, constraint)| (param.name.as_str(), &**constraint, None)), + ); + } + } + pub(crate) fn report_move_out_while_borrowed( &mut self, location: Location, diff --git a/src/test/ui/chalkify/closure.stderr b/src/test/ui/chalkify/closure.stderr index d5a48a7dc6f95..515e0cf01429d 100644 --- a/src/test/ui/chalkify/closure.stderr +++ b/src/test/ui/chalkify/closure.stderr @@ -12,6 +12,10 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | a = 1; | ^ +help: consider mutably borrowing `b` + | +LL | let mut c = &mut b; + | ++++ error: aborting due to previous error diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-multi-variant-diagnostics.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-multi-variant-diagnostics.stderr index 066c000c832d8..83d282aadb915 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-multi-variant-diagnostics.stderr +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-multi-variant-diagnostics.stderr @@ -11,6 +11,10 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | if let MultiVariant::Point(ref mut x, _) = point { | ^^^^^ +help: consider mutably borrowing `c` + | +LL | let a = &mut c; + | ++++ error: aborting due to previous error diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr index 2a6e00850fa8a..46323b75210cb 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr @@ -11,6 +11,10 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | let SingleVariant::Point(ref mut x, _) = point; | ^^^^^ +help: consider mutably borrowing `c` + | +LL | let b = &mut c; + | ++++ error: aborting due to previous error diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-struct-diagnostics.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-struct-diagnostics.stderr index d7fc51c55eae3..25029cc7bd8b8 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-struct-diagnostics.stderr +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-struct-diagnostics.stderr @@ -11,6 +11,10 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | x.y.a += 1; | ^^^^^ +help: consider mutably borrowing `hello` + | +LL | let b = &mut hello; + | ++++ error: aborting due to previous error diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics-1.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics-1.stderr index 63e2d300eb06a..06ef7baf9c068 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics-1.stderr +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-tuple-diagnostics-1.stderr @@ -11,6 +11,10 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | x.0 += 1; | ^^^ +help: consider mutably borrowing `hello` + | +LL | let b = &mut hello; + | ++++ error: aborting due to previous error diff --git a/src/test/ui/moves/borrow-closures-instead-of-move.rs b/src/test/ui/moves/borrow-closures-instead-of-move.rs new file mode 100644 index 0000000000000..51771ced7f22f --- /dev/null +++ b/src/test/ui/moves/borrow-closures-instead-of-move.rs @@ -0,0 +1,36 @@ +fn takes_fn(f: impl Fn()) { + loop { + takes_fnonce(f); + //~^ ERROR use of moved value + //~| HELP consider borrowing + } +} + +fn takes_fn_mut(m: impl FnMut()) { + if maybe() { + takes_fnonce(m); + //~^ HELP consider mutably borrowing + } + takes_fnonce(m); + //~^ ERROR use of moved value +} + +fn has_closure() { + let mut x = 0; + let mut closure = || { + x += 1; + }; + takes_fnonce(closure); + //~^ HELP consider mutably borrowing + closure(); + //~^ ERROR borrow of moved value +} + +fn maybe() -> bool { + false +} + +// Could also be Fn[Mut], here it doesn't matter +fn takes_fnonce(_: impl FnOnce()) {} + +fn main() {} diff --git a/src/test/ui/moves/borrow-closures-instead-of-move.stderr b/src/test/ui/moves/borrow-closures-instead-of-move.stderr new file mode 100644 index 0000000000000..3146b6959001e --- /dev/null +++ b/src/test/ui/moves/borrow-closures-instead-of-move.stderr @@ -0,0 +1,53 @@ +error[E0382]: use of moved value: `f` + --> $DIR/borrow-closures-instead-of-move.rs:3:22 + | +LL | fn takes_fn(f: impl Fn()) { + | - move occurs because `f` has type `impl Fn()`, which does not implement the `Copy` trait +LL | loop { +LL | takes_fnonce(f); + | ^ value moved here, in previous iteration of loop + | +help: consider borrowing `f` + | +LL | takes_fnonce(&f); + | + + +error[E0382]: use of moved value: `m` + --> $DIR/borrow-closures-instead-of-move.rs:14:18 + | +LL | fn takes_fn_mut(m: impl FnMut()) { + | - move occurs because `m` has type `impl FnMut()`, which does not implement the `Copy` trait +LL | if maybe() { +LL | takes_fnonce(m); + | - value moved here +... +LL | takes_fnonce(m); + | ^ value used here after move + | +help: consider mutably borrowing `m` + | +LL | takes_fnonce(&mut m); + | ++++ + +error[E0382]: borrow of moved value: `closure` + --> $DIR/borrow-closures-instead-of-move.rs:25:5 + | +LL | takes_fnonce(closure); + | ------- value moved here +LL | +LL | closure(); + | ^^^^^^^ value borrowed here after move + | +note: closure cannot be moved more than once as it is not `Copy` due to moving the variable `x` out of its environment + --> $DIR/borrow-closures-instead-of-move.rs:21:9 + | +LL | x += 1; + | ^ +help: consider mutably borrowing `closure` + | +LL | takes_fnonce(&mut closure); + | ++++ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0382`. diff --git a/src/test/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr b/src/test/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr index 2adaf576adf5c..4759b45892cc4 100644 --- a/src/test/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr +++ b/src/test/ui/moves/moves-based-on-type-no-recursive-stack-closure.stderr @@ -17,10 +17,10 @@ LL | let mut r = R {c: Box::new(f)}; LL | f(&mut r, false) | ^ value borrowed here after move | -help: consider further restricting this bound +help: consider mutably borrowing `f` | -LL | fn conspirator(mut f: F) where F: FnMut(&mut R, bool) + Copy { - | ++++++ +LL | let mut r = R {c: Box::new(&mut f)}; + | ++++ error: aborting due to 2 previous errors diff --git a/src/test/ui/not-copy-closure.stderr b/src/test/ui/not-copy-closure.stderr index 10bf570727fa0..93e011ccec4fa 100644 --- a/src/test/ui/not-copy-closure.stderr +++ b/src/test/ui/not-copy-closure.stderr @@ -11,6 +11,10 @@ note: closure cannot be moved more than once as it is not `Copy` due to moving t | LL | a += 1; | ^ +help: consider mutably borrowing `hello` + | +LL | let b = &mut hello; + | ++++ error: aborting due to previous error