diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 16cd7a0bcdd3..26b1052e0de2 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -1450,8 +1450,13 @@ impl<'hir> LoweringContext<'_, 'hir> { ) }; + // #82462: to correctly diagnose borrow errors, the block that contains + // the iter expr needs to have a span that covers the loop body. + let desugared_full_span = + self.mark_span_with_reason(DesugaringKind::ForLoop(ForLoopLoc::Head), e.span, None); + let match_expr = self.arena.alloc(self.expr_match( - desugared_span, + desugared_full_span, into_iter_expr, arena_vec![self; iter_arm], hir::MatchSource::ForLoopDesugar, @@ -1465,7 +1470,7 @@ impl<'hir> LoweringContext<'_, 'hir> { // surrounding scope of the `match` since the `match` is not a terminating scope. // // Also, add the attributes to the outer returned expr node. - self.expr_drop_temps_mut(desugared_span, match_expr, attrs.into()) + self.expr_drop_temps_mut(desugared_full_span, match_expr, attrs.into()) } /// Desugar `ExprKind::Try` from: `?` into: diff --git a/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff b/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff index 6d6c2721973f..02f6e55a9a8b 100644 --- a/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff +++ b/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff @@ -80,7 +80,7 @@ - StorageDead(_7); // scope 3 at $DIR/remove_storage_markers.rs:8:18: 8:19 - StorageDead(_6); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 - StorageDead(_4); // scope 1 at $DIR/remove_storage_markers.rs:10:5: 10:6 -- StorageDead(_2); // scope 1 at $DIR/remove_storage_markers.rs:8:18: 8:19 +- StorageDead(_2); // scope 1 at $DIR/remove_storage_markers.rs:10:5: 10:6 - StorageDead(_1); // scope 0 at $DIR/remove_storage_markers.rs:11:1: 11:2 return; // scope 0 at $DIR/remove_storage_markers.rs:11:2: 11:2 } diff --git a/src/test/ui/borrowck/issue-82462.nll.stderr b/src/test/ui/borrowck/issue-82462.nll.stderr new file mode 100644 index 000000000000..10497c30e64f --- /dev/null +++ b/src/test/ui/borrowck/issue-82462.nll.stderr @@ -0,0 +1,22 @@ +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/issue-82462.rs:18:9 + | +LL | for x in DroppingSlice(&*v).iter() { + | ------------------ + | | | + | | immutable borrow occurs here + | a temporary with access to the immutable borrow is created here ... +LL | v.push(*x); + | ^ mutable borrow occurs here +LL | break; +LL | } + | - ... and the immutable borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `DroppingSlice` + | +help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped + | +LL | }; + | + + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/borrowck/issue-82462.rs b/src/test/ui/borrowck/issue-82462.rs new file mode 100644 index 000000000000..5a3c64255cc6 --- /dev/null +++ b/src/test/ui/borrowck/issue-82462.rs @@ -0,0 +1,21 @@ +struct DroppingSlice<'a>(&'a [i32]); + +impl Drop for DroppingSlice<'_> { + fn drop(&mut self) { + println!("hi from slice"); + } +} + +impl DroppingSlice<'_> { + fn iter(&self) -> std::slice::Iter<'_, i32> { + self.0.iter() + } +} + +fn main() { + let mut v = vec![1, 2, 3, 4]; + for x in DroppingSlice(&*v).iter() { + v.push(*x); //~ERROR + break; + } +} diff --git a/src/test/ui/borrowck/issue-82462.stderr b/src/test/ui/borrowck/issue-82462.stderr new file mode 100644 index 000000000000..a2c291f77979 --- /dev/null +++ b/src/test/ui/borrowck/issue-82462.stderr @@ -0,0 +1,22 @@ +error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable + --> $DIR/issue-82462.rs:18:9 + | +LL | for x in DroppingSlice(&*v).iter() { + | ------------------ + | | | + | | immutable borrow occurs here + | a temporary with access to the immutable borrow is created here ... +LL | v.push(*x); + | ^^^^^^^^^^ mutable borrow occurs here +LL | break; +LL | } + | - ... and the immutable borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `DroppingSlice` + | +help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped + | +LL | }; + | + + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0502`. diff --git a/src/tools/clippy/clippy_lints/src/loops/for_kv_map.rs b/src/tools/clippy/clippy_lints/src/loops/for_kv_map.rs index 68bef2f4c8bb..12ffe7a13645 100644 --- a/src/tools/clippy/clippy_lints/src/loops/for_kv_map.rs +++ b/src/tools/clippy/clippy_lints/src/loops/for_kv_map.rs @@ -15,7 +15,6 @@ pub(super) fn check<'tcx>( pat: &'tcx Pat<'_>, arg: &'tcx Expr<'_>, body: &'tcx Expr<'_>, - expr: &'tcx Expr<'_>, ) { let pat_span = pat.span; @@ -43,7 +42,7 @@ pub(super) fn check<'tcx>( span_lint_and_then( cx, FOR_KV_MAP, - expr.span, + arg_span, &format!("you seem to want to iterate on a map's {}s", kind), |diag| { let map = sugg::Sugg::hir(cx, arg, "map"); diff --git a/src/tools/clippy/clippy_lints/src/loops/iter_next_loop.rs b/src/tools/clippy/clippy_lints/src/loops/iter_next_loop.rs index 9148fbfd497a..e640c62ebdac 100644 --- a/src/tools/clippy/clippy_lints/src/loops/iter_next_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/iter_next_loop.rs @@ -5,12 +5,12 @@ use rustc_hir::Expr; use rustc_lint::LateContext; use rustc_span::sym; -pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, expr: &Expr<'_>) -> bool { +pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool { if is_trait_method(cx, arg, sym::Iterator) { span_lint( cx, ITER_NEXT_LOOP, - expr.span, + arg.span, "you are iterating over `Iterator::next()` which is an Option; this will compile but is \ probably not what you want", ); diff --git a/src/tools/clippy/clippy_lints/src/loops/mod.rs b/src/tools/clippy/clippy_lints/src/loops/mod.rs index 2860cb68f42f..5df1b7964016 100644 --- a/src/tools/clippy/clippy_lints/src/loops/mod.rs +++ b/src/tools/clippy/clippy_lints/src/loops/mod.rs @@ -616,15 +616,15 @@ fn check_for_loop<'tcx>( needless_range_loop::check(cx, pat, arg, body, expr); explicit_counter_loop::check(cx, pat, arg, body, expr); } - check_for_loop_arg(cx, pat, arg, expr); - for_kv_map::check(cx, pat, arg, body, expr); + check_for_loop_arg(cx, pat, arg); + for_kv_map::check(cx, pat, arg, body); mut_range_bound::check(cx, arg, body); single_element_loop::check(cx, pat, arg, body, expr); same_item_push::check(cx, pat, arg, body, expr); manual_flatten::check(cx, pat, arg, body, span); } -fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>, expr: &Expr<'_>) { +fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) { let mut next_loop_linted = false; // whether or not ITER_NEXT_LOOP lint was used if let ExprKind::MethodCall(method, _, [self_arg], _) = arg.kind { @@ -637,7 +637,7 @@ fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>, expr: explicit_into_iter_loop::check(cx, self_arg, arg); }, "next" => { - next_loop_linted = iter_next_loop::check(cx, arg, expr); + next_loop_linted = iter_next_loop::check(cx, arg); }, _ => {}, } diff --git a/src/tools/clippy/clippy_lints/src/loops/needless_range_loop.rs b/src/tools/clippy/clippy_lints/src/loops/needless_range_loop.rs index e8f3550283a4..7157b8011855 100644 --- a/src/tools/clippy/clippy_lints/src/loops/needless_range_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/needless_range_loop.rs @@ -144,7 +144,7 @@ pub(super) fn check<'tcx>( span_lint_and_then( cx, NEEDLESS_RANGE_LOOP, - expr.span, + arg.span, &format!("the loop variable `{}` is used to index `{}`", ident.name, indexed), |diag| { multispan_sugg( @@ -170,7 +170,7 @@ pub(super) fn check<'tcx>( span_lint_and_then( cx, NEEDLESS_RANGE_LOOP, - expr.span, + arg.span, &format!("the loop variable `{}` is only used to index `{}`", ident.name, indexed), |diag| { multispan_sugg(