Skip to content

most borrowck diagnostic migration #101686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

most borrowck diagnostic migration #101686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b000803
borrowck some amount of diag migration
AndyJado Sep 20, 2022
ea4519e
opaque types descr
AndyJado Sep 20, 2022
0b411c8
according PR#10558 derive name changed
AndyJado Sep 25, 2022
d536c17
why fail?
AndyJado Sep 25, 2022
9768960
ui-test conflict err
AndyJado Sep 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/borrow_set.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
use crate::nll::ToRegionVid;
use crate::path_utils::allow_two_phase_borrow;
use crate::place_ext::PlaceExt;
Expand Down
90 changes: 26 additions & 64 deletions compiler/rustc_borrowck/src/borrowck_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@ use rustc_errors::{
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::Span;

use crate::session_diagnostics::{
AssignBorrowErr, BorrowAcrossDestructor, BorrowAcrossGeneratorYield, ClosureConstructLabel,
InteriorDropMoveErr, MoveBorrowedErr, PathShortLive, TwoClosuresUniquelyBorrowErr,
UseMutBorrowErr,
};

impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
pub(crate) fn cannot_move_when_borrowed(
&self,
span: Span,
desc: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
struct_span_err!(self, span, E0505, "cannot move out of {} because it is borrowed", desc,)
self.infcx.tcx.sess.create_err(MoveBorrowedErr { desc, span })
}

pub(crate) fn cannot_use_when_mutably_borrowed(
Expand All @@ -20,17 +26,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
borrow_span: Span,
borrow_desc: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
span,
E0503,
"cannot use {} because it was mutably borrowed",
desc,
);

err.span_label(borrow_span, format!("borrow of {} occurs here", borrow_desc));
err.span_label(span, format!("use of borrowed {}", borrow_desc));
err
self.infcx.tcx.sess.create_err(UseMutBorrowErr { desc, borrow_desc, span, borrow_span })
}

pub(crate) fn cannot_mutably_borrow_multiply(
Expand Down Expand Up @@ -90,26 +86,22 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
old_loan_span: Span,
old_load_end_span: Option<Span>,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
new_loan_span,
E0524,
"two closures require unique access to {} at the same time",
desc,
);
let case: ClosureConstructLabel;
let diff_span: Option<Span>;
if old_loan_span == new_loan_span {
err.span_label(
old_loan_span,
"closures are constructed here in different iterations of loop",
);
case = ClosureConstructLabel::Both { old_loan_span };
diff_span = None;
} else {
err.span_label(old_loan_span, "first closure is constructed here");
err.span_label(new_loan_span, "second closure is constructed here");
case = ClosureConstructLabel::First { old_loan_span };
diff_span = Some(new_loan_span);
}
Comment on lines +89 to 97
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

let (case, diff_span) = if /* .. */ {
    /* .. */
} else {
    /* .. */
};

I think this would be tidier if it was written this way.

if let Some(old_load_end_span) = old_load_end_span {
err.span_label(old_load_end_span, "borrow from first closure ends here");
}
err
self.infcx.tcx.sess.create_err(TwoClosuresUniquelyBorrowErr {
desc,
case,
new_loan_span,
old_load_end_span,
diff_span,
})
}

pub(crate) fn cannot_uniquely_borrow_by_one_closure(
Expand Down Expand Up @@ -233,17 +225,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
borrow_span: Span,
desc: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
span,
E0506,
"cannot assign to {} because it is borrowed",
desc,
);

err.span_label(borrow_span, format!("borrow of {} occurs here", desc));
err.span_label(span, format!("assignment to borrowed {} occurs here", desc));
err
self.infcx.tcx.sess.create_err(AssignBorrowErr { desc, span, borrow_span })
}

pub(crate) fn cannot_reassign_immutable(
Expand Down Expand Up @@ -303,15 +285,7 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
move_from_span: Span,
container_ty: Ty<'_>,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
move_from_span,
E0509,
"cannot move out of type `{}`, which implements the `Drop` trait",
container_ty,
);
err.span_label(move_from_span, "cannot move out of here");
err
self.infcx.tcx.sess.create_err(InteriorDropMoveErr { container_ty, move_from_span })
}

