Skip to content

Commit 5bfcfee

Browse files
estebankestebank
committed
Merge multiple mutable borrows of immutable binding errors
Fix #53466.
1 parent bb6e76d commit 5bfcfee

File tree

14 files changed

+199
-93
lines changed

14 files changed

+199
-93
lines changed

‎compiler/rustc_borrowck/src/borrowck_errors.rs

+14-14
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
1212
place: &str,
1313
borrow_place: &str,
1414
value_place: &str,
15-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
15+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
1616
self.infcx.tcx.sess.create_err(crate::session_diagnostics::MoveBorrow {
1717
place,
1818
span,
@@ -28,7 +28,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
2828
desc: &str,
2929
borrow_span: Span,
3030
borrow_desc: &str,
31-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
31+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
3232
let mut err = struct_span_err!(
3333
self,
3434
span,
@@ -50,7 +50,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
5050
old_loan_span: Span,
5151
old_opt_via: &str,
5252
old_load_end_span: Option<Span>,
53-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
53+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
5454
let via =
5555
|msg: &str| if msg.is_empty() { "".to_string() } else { format!(" (via {})", msg) };
5656
let mut err = struct_span_err!(
@@ -98,7 +98,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
9898
desc: &str,
9999
old_loan_span: Span,
100100
old_load_end_span: Option<Span>,
101-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
101+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
102102
let mut err = struct_span_err!(
103103
self,
104104
new_loan_span,
@@ -269,7 +269,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
269269
&self,
270270
span: Span,
271271
desc: &str,
272-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
272+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
273273
struct_span_err!(self, span, E0594, "cannot assign to {}", desc)
274274
}
275275

@@ -348,7 +348,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
348348
span: Span,
349349
path: &str,
350350
reason: &str,
351-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
351+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
352352
struct_span_err!(self, span, E0596, "cannot borrow {} as mutable{}", path, reason,)
353353
}
354354

@@ -359,7 +359,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
359359
immutable_place: &str,
360360
immutable_section: &str,
361361
action: &str,
362-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
362+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
363363
let mut err = struct_span_err!(
364364
self,
365365
mutate_span,
@@ -378,7 +378,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
378378
&self,
379379
span: Span,
380380
yield_span: Span,
381-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
381+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
382382
let mut err = struct_span_err!(
383383
self,
384384
span,
@@ -392,7 +392,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
392392
pub(crate) fn cannot_borrow_across_destructor(
393393
&self,
394394
borrow_span: Span,
395-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
395+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
396396
struct_span_err!(
397397
self,
398398
borrow_span,
@@ -405,7 +405,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
405405
&self,
406406
span: Span,
407407
path: &str,
408-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
408+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
409409
struct_span_err!(self, span, E0597, "{} does not live long enough", path,)
410410
}
411411

@@ -415,7 +415,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
415415
return_kind: &str,
416416
reference_desc: &str,
417417
path_desc: &str,
418-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
418+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
419419
let mut err = struct_span_err!(
420420
self,
421421
span,
@@ -440,7 +440,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
440440
closure_kind: &str,
441441
borrowed_path: &str,
442442
capture_span: Span,
443-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
443+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
444444
let mut err = struct_span_err!(
445445
self,
446446
closure_span,
@@ -458,14 +458,14 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
458458
pub(crate) fn thread_local_value_does_not_live_long_enough(
459459
&self,
460460
span: Span,
461-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
461+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
462462
struct_span_err!(self, span, E0712, "thread-local variable borrowed past end of function",)
463463
}
464464

465465
pub(crate) fn temporary_value_borrowed_for_too_long(
466466
&self,
467467
span: Span,
468-
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
468+
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
469469
struct_span_err!(self, span, E0716, "temporary value dropped while borrowed",)
470470
}
471471

‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

+69-21
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
180180
// the verbs used in some diagnostic messages.
181181
let act;
182182
let acted_on;
183+
let mut suggest = true;
184+
let mut mut_error = None;
185+
let mut count = 1;
183186

184187
let span = match error_access {
185188
AccessKind::Mutate => {
@@ -194,15 +197,50 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
194197

195198
let borrow_spans = self.borrow_spans(span, location);
196199
let borrow_span = borrow_spans.args_or_use();
197-
err = self.cannot_borrow_path_as_mutable_because(borrow_span, &item_msg, &reason);
198-
borrow_spans.var_span_label(
199-
&mut err,
200-
format!(
201-
"mutable borrow occurs due to use of {} in closure",
202-
self.describe_any_place(access_place.as_ref()),
203-
),
204-
"mutable",
205-
);
200+
match the_place_err {
201+
PlaceRef { local, projection: [] }
202+
if self.body.local_decls[local].can_be_made_mutable() =>
203+
{
204+
let span = self.body.local_decls[local].source_info.span;
205+
mut_error = Some(span);
206+
if let Some((buffer, c)) = self.get_buffered_mut_error(span) {
207+
// We've encountered a second (or more) attempt to mutably borrow an
208+
// immutable binding, so the likely problem is with the binding
209+
// declaration, not the use. We collect these in a single diagnostic
210+
// and make the binding the primary span of the error.
211+
err = buffer;
212+
count = c + 1;
213+
if count == 2 {
214+
err.replace_span_with(span, false);
215+
err.span_label(span, "not mutable");
216+
}
217+
suggest = false;
218+
} else {
219+
err = self.cannot_borrow_path_as_mutable_because(
220+
borrow_span,
221+
&item_msg,
222+
&reason,
223+
);
224+
}
225+
}
226+
_ => {
227+
err = self.cannot_borrow_path_as_mutable_because(
228+
borrow_span,
229+
&item_msg,
230+
&reason,
231+
);
232+
}
233+
}
234+
if suggest {
235+
borrow_spans.var_span_label(
236+
&mut err,
237+
format!(
238+
"mutable borrow occurs due to use of {} in closure",
239+
self.describe_any_place(access_place.as_ref()),
240+
),
241+
"mutable",
242+
);
243+
}
206244
borrow_span
207245
}
208246
};
@@ -276,7 +314,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
276314
pat_span: _,
277315
},
278316
)))) => {
279-
err.span_note(sp, "the binding is already a mutable borrow");
317+
if suggest {
318+
err.span_note(sp, "the binding is already a mutable borrow");
319+
}
280320
}
281321
_ => {
282322
err.span_note(
@@ -333,16 +373,20 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
333373
let local_decl = &self.body.local_decls[local];
334374
assert_eq!(local_decl.mutability, Mutability::Not);
335375

336-
err.span_label(span, format!("cannot {act}"));
337-
err.span_suggestion(
338-
local_decl.source_info.span,
339-
"consider changing this to be mutable",
340-
format!("mut {}", self.local_names[local].unwrap()),
341-
Applicability::MachineApplicable,
342-
);
343-
let tcx = self.infcx.tcx;
344-
if let ty::Closure(id, _) = *the_place_err.ty(self.body, tcx).ty.kind() {
345-
self.show_mutating_upvar(tcx, id.expect_local(), the_place_err, &mut err);
376+
if count < 10 {
377+
err.span_label(span, format!("cannot {act}"));
378+
}
379+
if suggest {
380+
err.span_suggestion(
381+
local_decl.source_info.span,
382+
"consider changing this to be mutable",
383+
format!("mut {}", self.local_names[local].unwrap()),
384+
Applicability::MachineApplicable,
385+
);
386+
let tcx = self.infcx.tcx;
387+
if let ty::Closure(id, _) = *the_place_err.ty(self.body, tcx).ty.kind() {
388+
self.show_mutating_upvar(tcx, id.expect_local(), the_place_err, &mut err);
389+
}
346390
}
347391
}
348392

@@ -615,7 +659,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
615659
}
616660
}
617661

618-
self.buffer_error(err);
662+
if let Some(span) = mut_error {
663+
self.buffer_mut_error(span, err, count);
664+
} else {
665+
self.buffer_error(err);
666+
}
619667
}
620668

621669
fn suggest_map_index_mut_alternatives(&self, ty: Ty<'tcx>, err: &mut Diagnostic, span: Span) {

‎compiler/rustc_borrowck/src/lib.rs

+24
Original file line numberDiff line numberDiff line change
@@ -2270,6 +2270,7 @@ mod error {
22702270
/// same primary span come out in a consistent order.
22712271
buffered_move_errors:
22722272
BTreeMap<Vec<MoveOutIndex>, (PlaceRef<'tcx>, DiagnosticBuilder<'tcx, ErrorGuaranteed>)>,
2273+
buffered_mut_errors: FxHashMap<Span, (DiagnosticBuilder<'tcx, ErrorGuaranteed>, usize)>,
22732274
/// Diagnostics to be reported buffer.
22742275
buffered: Vec<Diagnostic>,
22752276
/// Set to Some if we emit an error during borrowck
@@ -2281,6 +2282,7 @@ mod error {
22812282
BorrowckErrors {
22822283
tcx,
22832284
buffered_move_errors: BTreeMap::new(),
2285+
buffered_mut_errors: Default::default(),
22842286
buffered: Default::default(),
22852287
tainted_by_errors: None,
22862288
}
@@ -2331,12 +2333,34 @@ mod error {
23312333
}
23322334
}
23332335

2336+
pub fn get_buffered_mut_error(
2337+
&mut self,
2338+
span: Span,
2339+
) -> Option<(DiagnosticBuilder<'tcx, ErrorGuaranteed>, usize)> {
2340+
self.errors.buffered_mut_errors.remove(&span)
2341+
}
2342+
2343+
pub fn buffer_mut_error(
2344+
&mut self,
2345+
span: Span,
2346+
t: DiagnosticBuilder<'tcx, ErrorGuaranteed>,
2347+
count: usize,
2348+
) {
2349+
self.errors.buffered_mut_errors.insert(span, (t, count));
2350+
}
2351+
23342352
pub fn emit_errors(&mut self) -> Option<ErrorGuaranteed> {
23352353
// Buffer any move errors that we collected and de-duplicated.
23362354
for (_, (_, diag)) in std::mem::take(&mut self.errors.buffered_move_errors) {
23372355
// We have already set tainted for this error, so just buffer it.
23382356
diag.buffer(&mut self.errors.buffered);
23392357
}
2358+
for (_, (mut diag, count)) in std::mem::take(&mut self.errors.buffered_mut_errors) {
2359+
if count > 10 {
2360+
diag.note(&format!("...and {} other attempted mutable borrows", count - 10));
2361+
}
2362+
diag.buffer(&mut self.errors.buffered);
2363+
}
23402364

23412365
if !self.errors.buffered.is_empty() {
23422366
self.errors.buffered.sort_by_key(|diag| diag.sort_span);

‎compiler/rustc_errors/src/diagnostic.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -365,12 +365,12 @@ impl Diagnostic {
365365
self
366366
}
367367

368-
pub fn replace_span_with(&mut self, after: Span) -> &mut Self {
368+
pub fn replace_span_with(&mut self, after: Span, keep_label: bool) -> &mut Self {
369369
let before = self.span.clone();
370370
self.set_span(after);
371371
for span_label in before.span_labels() {
372372
if let Some(label) = span_label.label {
373-
if span_label.is_primary {
373+
if span_label.is_primary && keep_label {
374374
self.span.push_span_label(after, label);
375375
} else {
376376
self.span.push_span_label(span_label.span, label);

‎compiler/rustc_expand/src/mbe/diagnostics.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,12 @@ pub(super) fn emit_frag_parse_err(
198198
);
199199
if !e.span.is_dummy() {
200200
// early end of macro arm (#52866)
201-
e.replace_span_with(parser.token.span.shrink_to_hi());
201+
e.replace_span_with(parser.token.span.shrink_to_hi(), true);
202202
}
203203
}
204204
if e.span.is_dummy() {
205205
// Get around lack of span in error (#30128)
206-
e.replace_span_with(site_span);
206+
e.replace_span_with(site_span, true);
207207
if !parser.sess.source_map().is_imported(arm_span) {
208208
e.span_label(arm_span, "in this macro arm");
209209
}

‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3237,7 +3237,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
32373237
})) = call_node
32383238
{
32393239
if Some(rcvr.span) == err.span.primary_span() {
3240-
err.replace_span_with(path.ident.span);
3240+
err.replace_span_with(path.ident.span, true);
32413241
}
32423242
}
32433243
if let Some(Node::Expr(hir::Expr {

‎src/test/ui/asm/x86_64/type-check-5.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@ fn main() {
2222
// Outputs require mutable places
2323

2424
let v: Vec<u64> = vec![0, 1, 2];
25+
//~^ ERROR cannot borrow `v` as mutable, as it is not declared as mutable
2526
asm!("{}", in(reg) v[0]);
2627
asm!("{}", out(reg) v[0]);
27-
//~^ ERROR cannot borrow `v` as mutable, as it is not declared as mutable
2828
asm!("{}", inout(reg) v[0]);
29-
//~^ ERROR cannot borrow `v` as mutable, as it is not declared as mutable
3029

3130
// Sym operands must point to a function or static
3231

‎src/test/ui/asm/x86_64/type-check-5.stderr

+9-13
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,20 @@ LL | let mut y: u64 = 0;
2525
| +++
2626

2727
error[E0596]: cannot borrow `v` as mutable, as it is not declared as mutable
28-
--> $DIR/type-check-5.rs:26:29
28+
--> $DIR/type-check-5.rs:24:13
2929
|
3030
LL | let v: Vec<u64> = vec![0, 1, 2];
31-
| - help: consider changing this to be mutable: `mut v`
32-
LL | asm!("{}", in(reg) v[0]);
33-
LL | asm!("{}", out(reg) v[0]);
34-
| ^ cannot borrow as mutable
35-
36-
error[E0596]: cannot borrow `v` as mutable, as it is not declared as mutable
37-
--> $DIR/type-check-5.rs:28:31
38-
|
39-
LL | let v: Vec<u64> = vec![0, 1, 2];
40-
| - help: consider changing this to be mutable: `mut v`
31+
| ^
32+
| |
33+
| not mutable
34+
| help: consider changing this to be mutable: `mut v`
4135
...
36+
LL | asm!("{}", out(reg) v[0]);
37+
| - cannot borrow as mutable
4238
LL | asm!("{}", inout(reg) v[0]);
43-
| ^ cannot borrow as mutable
39+
| - cannot borrow as mutable
4440

45-
error: aborting due to 4 previous errors
41+
error: aborting due to 3 previous errors
4642

4743
Some errors have detailed explanations: E0381, E0596.
4844
For more information about an error, try `rustc --explain E0381`.

0 commit comments

Comments
 (0)