1
Fork 0

Merge multiple mutable borrows of immutable binding errors

Fix #53466.
This commit is contained in:
Esteban Küber 2022-12-29 22:46:17 -08:00
parent bb6e76df06
commit 5bfcfeee2a
14 changed files with 199 additions and 93 deletions

View file

@ -12,7 +12,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
place: &str,
borrow_place: &str,
value_place: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
self.infcx.tcx.sess.create_err(crate::session_diagnostics::MoveBorrow {
place,
span,
@ -28,7 +28,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
desc: &str,
borrow_span: Span,
borrow_desc: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
span,
@ -50,7 +50,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
old_loan_span: Span,
old_opt_via: &str,
old_load_end_span: Option<Span>,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
let via =
|msg: &str| if msg.is_empty() { "".to_string() } else { format!(" (via {})", msg) };
let mut err = struct_span_err!(
@ -98,7 +98,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
desc: &str,
old_loan_span: Span,
old_load_end_span: Option<Span>,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
new_loan_span,
@ -269,7 +269,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
&self,
span: Span,
desc: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
struct_span_err!(self, span, E0594, "cannot assign to {}", desc)
}
@ -348,7 +348,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
span: Span,
path: &str,
reason: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
struct_span_err!(self, span, E0596, "cannot borrow {} as mutable{}", path, reason,)
}
@ -359,7 +359,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
immutable_place: &str,
immutable_section: &str,
action: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
mutate_span,
@ -378,7 +378,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
&self,
span: Span,
yield_span: Span,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
span,
@ -392,7 +392,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
pub(crate) fn cannot_borrow_across_destructor(
&self,
borrow_span: Span,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
struct_span_err!(
self,
borrow_span,
@ -405,7 +405,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
&self,
span: Span,
path: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
struct_span_err!(self, span, E0597, "{} does not live long enough", path,)
}
@ -415,7 +415,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
return_kind: &str,
reference_desc: &str,
path_desc: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
span,
@ -440,7 +440,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
closure_kind: &str,
borrowed_path: &str,
capture_span: Span,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
closure_span,
@ -458,14 +458,14 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
pub(crate) fn thread_local_value_does_not_live_long_enough(
&self,
span: Span,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
struct_span_err!(self, span, E0712, "thread-local variable borrowed past end of function",)
}
pub(crate) fn temporary_value_borrowed_for_too_long(
&self,
span: Span,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
struct_span_err!(self, span, E0716, "temporary value dropped while borrowed",)
}

View file

@ -180,6 +180,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
// the verbs used in some diagnostic messages.
let act;
let acted_on;
let mut suggest = true;
let mut mut_error = None;
let mut count = 1;
let span = match error_access {
AccessKind::Mutate => {
@ -194,15 +197,50 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
let borrow_spans = self.borrow_spans(span, location);
let borrow_span = borrow_spans.args_or_use();
err = self.cannot_borrow_path_as_mutable_because(borrow_span, &item_msg, &reason);
borrow_spans.var_span_label(
&mut err,
format!(
"mutable borrow occurs due to use of {} in closure",
self.describe_any_place(access_place.as_ref()),
),
"mutable",
);
match the_place_err {
PlaceRef { local, projection: [] }
if self.body.local_decls[local].can_be_made_mutable() =>
{
let span = self.body.local_decls[local].source_info.span;
mut_error = Some(span);
if let Some((buffer, c)) = self.get_buffered_mut_error(span) {
// We've encountered a second (or more) attempt to mutably borrow an
// immutable binding, so the likely problem is with the binding
// declaration, not the use. We collect these in a single diagnostic
// and make the binding the primary span of the error.
err = buffer;
count = c + 1;
if count == 2 {
err.replace_span_with(span, false);
err.span_label(span, "not mutable");
}
suggest = false;
} else {
err = self.cannot_borrow_path_as_mutable_because(
borrow_span,
&item_msg,
&reason,
);
}
}
_ => {
err = self.cannot_borrow_path_as_mutable_because(
borrow_span,
&item_msg,
&reason,
);
}
}
if suggest {
borrow_spans.var_span_label(
&mut err,
format!(
"mutable borrow occurs due to use of {} in closure",
self.describe_any_place(access_place.as_ref()),
),
"mutable",
);
}
borrow_span
}
};
@ -276,7 +314,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
pat_span: _,
},
)))) => {
err.span_note(sp, "the binding is already a mutable borrow");
if suggest {
err.span_note(sp, "the binding is already a mutable borrow");
}
}
_ => {
err.span_note(
@ -333,16 +373,20 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
let local_decl = &self.body.local_decls[local];
assert_eq!(local_decl.mutability, Mutability::Not);
err.span_label(span, format!("cannot {act}"));
err.span_suggestion(
local_decl.source_info.span,
"consider changing this to be mutable",
format!("mut {}", self.local_names[local].unwrap()),
Applicability::MachineApplicable,
);
let tcx = self.infcx.tcx;
if let ty::Closure(id, _) = *the_place_err.ty(self.body, tcx).ty.kind() {
self.show_mutating_upvar(tcx, id.expect_local(), the_place_err, &mut err);
if count < 10 {
err.span_label(span, format!("cannot {act}"));
}
if suggest {
err.span_suggestion(
local_decl.source_info.span,
"consider changing this to be mutable",
format!("mut {}", self.local_names[local].unwrap()),
Applicability::MachineApplicable,
);
let tcx = self.infcx.tcx;
if let ty::Closure(id, _) = *the_place_err.ty(self.body, tcx).ty.kind() {
self.show_mutating_upvar(tcx, id.expect_local(), the_place_err, &mut err);
}
}
}
@ -615,7 +659,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
}
self.buffer_error(err);
if let Some(span) = mut_error {
self.buffer_mut_error(span, err, count);
} else {
self.buffer_error(err);
}
}
fn suggest_map_index_mut_alternatives(&self, ty: Ty<'tcx>, err: &mut Diagnostic, span: Span) {

View file

@ -2270,6 +2270,7 @@ mod error {
/// same primary span come out in a consistent order.
buffered_move_errors:
BTreeMap<Vec<MoveOutIndex>, (PlaceRef<'tcx>, DiagnosticBuilder<'tcx, ErrorGuaranteed>)>,
buffered_mut_errors: FxHashMap<Span, (DiagnosticBuilder<'tcx, ErrorGuaranteed>, usize)>,
/// Diagnostics to be reported buffer.
buffered: Vec<Diagnostic>,
/// Set to Some if we emit an error during borrowck
@ -2281,6 +2282,7 @@ mod error {
BorrowckErrors {
tcx,
buffered_move_errors: BTreeMap::new(),
buffered_mut_errors: Default::default(),
buffered: Default::default(),
tainted_by_errors: None,
}
@ -2331,12 +2333,34 @@ mod error {
}
}
pub fn get_buffered_mut_error(
&mut self,
span: Span,
) -> Option<(DiagnosticBuilder<'tcx, ErrorGuaranteed>, usize)> {
self.errors.buffered_mut_errors.remove(&span)
}
pub fn buffer_mut_error(
&mut self,
span: Span,
t: DiagnosticBuilder<'tcx, ErrorGuaranteed>,
count: usize,
) {
self.errors.buffered_mut_errors.insert(span, (t, count));
}
pub fn emit_errors(&mut self) -> Option<ErrorGuaranteed> {
// Buffer any move errors that we collected and de-duplicated.
for (_, (_, diag)) in std::mem::take(&mut self.errors.buffered_move_errors) {
// We have already set tainted for this error, so just buffer it.
diag.buffer(&mut self.errors.buffered);
}
for (_, (mut diag, count)) in std::mem::take(&mut self.errors.buffered_mut_errors) {
if count > 10 {
diag.note(&format!("...and {} other attempted mutable borrows", count - 10));
}
diag.buffer(&mut self.errors.buffered);
}
if !self.errors.buffered.is_empty() {
self.errors.buffered.sort_by_key(|diag| diag.sort_span);