diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs index 0b01c4efcdbd4..2e854ea5be7df 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs @@ -12,7 +12,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::{self, suggest_constraining_type_param, Ty}; use rustc_span::source_map::DesugaringKind; use rustc_span::symbol::sym; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::{MultiSpan, Span, DUMMY_SP}; use rustc_trait_selection::infer::InferCtxtExt; use crate::dataflow::drop_flag_effects; @@ -66,7 +66,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { self.move_spans(moved_place, location).or_else(|| self.borrow_spans(span, location)); let span = use_spans.args_or_use(); - let move_site_vec = self.get_moved_indexes(location, mpi); + let (move_site_vec, maybe_reinitialized_locations) = self.get_moved_indexes(location, mpi); debug!( "report_use_of_moved_or_uninitialized: move_site_vec={:?} use_spans={:?}", move_site_vec, use_spans @@ -139,6 +139,32 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { self.describe_place_with_options(moved_place, IncludingDowncast(true)), ); + let reinit_spans = maybe_reinitialized_locations + .iter() + .take(3) + .map(|loc| { + self.move_spans(self.move_data.move_paths[mpi].place.as_ref(), *loc) + .args_or_use() + }) + .collect::>(); + let reinits = maybe_reinitialized_locations.len(); + if reinits == 1 { + err.span_label(reinit_spans[0], "this reinitialization might get skipped"); + } else if reinits > 1 { + err.span_note( + MultiSpan::from_spans(reinit_spans), + &if reinits <= 3 { + format!("these {} reinitializations might get skipped", reinits) + } else { + format!( + "these 3 reinitializations and {} other{} might get skipped", + reinits - 3, + if reinits == 4 { "" } else { "s" } + ) + }, + ); + } + self.add_moved_or_invoked_closure_note(location, used_place, &mut err); let mut is_loop_move = false; @@ -219,7 +245,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ), ); } - if is_option_or_result { + if is_option_or_result && maybe_reinitialized_locations.is_empty() { err.span_suggestion_verbose( fn_call_span.shrink_to_lo(), "consider calling `.as_ref()` to borrow the type's contents", @@ -260,19 +286,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - if let UseSpans::PatUse(span) = move_spans { - err.span_suggestion_verbose( - span.shrink_to_lo(), - &format!( - "borrow this field in the pattern to avoid moving {}", - self.describe_place(moved_place.as_ref()) - .map(|n| format!("`{}`", n)) - .unwrap_or_else(|| "the value".to_string()) - ), - "ref ".to_string(), - Applicability::MachineApplicable, - ); - in_pattern = true; + if let (UseSpans::PatUse(span), []) = + (move_spans, &maybe_reinitialized_locations[..]) + { + if maybe_reinitialized_locations.is_empty() { + err.span_suggestion_verbose( + span.shrink_to_lo(), + &format!( + "borrow this field in the pattern to avoid moving {}", + self.describe_place(moved_place.as_ref()) + .map(|n| format!("`{}`", n)) + .unwrap_or_else(|| "the value".to_string()) + ), + "ref ".to_string(), + Applicability::MachineApplicable, + ); + in_pattern = true; + } } if let Some(DesugaringKind::ForLoop(_)) = move_span.desugaring_kind() { @@ -1465,7 +1495,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { err } - fn get_moved_indexes(&mut self, location: Location, mpi: MovePathIndex) -> Vec { + fn get_moved_indexes( + &mut self, + location: Location, + mpi: MovePathIndex, + ) -> (Vec, Vec) { fn predecessor_locations( body: &'a mir::Body<'tcx>, location: Location, @@ -1488,6 +1522,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { })); let mut visited = FxHashSet::default(); + let mut move_locations = FxHashSet::default(); + let mut reinits = vec![]; let mut result = vec![]; 'dfs: while let Some((location, is_back_edge)) = stack.pop() { @@ -1529,6 +1565,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { move_paths[path].place ); result.push(MoveSite { moi: *moi, traversed_back_edge: is_back_edge }); + move_locations.insert(location); // Strictly speaking, we could continue our DFS here. There may be // other moves that can reach the point of error. But it is kind of @@ -1565,6 +1602,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { }, ); if any_match { + reinits.push(location); continue 'dfs; } @@ -1574,7 +1612,25 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { })); } - result + // Check if we can reach these reinits from a move location. + let reinits_reachable = reinits + .into_iter() + .filter(|reinit| { + let mut visited = FxHashSet::default(); + let mut stack = vec![*reinit]; + while let Some(location) = stack.pop() { + if !visited.insert(location) { + continue; + } + if move_locations.contains(&location) { + return true; + } + stack.extend(predecessor_locations(self.body, location)); + } + false + }) + .collect::>(); + (result, reinits_reachable) } pub(in crate::borrow_check) fn report_illegal_mutation_of_borrowed( diff --git a/src/test/ui/borrowck/issue-83760.rs b/src/test/ui/borrowck/issue-83760.rs new file mode 100644 index 0000000000000..e25b4f727856e --- /dev/null +++ b/src/test/ui/borrowck/issue-83760.rs @@ -0,0 +1,40 @@ +struct Struct; + +fn test1() { + let mut val = Some(Struct); + while let Some(foo) = val { //~ ERROR use of moved value + if true { + val = None; + } else { + + } + } +} + +fn test2() { + let mut foo = Some(Struct); + let _x = foo.unwrap(); + if true { + foo = Some(Struct); + } else { + } + let _y = foo; //~ ERROR use of moved value: `foo` +} + +fn test3() { + let mut foo = Some(Struct); + let _x = foo.unwrap(); + if true { + foo = Some(Struct); + } else if true { + foo = Some(Struct); + } else if true { + foo = Some(Struct); + } else if true { + foo = Some(Struct); + } else { + } + let _y = foo; //~ ERROR use of moved value: `foo` +} + +fn main() {} diff --git a/src/test/ui/borrowck/issue-83760.stderr b/src/test/ui/borrowck/issue-83760.stderr new file mode 100644 index 0000000000000..beeda5685dc09 --- /dev/null +++ b/src/test/ui/borrowck/issue-83760.stderr @@ -0,0 +1,62 @@ +error[E0382]: use of moved value + --> $DIR/issue-83760.rs:5:20 + | +LL | while let Some(foo) = val { + | ^^^ value moved here, in previous iteration of loop +LL | if true { +LL | val = None; + | ---------- this reinitialization might get skipped + | + = note: move occurs because value has type `Struct`, which does not implement the `Copy` trait + +error[E0382]: use of moved value: `foo` + --> $DIR/issue-83760.rs:21:14 + | +LL | let mut foo = Some(Struct); + | ------- move occurs because `foo` has type `Option`, which does not implement the `Copy` trait +LL | let _x = foo.unwrap(); + | -------- `foo` moved due to this method call +LL | if true { +LL | foo = Some(Struct); + | ------------------ this reinitialization might get skipped +... +LL | let _y = foo; + | ^^^ value used here after move + | +note: this function takes ownership of the receiver `self`, which moves `foo` + --> $SRC_DIR/core/src/option.rs:LL:COL + | +LL | pub const fn unwrap(self) -> T { + | ^^^^ + +error[E0382]: use of moved value: `foo` + --> $DIR/issue-83760.rs:37:14 + | +LL | let mut foo = Some(Struct); + | ------- move occurs because `foo` has type `Option`, which does not implement the `Copy` trait +LL | let _x = foo.unwrap(); + | -------- `foo` moved due to this method call +... +LL | let _y = foo; + | ^^^ value used here after move + | +note: these 3 reinitializations and 1 other might get skipped + --> $DIR/issue-83760.rs:30:9 + | +LL | foo = Some(Struct); + | ^^^^^^^^^^^^^^^^^^ +LL | } else if true { +LL | foo = Some(Struct); + | ^^^^^^^^^^^^^^^^^^ +LL | } else if true { +LL | foo = Some(Struct); + | ^^^^^^^^^^^^^^^^^^ +note: this function takes ownership of the receiver `self`, which moves `foo` + --> $SRC_DIR/core/src/option.rs:LL:COL + | +LL | pub const fn unwrap(self) -> T { + | ^^^^ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0382`.