diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 1b4bb11d87b38..b7e5059f9e456 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1102,8 +1102,61 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { // suggestion more targeted to the `mk_iter(val).next()` case. Maybe do that only to // check for whether to suggest `let value` or `let mut value`. + // This closure checks if `cur_use_expr` is defined in scope visible to `new_use_expr`. + // Used to check *if the `value` can be moved out of the loop*. + let is_var_visible_to_new_use = |cur_use_expr: &hir::Expr<'_>, + new_use_expr: &hir::Expr<'_>| + -> bool { + let new_use_hir_id = new_use_expr.hir_id; + let new_use_span = new_use_expr.span; + + if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = cur_use_expr.kind { + if let hir::def::Res::Local(def_hir_id) = path.res { + if let Some(def_direct_parent) = tcx.hir_get_enclosing_scope(def_hir_id) + { + let mut first = true; + let mut id = new_use_hir_id; + // iterate over the parents of the new use expression + while let Some(parent_hir_id) = tcx.hir_get_enclosing_scope(id) { + if first { + first = false; + // if they are in the same scope, + // the parent of the new use expr is the same as the parent of the definition, + // the definition should be before the new use expression + if parent_hir_id == def_direct_parent { + return tcx.hir_span(def_hir_id).lo() + < new_use_span.lo(); + } + } else if parent_hir_id == def_direct_parent { + // if they are not in the same scope, + // but the parent of the definition is the ancestor of the new use expr, + // the definition is visible to the new use expr + return true; + } + id = parent_hir_id; + } + } + } + } + false + }; + + // Issue: https://github.com/rust-lang/rust/issues/141880 + let mut permit = true; + if let hir::ExprKind::MethodCall(_, obj, args, ..) = parent.kind { + if !is_var_visible_to_new_use(obj, in_loop) { + permit = false; + } + for arg in args { + if !is_var_visible_to_new_use(arg, in_loop) { + permit = false; + } + } + } + let span = in_loop.span; - if !finder.found_breaks.is_empty() + if permit + && !finder.found_breaks.is_empty() && let Ok(value) = sm.span_to_snippet(parent.span) { // We know with high certainty that this move would affect the early return of a diff --git a/tests/ui/borrowck/suggest-move-expr-out-of-loop-issue-141880.rs b/tests/ui/borrowck/suggest-move-expr-out-of-loop-issue-141880.rs new file mode 100644 index 0000000000000..afd608b89ab4c --- /dev/null +++ b/tests/ui/borrowck/suggest-move-expr-out-of-loop-issue-141880.rs @@ -0,0 +1,35 @@ +#[derive(Default)] +struct Foo{ + x: String, +} + +impl Foo { + fn foo(&mut self, name: String) { + self.x = name; + } +} + + + +fn main() { + let name1 = String::from("foo"); + for _ in 0..3 { + let mut foo = Foo::default(); + foo.foo(name1); //~ ERROR use of moved value: `name1` [E0382] + println!("{}", foo.x); + } + + let name2 = String::from("bar"); + for mut foo in [1,2,3].iter_mut().map(|m| Foo::default()) { + foo.foo(name2); //~ ERROR use of moved value: `name2` [E0382] + println!("{}", foo.x); + } + + + let name3 = String::from("baz"); + let mut foo = Foo::default(); + for _ in 0..10 { + foo.foo(name3); //~ ERROR use of moved value: `name3` [E0382] + println!("{}", foo.x); + } +} diff --git a/tests/ui/borrowck/suggest-move-expr-out-of-loop-issue-141880.stderr b/tests/ui/borrowck/suggest-move-expr-out-of-loop-issue-141880.stderr new file mode 100644 index 0000000000000..f26aa4d9719d3 --- /dev/null +++ b/tests/ui/borrowck/suggest-move-expr-out-of-loop-issue-141880.stderr @@ -0,0 +1,71 @@ +error[E0382]: use of moved value: `name1` + --> $DIR/suggest-move-expr-out-of-loop-issue-141880.rs:18:17 + | +LL | let name1 = String::from("foo"); + | ----- move occurs because `name1` has type `String`, which does not implement the `Copy` trait +LL | for _ in 0..3 { + | ------------- inside of this loop +LL | let mut foo = Foo::default(); +LL | foo.foo(name1); + | ^^^^^ value moved here, in previous iteration of loop + | +note: consider changing this parameter type in method `foo` to borrow instead if owning the value isn't necessary + --> $DIR/suggest-move-expr-out-of-loop-issue-141880.rs:7:29 + | +LL | fn foo(&mut self, name: String) { + | --- in this method ^^^^^^ this parameter takes ownership of the value +help: consider cloning the value if the performance cost is acceptable + | +LL | foo.foo(name1.clone()); + | ++++++++ + +error[E0382]: use of moved value: `name2` + --> $DIR/suggest-move-expr-out-of-loop-issue-141880.rs:24:17 + | +LL | let name2 = String::from("bar"); + | ----- move occurs because `name2` has type `String`, which does not implement the `Copy` trait +LL | for mut foo in [1,2,3].iter_mut().map(|m| Foo::default()) { + | ---------------------------------------------------------- inside of this loop +LL | foo.foo(name2); + | ^^^^^ value moved here, in previous iteration of loop + | +note: consider changing this parameter type in method `foo` to borrow instead if owning the value isn't necessary + --> $DIR/suggest-move-expr-out-of-loop-issue-141880.rs:7:29 + | +LL | fn foo(&mut self, name: String) { + | --- in this method ^^^^^^ this parameter takes ownership of the value +help: consider cloning the value if the performance cost is acceptable + | +LL | foo.foo(name2.clone()); + | ++++++++ + +error[E0382]: use of moved value: `name3` + --> $DIR/suggest-move-expr-out-of-loop-issue-141880.rs:32:17 + | +LL | let name3 = String::from("baz"); + | ----- move occurs because `name3` has type `String`, which does not implement the `Copy` trait +LL | let mut foo = Foo::default(); +LL | for _ in 0..10 { + | -------------- inside of this loop +LL | foo.foo(name3); + | ^^^^^ value moved here, in previous iteration of loop + | +note: consider changing this parameter type in method `foo` to borrow instead if owning the value isn't necessary + --> $DIR/suggest-move-expr-out-of-loop-issue-141880.rs:7:29 + | +LL | fn foo(&mut self, name: String) { + | --- in this method ^^^^^^ this parameter takes ownership of the value +help: consider moving the expression out of the loop so it is only moved once + | +LL ~ let mut value = foo.foo(name3); +LL ~ for _ in 0..10 { +LL ~ value; + | +help: consider cloning the value if the performance cost is acceptable + | +LL | foo.foo(name3.clone()); + | ++++++++ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0382`.