pub(crate) fn cannot_act_on_moved_value(
Expand Down Expand Up @@ -370,34 +344,22 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
span: Span,
yield_span: Span,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
span,
E0626,
"borrow may still be in use when generator yields",
);
err.span_label(yield_span, "possible yield occurs here");
err
self.infcx.tcx.sess.create_err(BorrowAcrossGeneratorYield { span, yield_span })
}

pub(crate) fn cannot_borrow_across_destructor(
&self,
borrow_span: Span,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
struct_span_err!(
self,
borrow_span,
E0713,
"borrow may still be in use when destructor runs",
)
self.infcx.tcx.sess.create_err(BorrowAcrossDestructor { borrow_span })
}

pub(crate) fn path_does_not_live_long_enough(
&self,
span: Span,
path: &str,
) -> DiagnosticBuilder<'cx, ErrorGuaranteed> {
struct_span_err!(self, span, E0597, "{} does not live long enough", path,)
self.infcx.tcx.sess.create_err(PathShortLive { path, span })
}

pub(crate) fn cannot_return_reference_to_local(
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/constraint_generation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
use rustc_infer::infer::InferCtxt;
use rustc_middle::mir::visit::TyContext;
use rustc_middle::mir::visit::Visitor;
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_borrowck/src/constraints/graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]

use rustc_data_structures::graph;
use rustc_index::vec::IndexVec;
use rustc_middle::mir::ConstraintCategory;
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_borrowck/src/constraints/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]

use rustc_data_structures::graph::scc::Sccs;
use rustc_index::vec::IndexVec;
use rustc_middle::mir::ConstraintCategory;
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/consumers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
//! This file provides API for compiler consumers.

use rustc_hir::def_id::LocalDefId;
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/dataflow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
use rustc_data_structures::fx::FxHashMap;
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::{self, BasicBlock, Body, Location, Place};
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/def_use.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
use rustc_middle::mir::visit::{
MutatingUseContext, NonMutatingUseContext, NonUseContext, PlaceContext,
};
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]

use rustc_errors::{DiagnosticBuilder, ErrorGuaranteed};
use rustc_infer::infer::canonical::Canonical;
use rustc_infer::infer::error_reporting::nice_region_error::NiceRegionError;
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let move_spans = self.move_spans(moved_place.as_ref(), move_out.source);
let move_span = move_spans.args_or_use();

let move_msg = if move_spans.for_closure() { " into closure" } else { "" };
let move_msg = if move_spans.for_closure() { "InClosure" } else { "" };

let loop_message = if location == move_out.source || move_site.traversed_back_edge {
", in previous iteration of loop"
"InLoop"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better if we could avoid having strings like these and dispatching on them - it isn't easy for translators to know what possible values there are.

} else {
""
};
Expand Down
103 changes: 41 additions & 62 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use rustc_span::symbol::{kw, Symbol};
use rustc_span::{sym, DesugaringKind, Span};

