From cc606174a6b18572f523790e80bcd59c95649b2a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 25 Apr 2024 11:48:57 -0400 Subject: [PATCH 1/8] Don't ICE when codegen_select returns ambiguity in new solver --- compiler/rustc_ty_utils/src/instance.rs | 17 +++++------------ .../issue-69602-type-err-during-codegen-ice.rs | 1 - ...sue-69602-type-err-during-codegen-ice.stderr | 6 ------ .../next-solver/ambiguous-impl-in-resolve.rs | 17 +++++++++++++++++ 4 files changed, 22 insertions(+), 19 deletions(-) create mode 100644 tests/ui/traits/next-solver/ambiguous-impl-in-resolve.rs diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index c1661fa63a8b8..d0aa4eb2e714a 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -101,18 +101,11 @@ fn resolve_associated_item<'tcx>( let vtbl = match tcx.codegen_select_candidate((param_env, trait_ref)) { Ok(vtbl) => vtbl, - Err(CodegenObligationError::Ambiguity) => { - let reported = tcx.dcx().span_delayed_bug( - tcx.def_span(trait_item_id), - format!( - "encountered ambiguity selecting `{trait_ref:?}` during codegen, presuming due to \ - overflow or prior type error", - ), - ); - return Err(reported); - } - Err(CodegenObligationError::Unimplemented) => return Ok(None), - Err(CodegenObligationError::FulfillmentError) => return Ok(None), + Err( + CodegenObligationError::Ambiguity + | CodegenObligationError::Unimplemented + | CodegenObligationError::FulfillmentError, + ) => return Ok(None), }; // Now that we know which impl is being used, we can dispatch to diff --git a/tests/ui/issues/issue-69602-type-err-during-codegen-ice.rs b/tests/ui/issues/issue-69602-type-err-during-codegen-ice.rs index e98affc5cc299..2c5257ce063cb 100644 --- a/tests/ui/issues/issue-69602-type-err-during-codegen-ice.rs +++ b/tests/ui/issues/issue-69602-type-err-during-codegen-ice.rs @@ -19,5 +19,4 @@ impl TraitB for B { //~ ERROR not all trait items implemented, missing: `MyA` fn main() { let _ = [0; B::VALUE]; - //~^ constant } diff --git a/tests/ui/issues/issue-69602-type-err-during-codegen-ice.stderr b/tests/ui/issues/issue-69602-type-err-during-codegen-ice.stderr index 6f9302bc4a549..fa1d7dffbd49a 100644 --- a/tests/ui/issues/issue-69602-type-err-during-codegen-ice.stderr +++ b/tests/ui/issues/issue-69602-type-err-during-codegen-ice.stderr @@ -13,12 +13,6 @@ LL | type MyA: TraitA; LL | impl TraitB for B { | ^^^^^^^^^^^^^^^^^ missing `MyA` in implementation -note: erroneous constant encountered - --> $DIR/issue-69602-type-err-during-codegen-ice.rs:21:17 - | -LL | let _ = [0; B::VALUE]; - | ^^^^^^^^ - error: aborting due to 2 previous errors Some errors have detailed explanations: E0046, E0437. diff --git a/tests/ui/traits/next-solver/ambiguous-impl-in-resolve.rs b/tests/ui/traits/next-solver/ambiguous-impl-in-resolve.rs new file mode 100644 index 0000000000000..78dffcbf6abc3 --- /dev/null +++ b/tests/ui/traits/next-solver/ambiguous-impl-in-resolve.rs @@ -0,0 +1,17 @@ +//@ check-pass +//@ compile-flags: -Znext-solver + +trait Local {} + +trait Overlap { fn f(); } +impl Overlap for Option where Self: Clone, { fn f() {} } +impl Overlap for Option where Self: Local, { fn f() {} } + +fn test() +where + Option: Clone + Local, +{ + as Overlap>::f(); +} + +fn main() {} From 386236f289bad37370989866930be95b9067d07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 24 Apr 2024 01:26:32 +0000 Subject: [PATCH 2/8] Detect borrow error involving sub-slices and suggest `split_at_mut` ``` error[E0499]: cannot borrow `foo` as mutable more than once at a time --> $DIR/suggest-split-at-mut.rs:13:18 | LL | let a = &mut foo[..2]; | --- first mutable borrow occurs here LL | let b = &mut foo[2..]; | ^^^ second mutable borrow occurs here LL | a[0] = 5; | ---- first borrow later used here | = help: use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices ``` Address most of #58792. For follow up work, we should emit a structured suggestion for cases where we can identify the exact `let (a, b) = foo.split_at_mut(2);` call that is needed. --- .../src/diagnostics/conflict_errors.rs | 41 +++++++++++-------- ...borrowck-overloaded-index-autoderef.stderr | 4 ++ tests/ui/suggestions/suggest-split-at-mut.rs | 16 +++++++- .../suggestions/suggest-split-at-mut.stderr | 14 ++++++- 4 files changed, 57 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 40c49dcfab61c..fae7f189bfd8c 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1581,6 +1581,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { &mut err, place, issued_borrow.borrowed_place, + span, + issued_span, ); self.suggest_using_closure_argument_instead_of_capture( &mut err, @@ -2011,10 +2013,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { err: &mut Diag<'_>, place: Place<'tcx>, borrowed_place: Place<'tcx>, + span: Span, + issued_span: Span, ) { let tcx = self.infcx.tcx; let hir = tcx.hir(); - if let ([ProjectionElem::Index(index1)], [ProjectionElem::Index(index2)]) | ( [ProjectionElem::Deref, ProjectionElem::Index(index1)], @@ -2023,28 +2026,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { { let mut note_default_suggestion = || { err.help( - "consider using `.split_at_mut(position)` or similar method to obtain \ - two mutable non-overlapping sub-slices", + "consider using `.split_at_mut(position)` or similar method to obtain two \ + mutable non-overlapping sub-slices", ) - .help("consider using `.swap(index_1, index_2)` to swap elements at the specified indices"); - }; - - let Some(body_id) = tcx.hir_node(self.mir_hir_id()).body_id() else { - note_default_suggestion(); - return; + .help( + "consider using `.swap(index_1, index_2)` to swap elements at the specified \ + indices", + ); }; - let mut expr_finder = - FindExprBySpan::new(self.body.local_decls[*index1].source_info.span, tcx); - expr_finder.visit_expr(hir.body(body_id).value); - let Some(index1) = expr_finder.result else { + let Some(index1) = self.find_expr(self.body.local_decls[*index1].source_info.span) + else { note_default_suggestion(); return; }; - expr_finder = FindExprBySpan::new(self.body.local_decls[*index2].source_info.span, tcx); - expr_finder.visit_expr(hir.body(body_id).value); - let Some(index2) = expr_finder.result else { + let Some(index2) = self.find_expr(self.body.local_decls[*index2].source_info.span) + else { note_default_suggestion(); return; }; @@ -2102,7 +2100,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { format!("{obj_str}.swap({index1_str}, {index2_str})"), Applicability::MachineApplicable, ); + return; } + let Some(index1) = self.find_expr(span) else { return }; + let hir::Node::Expr(parent) = tcx.parent_hir_node(index1.hir_id) else { return }; + let hir::ExprKind::Index(..) = parent.kind else { return }; + let Some(index2) = self.find_expr(issued_span) else { return }; + let hir::Node::Expr(parent) = tcx.parent_hir_node(index2.hir_id) else { return }; + let hir::ExprKind::Index(..) = parent.kind else { return }; + err.help( + "use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping \ + sub-slices", + ); } /// Suggest using `while let` for call `next` on an iterator in a for loop. diff --git a/tests/ui/borrowck/borrowck-overloaded-index-autoderef.stderr b/tests/ui/borrowck/borrowck-overloaded-index-autoderef.stderr index 2e7a6778dbc30..44a9a6ac81146 100644 --- a/tests/ui/borrowck/borrowck-overloaded-index-autoderef.stderr +++ b/tests/ui/borrowck/borrowck-overloaded-index-autoderef.stderr @@ -17,6 +17,8 @@ LL | let q = &mut f[&s]; | ^ second mutable borrow occurs here LL | p.use_mut(); | - first borrow later used here + | + = help: use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices error[E0499]: cannot borrow `f.foo` as mutable more than once at a time --> $DIR/borrowck-overloaded-index-autoderef.rs:53:18 @@ -27,6 +29,8 @@ LL | let q = &mut f.foo[&s]; | ^^^^^ second mutable borrow occurs here LL | p.use_mut(); | - first borrow later used here + | + = help: use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices error[E0502]: cannot borrow `f.foo` as mutable because it is also borrowed as immutable --> $DIR/borrowck-overloaded-index-autoderef.rs:65:18 diff --git a/tests/ui/suggestions/suggest-split-at-mut.rs b/tests/ui/suggestions/suggest-split-at-mut.rs index d294c20b8240e..a80d35b19515f 100644 --- a/tests/ui/suggestions/suggest-split-at-mut.rs +++ b/tests/ui/suggestions/suggest-split-at-mut.rs @@ -1,4 +1,4 @@ -fn main() { +fn foo() { let mut foo = [1, 2, 3, 4]; let a = &mut foo[2]; let b = &mut foo[3]; //~ ERROR cannot borrow `foo[_]` as mutable more than once at a time @@ -6,3 +6,17 @@ fn main() { *b = 6; println!("{:?} {:?}", a, b); } + +fn bar() { + let mut foo = [1,2,3,4]; + let a = &mut foo[..2]; + let b = &mut foo[2..]; //~ ERROR cannot borrow `foo` as mutable more than once at a time + a[0] = 5; + b[0] = 6; + println!("{:?} {:?}", a, b); +} + +fn main() { + foo(); + bar(); +} diff --git a/tests/ui/suggestions/suggest-split-at-mut.stderr b/tests/ui/suggestions/suggest-split-at-mut.stderr index c42f09e3201db..f7d04b1db01d6 100644 --- a/tests/ui/suggestions/suggest-split-at-mut.stderr +++ b/tests/ui/suggestions/suggest-split-at-mut.stderr @@ -11,6 +11,18 @@ LL | *a = 5; = help: consider using `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices = help: consider using `.swap(index_1, index_2)` to swap elements at the specified indices -error: aborting due to 1 previous error +error[E0499]: cannot borrow `foo` as mutable more than once at a time + --> $DIR/suggest-split-at-mut.rs:13:18 + | +LL | let a = &mut foo[..2]; + | --- first mutable borrow occurs here +LL | let b = &mut foo[2..]; + | ^^^ second mutable borrow occurs here +LL | a[0] = 5; + | ---- first borrow later used here + | + = help: use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0499`. From dbaa4e21483e0e767c2746f0351d11be6361e20a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 24 Apr 2024 18:00:03 +0000 Subject: [PATCH 3/8] Only suggest `split_at_mut` on indexing borrowck errors for std types --- .../src/diagnostics/conflict_errors.rs | 30 ++++++++++++++----- ...borrowck-overloaded-index-autoderef.stderr | 4 --- .../suggestions/suggest-split-at-mut.stderr | 2 +- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index fae7f189bfd8c..832f3b92fa9da 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -2018,12 +2018,25 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) { let tcx = self.infcx.tcx; let hir = tcx.hir(); + + let has_split_at_mut = |ty: Ty<'tcx>| { + let ty = ty.peel_refs(); + match ty.kind() { + ty::Array(..) | ty::Slice(..) => true, + ty::Adt(def, _) if tcx.get_diagnostic_item(sym::Vec) == Some(def.did()) => true, + _ if ty == tcx.types.str_ => true, + _ => false, + } + }; if let ([ProjectionElem::Index(index1)], [ProjectionElem::Index(index2)]) | ( [ProjectionElem::Deref, ProjectionElem::Index(index1)], [ProjectionElem::Deref, ProjectionElem::Index(index2)], ) = (&place.projection[..], &borrowed_place.projection[..]) { + let decl1 = &self.body.local_decls[*index1]; + let decl2 = &self.body.local_decls[*index2]; + let mut note_default_suggestion = || { err.help( "consider using `.split_at_mut(position)` or similar method to obtain two \ @@ -2035,14 +2048,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); }; - let Some(index1) = self.find_expr(self.body.local_decls[*index1].source_info.span) - else { + let Some(index1) = self.find_expr(decl1.source_info.span) else { note_default_suggestion(); return; }; - let Some(index2) = self.find_expr(self.body.local_decls[*index2].source_info.span) - else { + let Some(index2) = self.find_expr(decl2.source_info.span) else { note_default_suggestion(); return; }; @@ -2102,16 +2113,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); return; } + let place_ty = PlaceRef::ty(&place.as_ref(), self.body, tcx).ty; + let borrowed_place_ty = PlaceRef::ty(&borrowed_place.as_ref(), self.body, tcx).ty; + if !has_split_at_mut(place_ty) && !has_split_at_mut(borrowed_place_ty) { + // Only mention `split_at_mut` on `Vec`, array and slices. + return; + } let Some(index1) = self.find_expr(span) else { return }; let hir::Node::Expr(parent) = tcx.parent_hir_node(index1.hir_id) else { return }; let hir::ExprKind::Index(..) = parent.kind else { return }; let Some(index2) = self.find_expr(issued_span) else { return }; let hir::Node::Expr(parent) = tcx.parent_hir_node(index2.hir_id) else { return }; let hir::ExprKind::Index(..) = parent.kind else { return }; - err.help( - "use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping \ - sub-slices", - ); + err.help("use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices"); } /// Suggest using `while let` for call `next` on an iterator in a for loop. diff --git a/tests/ui/borrowck/borrowck-overloaded-index-autoderef.stderr b/tests/ui/borrowck/borrowck-overloaded-index-autoderef.stderr index 44a9a6ac81146..2e7a6778dbc30 100644 --- a/tests/ui/borrowck/borrowck-overloaded-index-autoderef.stderr +++ b/tests/ui/borrowck/borrowck-overloaded-index-autoderef.stderr @@ -17,8 +17,6 @@ LL | let q = &mut f[&s]; | ^ second mutable borrow occurs here LL | p.use_mut(); | - first borrow later used here - | - = help: use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices error[E0499]: cannot borrow `f.foo` as mutable more than once at a time --> $DIR/borrowck-overloaded-index-autoderef.rs:53:18 @@ -29,8 +27,6 @@ LL | let q = &mut f.foo[&s]; | ^^^^^ second mutable borrow occurs here LL | p.use_mut(); | - first borrow later used here - | - = help: use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices error[E0502]: cannot borrow `f.foo` as mutable because it is also borrowed as immutable --> $DIR/borrowck-overloaded-index-autoderef.rs:65:18 diff --git a/tests/ui/suggestions/suggest-split-at-mut.stderr b/tests/ui/suggestions/suggest-split-at-mut.stderr index f7d04b1db01d6..1bae7dc475ee7 100644 --- a/tests/ui/suggestions/suggest-split-at-mut.stderr +++ b/tests/ui/suggestions/suggest-split-at-mut.stderr @@ -21,7 +21,7 @@ LL | let b = &mut foo[2..]; LL | a[0] = 5; | ---- first borrow later used here | - = help: use `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices + = help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices error: aborting due to 2 previous errors From 9f9f0aa534384e84f92daa2ebe250318e58049a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 24 Apr 2024 18:14:37 +0000 Subject: [PATCH 4/8] Mention `split_at_mut` when mixing mutability in indexing ops Emit suggestion when encountering ```rust let a = &mut foo[0]; let b = &foo[1]; a.use_mut(); ``` --- .../src/diagnostics/conflict_errors.rs | 19 ++++++- .../borrowck/borrowck-assign-comp-idx.stderr | 2 + tests/ui/suggestions/suggest-split-at-mut.rs | 32 +++++++++++ .../suggestions/suggest-split-at-mut.stderr | 57 ++++++++++++++++++- 4 files changed, 106 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 832f3b92fa9da..9216f9e4971d9 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1527,7 +1527,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow }, ) => { first_borrow_desc = "mutable "; - self.cannot_reborrow_already_borrowed( + let mut err = self.cannot_reborrow_already_borrowed( span, &desc_place, &msg_place, @@ -1537,7 +1537,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { "mutable", &msg_borrow, None, - ) + ); + self.suggest_slice_method_if_applicable( + &mut err, + place, + issued_borrow.borrowed_place, + span, + issued_span, + ); + err } ( BorrowKind::Mut { kind: MutBorrowKind::Default | MutBorrowKind::TwoPhaseBorrow }, @@ -1555,6 +1563,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { &msg_borrow, None, ); + self.suggest_slice_method_if_applicable( + &mut err, + place, + issued_borrow.borrowed_place, + span, + issued_span, + ); self.suggest_binding_for_closure_capture_self(&mut err, &issued_spans); self.suggest_using_closure_argument_instead_of_capture( &mut err, diff --git a/tests/ui/borrowck/borrowck-assign-comp-idx.stderr b/tests/ui/borrowck/borrowck-assign-comp-idx.stderr index b80174ae6872e..914c79ecee552 100644 --- a/tests/ui/borrowck/borrowck-assign-comp-idx.stderr +++ b/tests/ui/borrowck/borrowck-assign-comp-idx.stderr @@ -9,6 +9,8 @@ LL | p[0] = 5; LL | LL | println!("{}", *q); | -- immutable borrow later used here + | + = help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices error[E0502]: cannot borrow `p` as mutable because it is also borrowed as immutable --> $DIR/borrowck-assign-comp-idx.rs:27:9 diff --git a/tests/ui/suggestions/suggest-split-at-mut.rs b/tests/ui/suggestions/suggest-split-at-mut.rs index a80d35b19515f..93f9120048be3 100644 --- a/tests/ui/suggestions/suggest-split-at-mut.rs +++ b/tests/ui/suggestions/suggest-split-at-mut.rs @@ -16,6 +16,38 @@ fn bar() { println!("{:?} {:?}", a, b); } +fn baz() { + let mut foo = [1,2,3,4]; + let a = &foo[..2]; + let b = &mut foo[2..]; //~ ERROR cannot borrow `foo` as mutable because it is also borrowed as immutable + b[0] = 6; + println!("{:?} {:?}", a, b); +} + +fn qux() { + let mut foo = [1,2,3,4]; + let a = &mut foo[..2]; + let b = &foo[2..]; //~ ERROR cannot borrow `foo` as immutable because it is also borrowed as mutable + a[0] = 5; + println!("{:?} {:?}", a, b); +} + +fn bad() { + let mut foo = [1,2,3,4]; + let a = &foo[1]; + let b = &mut foo[2]; //~ ERROR cannot borrow `foo[_]` as mutable because it is also borrowed as immutable + *b = 6; + println!("{:?} {:?}", a, b); +} + +fn bat() { + let mut foo = [1,2,3,4]; + let a = &mut foo[1]; + let b = &foo[2]; //~ ERROR cannot borrow `foo[_]` as immutable because it is also borrowed as mutable + *a = 5; + println!("{:?} {:?}", a, b); +} + fn main() { foo(); bar(); diff --git a/tests/ui/suggestions/suggest-split-at-mut.stderr b/tests/ui/suggestions/suggest-split-at-mut.stderr index 1bae7dc475ee7..68b1ddc45c742 100644 --- a/tests/ui/suggestions/suggest-split-at-mut.stderr +++ b/tests/ui/suggestions/suggest-split-at-mut.stderr @@ -23,6 +23,59 @@ LL | a[0] = 5; | = help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices -error: aborting due to 2 previous errors +error[E0502]: cannot borrow `foo` as mutable because it is also borrowed as immutable + --> $DIR/suggest-split-at-mut.rs:22:18 + | +LL | let a = &foo[..2]; + | --- immutable borrow occurs here +LL | let b = &mut foo[2..]; + | ^^^ mutable borrow occurs here +LL | b[0] = 6; +LL | println!("{:?} {:?}", a, b); + | - immutable borrow later used here + | + = help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices + +error[E0502]: cannot borrow `foo` as immutable because it is also borrowed as mutable + --> $DIR/suggest-split-at-mut.rs:30:14 + | +LL | let a = &mut foo[..2]; + | --- mutable borrow occurs here +LL | let b = &foo[2..]; + | ^^^ immutable borrow occurs here +LL | a[0] = 5; + | ---- mutable borrow later used here + | + = help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices + +error[E0502]: cannot borrow `foo[_]` as mutable because it is also borrowed as immutable + --> $DIR/suggest-split-at-mut.rs:38:13 + | +LL | let a = &foo[1]; + | ------- immutable borrow occurs here +LL | let b = &mut foo[2]; + | ^^^^^^^^^^^ mutable borrow occurs here +LL | *b = 6; +LL | println!("{:?} {:?}", a, b); + | - immutable borrow later used here + | + = help: consider using `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices + = help: consider using `.swap(index_1, index_2)` to swap elements at the specified indices + +error[E0502]: cannot borrow `foo[_]` as immutable because it is also borrowed as mutable + --> $DIR/suggest-split-at-mut.rs:46:13 + | +LL | let a = &mut foo[1]; + | ----------- mutable borrow occurs here +LL | let b = &foo[2]; + | ^^^^^^^ immutable borrow occurs here +LL | *a = 5; + | ------ mutable borrow later used here + | + = help: consider using `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices + = help: consider using `.swap(index_1, index_2)` to swap elements at the specified indices + +error: aborting due to 6 previous errors -For more information about this error, try `rustc --explain E0499`. +Some errors have detailed explanations: E0499, E0502. +For more information about an error, try `rustc --explain E0499`. From ad6ae612462c77fa6c5d1c98df9f197c570e0e81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 24 Apr 2024 21:36:10 +0000 Subject: [PATCH 5/8] Don't suggest `split_at_mut` when the multiple borrows have the same index --- .../src/diagnostics/conflict_errors.rs | 8 +++-- compiler/rustc_hir/src/hir.rs | 35 +++++++++++++++++++ .../borrowck/borrowck-assign-comp-idx.stderr | 2 -- tests/ui/suggestions/suggest-split-at-mut.rs | 8 +++++ .../suggestions/suggest-split-at-mut.stderr | 12 ++++++- 5 files changed, 60 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 9216f9e4971d9..84f9ebf18a80f 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -2136,10 +2136,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } let Some(index1) = self.find_expr(span) else { return }; let hir::Node::Expr(parent) = tcx.parent_hir_node(index1.hir_id) else { return }; - let hir::ExprKind::Index(..) = parent.kind else { return }; + let hir::ExprKind::Index(_, idx1, _) = parent.kind else { return }; let Some(index2) = self.find_expr(issued_span) else { return }; let hir::Node::Expr(parent) = tcx.parent_hir_node(index2.hir_id) else { return }; - let hir::ExprKind::Index(..) = parent.kind else { return }; + let hir::ExprKind::Index(_, idx2, _) = parent.kind else { return }; + if idx1.equals(idx2) { + // `let a = &mut foo[0]` and `let b = &mut foo[0]`? Don't mention `split_at_mut` + return; + } err.help("use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices"); } diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 2268905430a7a..1e66d42c227c5 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1811,6 +1811,41 @@ impl Expr<'_> { } } + /// Whether this and the `other` expression are the same for purposes of an indexing operation. + /// + /// This is only used for diagnostics to see if we have things like `foo[i]` where `foo` is + /// borrowed multiple times with `i`. + pub fn equals(&self, other: &Expr<'_>) -> bool { + match (self.kind, other.kind) { + (ExprKind::Lit(lit1), ExprKind::Lit(lit2)) => lit1.node == lit2.node, + ( + ExprKind::Path(QPath::LangItem(item1, _)), + ExprKind::Path(QPath::LangItem(item2, _)), + ) => item1 == item2, + ( + ExprKind::Path(QPath::Resolved(None, path1)), + ExprKind::Path(QPath::Resolved(None, path2)), + ) => path1.res == path2.res, + ( + ExprKind::Struct(QPath::LangItem(LangItem::RangeTo, _), [val1], None), + ExprKind::Struct(QPath::LangItem(LangItem::RangeTo, _), [val2], None), + ) + | ( + ExprKind::Struct(QPath::LangItem(LangItem::RangeToInclusive, _), [val1], None), + ExprKind::Struct(QPath::LangItem(LangItem::RangeToInclusive, _), [val2], None), + ) + | ( + ExprKind::Struct(QPath::LangItem(LangItem::RangeFrom, _), [val1], None), + ExprKind::Struct(QPath::LangItem(LangItem::RangeFrom, _), [val2], None), + ) => val1.expr.equals(val2.expr), + ( + ExprKind::Struct(QPath::LangItem(LangItem::Range, _), [val1, val3], None), + ExprKind::Struct(QPath::LangItem(LangItem::Range, _), [val2, val4], None), + ) => val1.expr.equals(val2.expr) && val3.expr.equals(val4.expr), + _ => false, + } + } + pub fn method_ident(&self) -> Option { match self.kind { ExprKind::MethodCall(receiver_method, ..) => Some(receiver_method.ident), diff --git a/tests/ui/borrowck/borrowck-assign-comp-idx.stderr b/tests/ui/borrowck/borrowck-assign-comp-idx.stderr index 914c79ecee552..b80174ae6872e 100644 --- a/tests/ui/borrowck/borrowck-assign-comp-idx.stderr +++ b/tests/ui/borrowck/borrowck-assign-comp-idx.stderr @@ -9,8 +9,6 @@ LL | p[0] = 5; LL | LL | println!("{}", *q); | -- immutable borrow later used here - | - = help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices error[E0502]: cannot borrow `p` as mutable because it is also borrowed as immutable --> $DIR/borrowck-assign-comp-idx.rs:27:9 diff --git a/tests/ui/suggestions/suggest-split-at-mut.rs b/tests/ui/suggestions/suggest-split-at-mut.rs index 93f9120048be3..61704abbd3649 100644 --- a/tests/ui/suggestions/suggest-split-at-mut.rs +++ b/tests/ui/suggestions/suggest-split-at-mut.rs @@ -48,6 +48,14 @@ fn bat() { println!("{:?} {:?}", a, b); } +fn ang() { + let mut foo = [1,2,3,4]; + let a = &mut foo[0..]; + let b = &foo[0..]; //~ ERROR cannot borrow `foo` as immutable because it is also borrowed as mutable + a[0] = 5; + println!("{:?} {:?}", a, b); +} + fn main() { foo(); bar(); diff --git a/tests/ui/suggestions/suggest-split-at-mut.stderr b/tests/ui/suggestions/suggest-split-at-mut.stderr index 68b1ddc45c742..c1d93367ccb61 100644 --- a/tests/ui/suggestions/suggest-split-at-mut.stderr +++ b/tests/ui/suggestions/suggest-split-at-mut.stderr @@ -75,7 +75,17 @@ LL | *a = 5; = help: consider using `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices = help: consider using `.swap(index_1, index_2)` to swap elements at the specified indices -error: aborting due to 6 previous errors +error[E0502]: cannot borrow `foo` as immutable because it is also borrowed as mutable + --> $DIR/suggest-split-at-mut.rs:54:14 + | +LL | let a = &mut foo[0..]; + | --- mutable borrow occurs here +LL | let b = &foo[0..]; + | ^^^ immutable borrow occurs here +LL | a[0] = 5; + | ---- mutable borrow later used here + +error: aborting due to 7 previous errors Some errors have detailed explanations: E0499, E0502. For more information about an error, try `rustc --explain E0499`. From abdb64d4eaa989ff5ef96099f6e1585472de1622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 24 Apr 2024 21:53:33 +0000 Subject: [PATCH 6/8] Check equivalence of indices in more cases --- .../rustc_borrowck/src/diagnostics/conflict_errors.rs | 8 +++++++- tests/ui/suggestions/suggest-split-at-mut.stderr | 9 +++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 84f9ebf18a80f..b800e8d45d2d8 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -2116,7 +2116,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { None } }) else { - note_default_suggestion(); + let hir::Node::Expr(parent) = tcx.parent_hir_node(index1.hir_id) else { return }; + let hir::ExprKind::Index(_, idx1, _) = parent.kind else { return }; + let hir::Node::Expr(parent) = tcx.parent_hir_node(index2.hir_id) else { return }; + let hir::ExprKind::Index(_, idx2, _) = parent.kind else { return }; + if !idx1.equals(idx2) { + err.help("use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices"); + } return; }; diff --git a/tests/ui/suggestions/suggest-split-at-mut.stderr b/tests/ui/suggestions/suggest-split-at-mut.stderr index c1d93367ccb61..4502ec1f393ba 100644 --- a/tests/ui/suggestions/suggest-split-at-mut.stderr +++ b/tests/ui/suggestions/suggest-split-at-mut.stderr @@ -8,8 +8,7 @@ LL | let b = &mut foo[3]; LL | *a = 5; | ------ first borrow later used here | - = help: consider using `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices - = help: consider using `.swap(index_1, index_2)` to swap elements at the specified indices + = help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices error[E0499]: cannot borrow `foo` as mutable more than once at a time --> $DIR/suggest-split-at-mut.rs:13:18 @@ -59,8 +58,7 @@ LL | *b = 6; LL | println!("{:?} {:?}", a, b); | - immutable borrow later used here | - = help: consider using `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices - = help: consider using `.swap(index_1, index_2)` to swap elements at the specified indices + = help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices error[E0502]: cannot borrow `foo[_]` as immutable because it is also borrowed as mutable --> $DIR/suggest-split-at-mut.rs:46:13 @@ -72,8 +70,7 @@ LL | let b = &foo[2]; LL | *a = 5; | ------ mutable borrow later used here | - = help: consider using `.split_at_mut(position)` or similar method to obtain two mutable non-overlapping sub-slices - = help: consider using `.swap(index_1, index_2)` to swap elements at the specified indices + = help: use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices error[E0502]: cannot borrow `foo` as immutable because it is also borrowed as mutable --> $DIR/suggest-split-at-mut.rs:54:14 From ce705847535d98a2409fc1160b61506d9e6e36fe Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 25 Apr 2024 17:30:51 +0000 Subject: [PATCH 7/8] remove trivial bounds --- library/core/src/iter/adapters/step_by.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/library/core/src/iter/adapters/step_by.rs b/library/core/src/iter/adapters/step_by.rs index 616dd0afc517d..a5cf069d407c0 100644 --- a/library/core/src/iter/adapters/step_by.rs +++ b/library/core/src/iter/adapters/step_by.rs @@ -515,9 +515,7 @@ macro_rules! spec_int_ranges_r { unsafe impl StepByBackImpl> for StepBy> { #[inline] - fn spec_next_back(&mut self) -> Option - where Range<$t>: DoubleEndedIterator + ExactSizeIterator, - { + fn spec_next_back(&mut self) -> Option { let step = self.original_step().get() as $t; let remaining = self.iter.end; if remaining > 0 { @@ -533,9 +531,7 @@ macro_rules! spec_int_ranges_r { // We have to repeat them here so that the specialization overrides the StepByImplBack defaults #[inline] - fn spec_nth_back(&mut self, n: usize) -> Option - where Self: DoubleEndedIterator, - { + fn spec_nth_back(&mut self, n: usize) -> Option { if self.advance_back_by(n).is_err() { return None; } @@ -544,10 +540,9 @@ macro_rules! spec_int_ranges_r { #[inline] fn spec_try_rfold(&mut self, init: Acc, mut f: F) -> R - where - Self: DoubleEndedIterator, - F: FnMut(Acc, Self::Item) -> R, - R: Try + where + F: FnMut(Acc, Self::Item) -> R, + R: Try { let mut accum = init; while let Some(x) = self.next_back() { @@ -558,9 +553,8 @@ macro_rules! spec_int_ranges_r { #[inline] fn spec_rfold(mut self, init: Acc, mut f: F) -> Acc - where - Self: DoubleEndedIterator, - F: FnMut(Acc, Self::Item) -> Acc + where + F: FnMut(Acc, Self::Item) -> Acc { let mut accum = init; while let Some(x) = self.next_back() { From 64a4cdcfd4db72b7fc9b24521111f4f0edac3531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 25 Apr 2024 18:26:36 +0000 Subject: [PATCH 8/8] review comment: rename method --- .../rustc_borrowck/src/diagnostics/conflict_errors.rs | 4 ++-- compiler/rustc_hir/src/hir.rs | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index b800e8d45d2d8..da58db5752526 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -2120,7 +2120,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let hir::ExprKind::Index(_, idx1, _) = parent.kind else { return }; let hir::Node::Expr(parent) = tcx.parent_hir_node(index2.hir_id) else { return }; let hir::ExprKind::Index(_, idx2, _) = parent.kind else { return }; - if !idx1.equals(idx2) { + if !idx1.equivalent_for_indexing(idx2) { err.help("use `.split_at_mut(position)` to obtain two mutable non-overlapping sub-slices"); } return; @@ -2146,7 +2146,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let Some(index2) = self.find_expr(issued_span) else { return }; let hir::Node::Expr(parent) = tcx.parent_hir_node(index2.hir_id) else { return }; let hir::ExprKind::Index(_, idx2, _) = parent.kind else { return }; - if idx1.equals(idx2) { + if idx1.equivalent_for_indexing(idx2) { // `let a = &mut foo[0]` and `let b = &mut foo[0]`? Don't mention `split_at_mut` return; } diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 1e66d42c227c5..1646ea50fb0f1 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1815,7 +1815,7 @@ impl Expr<'_> { /// /// This is only used for diagnostics to see if we have things like `foo[i]` where `foo` is /// borrowed multiple times with `i`. - pub fn equals(&self, other: &Expr<'_>) -> bool { + pub fn equivalent_for_indexing(&self, other: &Expr<'_>) -> bool { match (self.kind, other.kind) { (ExprKind::Lit(lit1), ExprKind::Lit(lit2)) => lit1.node == lit2.node, ( @@ -1837,11 +1837,14 @@ impl Expr<'_> { | ( ExprKind::Struct(QPath::LangItem(LangItem::RangeFrom, _), [val1], None), ExprKind::Struct(QPath::LangItem(LangItem::RangeFrom, _), [val2], None), - ) => val1.expr.equals(val2.expr), + ) => val1.expr.equivalent_for_indexing(val2.expr), ( ExprKind::Struct(QPath::LangItem(LangItem::Range, _), [val1, val3], None), ExprKind::Struct(QPath::LangItem(LangItem::Range, _), [val2, val4], None), - ) => val1.expr.equals(val2.expr) && val3.expr.equals(val4.expr), + ) => { + val1.expr.equivalent_for_indexing(val2.expr) + && val3.expr.equivalent_for_indexing(val4.expr) + } _ => false, } }