use crate::region_infer::{BlameConstraint, ExtraConstraintInfo};
use crate::session_diagnostics::{BorrowUsedHere, UsedLaterDropped};
use crate::{
borrow_set::BorrowData, nll::ConstraintDescription, region_infer::Cause, MirBorrowckCtxt,
WriteKind,
Expand Down Expand Up @@ -66,6 +67,12 @@ impl<'tcx> BorrowExplanation<'tcx> {
borrow_span: Option<Span>,
multiple_borrow_span: Option<(Span, Span)>,
) {
let br_desc = match borrow_desc {
"first " => "first",
"immutable " => "immutable",
"mutable " => "mutable",
_ => "",
};
match *self {
BorrowExplanation::UsedLater(later_use_kind, var_or_use_span, path_span) => {
let message = match later_use_kind {
Expand Down Expand Up @@ -120,13 +127,12 @@ impl<'tcx> BorrowExplanation<'tcx> {
// path_span is only present in the case of closure capture
assert!(matches!(later_use_kind, LaterUseKind::ClosureCapture));
if borrow_span.map(|sp| !sp.overlaps(var_or_use_span)).unwrap_or(true) {
let path_label = "used here by closure";
err.subdiagnostic(BorrowUsedHere::ByClosure { path_span });
let capture_kind_label = message;
err.span_label(
var_or_use_span,
format!("{}borrow later {}", borrow_desc, capture_kind_label),
);
err.span_label(path_span, path_label);
}
}
}
Expand All @@ -145,59 +151,47 @@ impl<'tcx> BorrowExplanation<'tcx> {
}
}
}
let (dtor_desc, type_desc) = match ty.kind() {
let (dtor_code, type_code, type_desc) = match ty.kind() {
// If type is an ADT that implements Drop, then
// simplify output by reporting just the ADT name.
ty::Adt(adt, _substs) if adt.has_dtor(tcx) && !adt.is_box() => {
("`Drop` code", format!("type `{}`", tcx.def_path_str(adt.did())))
(0, 0, format!(" `{}`", tcx.def_path_str(adt.did()).to_string()))
}

// Otherwise, just report the whole type (and use
// the intentionally fuzzy phrase "destructor")
ty::Closure(..) => ("destructor", "closure".to_owned()),
ty::Generator(..) => ("destructor", "generator".to_owned()),
ty::Closure(..) => (1, 1, "".to_owned()),
ty::Generator(..) => (1, 2, "".to_owned()),

_ => ("destructor", format!("type `{}`", local_decl.ty)),
_ => (1, 0, format!(" `{}`", local_decl.ty.to_string())),
};

match local_names[dropped_local] {
Some(local_name) if !local_decl.from_compiler_desugaring() => {
let message = format!(
"{B}borrow might be used here, when `{LOC}` is dropped \
and runs the {DTOR} for {TYPE}",
B = borrow_desc,
LOC = local_name,
TYPE = type_desc,
DTOR = dtor_desc
);
err.span_label(body.source_info(drop_loc).span, message);

err.subdiagnostic(UsedLaterDropped::UsedHere {
borrow_desc: br_desc,
local_name,
type_desc,
type_code,
dtor_code,
span: body.source_info(drop_loc).span,
});
if should_note_order {
err.note(
"values in a scope are dropped \
in the opposite order they are defined",
);
err.subdiagnostic(UsedLaterDropped::OppositeOrder);
}
}
_ => {
err.span_label(
local_decl.source_info.span,
format!(
"a temporary with access to the {B}borrow \
is created here ...",
B = borrow_desc
),
);
let message = format!(
"... and the {B}borrow might be used here, \
when that temporary is dropped \
and runs the {DTOR} for {TYPE}",
B = borrow_desc,
TYPE = type_desc,
DTOR = dtor_desc
);
err.span_label(body.source_info(drop_loc).span, message);

err.subdiagnostic(UsedLaterDropped::TemporaryCreatedHere {
borrow_desc: br_desc,
span: local_decl.source_info.span,
});
err.subdiagnostic(UsedLaterDropped::MightUsedHere {
borrow_desc: br_desc,
type_desc,
type_code,
dtor_code,
span: body.source_info(drop_loc).span,
});
if let Some(info) = &local_decl.is_block_tail {
if info.tail_result_is_ignored {
// #85581: If the first mutable borrow's scope contains
Expand All @@ -208,31 +202,16 @@ impl<'tcx> BorrowExplanation<'tcx> {
})
.unwrap_or(false)
{
err.span_suggestion_verbose(
info.span.shrink_to_hi(),
"consider adding semicolon after the expression so its \
temporaries are dropped sooner, before the local variables \
declared by the block are dropped",
";",
Applicability::MaybeIncorrect,
);
err.subdiagnostic(UsedLaterDropped::AddSemicolon {
span: info.span.shrink_to_hi(),
});
}
} else {
err.note(
"the temporary is part of an expression at the end of a \
block;\nconsider forcing this temporary to be dropped sooner, \
before the block's local variables are dropped",
);
err.multipart_suggestion(
"for example, you could save the expression's value in a new \
local variable `x` and then make `x` be the expression at the \
end of the block",
vec![
(info.span.shrink_to_lo(), "let x = ".to_string()),
(info.span.shrink_to_hi(), "; x".to_string()),
],
Applicability::MaybeIncorrect,
);
err.subdiagnostic(UsedLaterDropped::ManualDrop);
err.subdiagnostic(UsedLaterDropped::MoveBlockEnd {
lo_span: info.span.shrink_to_lo(),
hi_span: info.span.shrink_to_hi(),
});
};
}
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/find_all_local_uses.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]

use std::collections::BTreeSet;

use rustc_middle::mir::visit::{PlaceContext, Visitor};
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/find_use.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]

use std::collections::VecDeque;
use std::rc::Rc;

Expand Down
Loading