rust-lang/rust
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Closure capture borrow diagnostics for disjoint captures

Browse Source
This commit is contained in:
Chris Pardy
2021-03-17 02:51:27 -04:00
parent 6e2a34474b
commit 08c4fbcea7
56 changed files with 532 additions and 152 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs
View File

@@ -99,7 +99,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
err.span_label(span, format!("use of possibly-uninitialized {}", item_msg));
use_spans.var_span_label(
use_spans.var_span_label_path_only(
&mut err,
format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()),
);
@@ -255,6 +255,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
partially_str,
move_spans.describe()
),
"moved",
);
}
}
@@ -304,7 +305,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}
use_spans.var_span_label(
use_spans.var_span_label_path_only(
&mut err,
format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()),
);
@@ -434,13 +435,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
err.span_label(borrow_span, format!("borrow of {} occurs here", borrow_msg));
err.span_label(span, format!("move out of {} occurs here", value_msg));
borrow_spans.var_span_label(
borrow_spans.var_span_label_path_only(
&mut err,
format!("borrow occurs due to use{}", borrow_spans.describe()),
);
move_spans
.var_span_label(&mut err, format!("move occurs due to use{}", move_spans.describe()));
move_spans.var_span_label(
&mut err,
format!("move occurs due to use{}", move_spans.describe()),
"moved",
);
self.explain_why_borrow_contains_point(location, borrow, None)
.add_explanation_to_diagnostic(
@@ -468,6 +472,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let use_spans = self.move_spans(place.as_ref(), location);
let span = use_spans.var_or_use();
// If the attempted use is in a closure then we do not care about the path span of the place we are currently trying to use
// we call `var_span_label` on `borrow_spans` to annotate if the existing borrow was in a closure
let mut err = self.cannot_use_when_mutably_borrowed(
span,
&self.describe_any_place(place.as_ref()),
@@ -475,11 +481,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
&self.describe_any_place(borrow.borrowed_place.as_ref()),
);
borrow_spans.var_span_label(&mut err, {
let place = &borrow.borrowed_place;
let desc_place = self.describe_any_place(place.as_ref());
format!("borrow occurs due to use of {}{}", desc_place, borrow_spans.describe())
});
borrow_spans.var_span_label(
&mut err,
{
let place = &borrow.borrowed_place;
let desc_place = self.describe_any_place(place.as_ref());
format!("borrow occurs due to use of {}{}", desc_place, borrow_spans.describe())
},
"mutable",
);
self.explain_why_borrow_contains_point(location, borrow, None)
.add_explanation_to_diagnostic(
@@ -591,6 +601,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
desc_place,
borrow_spans.describe(),
),
"immutable",
);
return err;
@@ -667,7 +678,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if issued_spans == borrow_spans {
borrow_spans.var_span_label(
&mut err,
format!("borrows occur due to use of {}{}", desc_place, borrow_spans.describe()),
format!("borrows occur due to use of {}{}", desc_place, borrow_spans.describe(),),
gen_borrow_kind.describe_mutability(),
);
} else {
let borrow_place = &issued_borrow.borrowed_place;
@@ -679,6 +691,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
borrow_place_desc,
issued_spans.describe(),
),
issued_borrow.kind.describe_mutability(),
);
borrow_spans.var_span_label(
@@ -688,6 +701,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
desc_place,
borrow_spans.describe(),
),
gen_borrow_kind.describe_mutability(),
);
}
@@ -847,7 +861,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
self.prefixes(borrow.borrowed_place.as_ref(), PrefixSet::All).last().unwrap();
let borrow_spans = self.retrieve_borrow_spans(borrow);
let borrow_span = borrow_spans.var_or_use();
let borrow_span = borrow_spans.var_or_use_path_span();
assert!(root_place.projection.is_empty());
let proper_span = self.body.local_decls[root_place.local].source_info.span;
@@ -987,7 +1001,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
location, name, borrow, drop_span, borrow_spans
);
let borrow_span = borrow_spans.var_or_use();
let borrow_span = borrow_spans.var_or_use_path_span();
if let BorrowExplanation::MustBeValidFor {
category,
span,
@@ -1575,6 +1589,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
loan_spans.var_span_label(
&mut err,
format!("borrow occurs due to use{}", loan_spans.describe()),
loan.kind.describe_mutability(),
);
err.buffer(&mut self.errors_buffer);
@@ -1585,8 +1600,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let mut err = self.cannot_assign_to_borrowed(span, loan_span, &descr_place);
loan_spans
.var_span_label(&mut err, format!("borrow occurs due to use{}", loan_spans.describe()));
loan_spans.var_span_label(
&mut err,
format!("borrow occurs due to use{}", loan_spans.describe()),
loan.kind.describe_mutability(),
);
self.explain_why_borrow_contains_point(location, loan, None).add_explanation_to_diagnostic(
self.infcx.tcx,

71
compiler/rustc_mir/src/borrow_check/diagnostics/explain_borrow.rs
View File

@@ -24,8 +24,8 @@ use super::{find_use, RegionName, UseSpans};
#[derive(Debug)]
pub(in crate::borrow_check) enum BorrowExplanation {
UsedLater(LaterUseKind, Span),
UsedLaterInLoop(LaterUseKind, Span),
UsedLater(LaterUseKind, Span, Option<Span>),
UsedLaterInLoop(LaterUseKind, Span, Option<Span>),
UsedLaterWhenDropped {
drop_loc: Location,
dropped_local: Local,
@@ -67,7 +67,7 @@ impl BorrowExplanation {
borrow_span: Option<Span>,
) {
match *self {
BorrowExplanation::UsedLater(later_use_kind, var_or_use_span) => {
BorrowExplanation::UsedLater(later_use_kind, var_or_use_span, path_span) => {
let message = match later_use_kind {
LaterUseKind::TraitCapture => "captured here by trait object",
LaterUseKind::ClosureCapture => "captured here by closure",
@@ -75,14 +75,31 @@ impl BorrowExplanation {
LaterUseKind::FakeLetRead => "stored here",
LaterUseKind::Other => "used here",
};
if !borrow_span.map_or(false, |sp| sp.overlaps(var_or_use_span)) {
err.span_label(
var_or_use_span,
format!("{}borrow later {}", borrow_desc, message),
);
// We can use `var_or_use_span` if either `path_span` is not present, or both spans are the same
if path_span.map_or(true, |path_span| path_span == var_or_use_span) {
if !borrow_span.map_or(false, |sp| sp.overlaps(var_or_use_span)) {
err.span_label(
var_or_use_span,
format!("{}borrow later {}", borrow_desc, message),
);
}
} else {
// path_span must be `Some` as otherwise the if condition is true
let path_span = path_span.unwrap();
// path_span is only present in the case of closure capture
assert!(matches!(later_use_kind, LaterUseKind::ClosureCapture));
if !borrow_span.map_or(false, |sp| sp.overlaps(var_or_use_span)) {
let path_label = "used here by closure";
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);
}
}
}
BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span) => {
BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span, path_span) => {
let message = match later_use_kind {
LaterUseKind::TraitCapture => {
"borrow captured here by trait object, in later iteration of loop"
@@ -94,7 +111,24 @@ impl BorrowExplanation {
LaterUseKind::FakeLetRead => "borrow later stored here",
LaterUseKind::Other => "borrow used here, in later iteration of loop",
};
err.span_label(var_or_use_span, format!("{}{}", borrow_desc, message));
// We can use `var_or_use_span` if either `path_span` is not present, or both spans are the same
if path_span.map_or(true, |path_span| path_span == var_or_use_span) {
err.span_label(var_or_use_span, format!("{}{}", borrow_desc, message));
} else {
// path_span must be `Some` as otherwise the if condition is true
let path_span = path_span.unwrap();
// path_span is only present in the case of closure capture
assert!(matches!(later_use_kind, LaterUseKind::ClosureCapture));
if !borrow_span.map_or(false, |sp| sp.overlaps(var_or_use_span)) {
let path_label = "used here by closure";
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);
}
}
}
BorrowExplanation::UsedLaterWhenDropped {
drop_loc,
@@ -311,13 +345,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let borrow_location = location;
if self.is_use_in_later_iteration_of_loop(borrow_location, location) {
let later_use = self.later_use_kind(borrow, spans, location);
BorrowExplanation::UsedLaterInLoop(later_use.0, later_use.1)
BorrowExplanation::UsedLaterInLoop(later_use.0, later_use.1, later_use.2)
} else {
// Check if the location represents a `FakeRead`, and adapt the error
// message to the `FakeReadCause` it is from: in particular,
// the ones inserted in optimized `let var = <expr>` patterns.
let later_use = self.later_use_kind(borrow, spans, location);
BorrowExplanation::UsedLater(later_use.0, later_use.1)
BorrowExplanation::UsedLater(later_use.0, later_use.1, later_use.2)
}
}
@@ -498,16 +532,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
/// Determine how the borrow was later used.
/// First span returned points to the location of the conflicting use
/// Second span if `Some` is returned in the case of closures and points
/// to the use of the path
fn later_use_kind(
&self,
borrow: &BorrowData<'tcx>,
use_spans: UseSpans<'tcx>,
location: Location,
) -> (LaterUseKind, Span) {
) -> (LaterUseKind, Span, Option<Span>) {
match use_spans {
UseSpans::ClosureUse { var_span, .. } => {
UseSpans::ClosureUse { capture_kind_span, path_span, .. } => {
// Used in a closure.
(LaterUseKind::ClosureCapture, var_span)
(LaterUseKind::ClosureCapture, capture_kind_span, Some(path_span))
}
UseSpans::PatUse(span)
| UseSpans::OtherUse(span)
@@ -542,7 +579,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}
};
return (LaterUseKind::Call, function_span);
return (LaterUseKind::Call, function_span, None);
} else {
LaterUseKind::Other
}
@@ -550,7 +587,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
LaterUseKind::Other
};
(kind, span)
(kind, span, None)
}
}
}

115
compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs
View File

@@ -18,7 +18,6 @@ use rustc_span::{
Span,
};
use rustc_target::abi::VariantIdx;
use std::iter;
use super::borrow_set::BorrowData;
use super::MirBorrowckCtxt;
@@ -216,11 +215,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
PlaceRef { local, projection: [proj_base @ .., elem] } => {
match elem {
ProjectionElem::Deref => {
// FIXME(project-rfc_2229#36): print capture precisely here.
let upvar_field_projection = self.is_upvar_field_projection(place);
if let Some(field) = upvar_field_projection {
let var_index = field.index();
let name = self.upvars[var_index].name.to_string();
let name = self.upvars[var_index].place.to_string(self.infcx.tcx);
if self.upvars[var_index].by_ref {
buf.push_str(&name);
} else {
@@ -265,7 +263,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let upvar_field_projection = self.is_upvar_field_projection(place);
if let Some(field) = upvar_field_projection {
let var_index = field.index();
let name = self.upvars[var_index].name.to_string();
let name = self.upvars[var_index].place.to_string(self.infcx.tcx);
buf.push_str(&name);
} else {
let field_name = self
@@ -550,8 +548,12 @@ pub(super) enum UseSpans<'tcx> {
/// The span of the args of the closure, including the `move` keyword if
/// it's present.
args_span: Span,
/// The span of the first use of the captured variable inside the closure.
var_span: Span,
/// The span of the use resulting in capture kind
/// Check `ty::CaptureInfo` for more details
capture_kind_span: Span,
/// The span of the use resulting in the captured path
/// Check `ty::CaptureInfo` for more details
path_span: Span,
},
/// The access is caused by using a variable as the receiver of a method
/// that takes 'self'
@@ -606,9 +608,21 @@ impl UseSpans<'_> {
}
}
pub(super) fn var_or_use_path_span(self) -> Span {
match self {
UseSpans::ClosureUse { path_span: span, .. }
| UseSpans::PatUse(span)
| UseSpans::OtherUse(span) => span,
UseSpans::FnSelfUse {
fn_call_span, kind: FnSelfUseKind::DerefCoercion { .. }, ..
} => fn_call_span,
UseSpans::FnSelfUse { var_span, .. } => var_span,
}
}
pub(super) fn var_or_use(self) -> Span {
match self {
UseSpans::ClosureUse { var_span: span, .. }
UseSpans::ClosureUse { capture_kind_span: span, .. }
| UseSpans::PatUse(span)
| UseSpans::OtherUse(span) => span,
UseSpans::FnSelfUse {
@@ -637,13 +651,34 @@ impl UseSpans<'_> {
}
// Add a span label to the use of the captured variable, if it exists.
pub(super) fn var_span_label(
// only adds label to the `path_span`
pub(super) fn var_span_label_path_only(
self,
err: &mut DiagnosticBuilder<'_>,
message: impl Into<String>,
) {
if let UseSpans::ClosureUse { var_span, .. } = self {
err.span_label(var_span, message);
if let UseSpans::ClosureUse { path_span, .. } = self {
err.span_label(path_span, message);
}
}
// Add a span label to the use of the captured variable, if it exists.
pub(super) fn var_span_label(
self,
err: &mut DiagnosticBuilder<'_>,
message: impl Into<String>,
kind_desc: impl Into<String>,
) {
if let UseSpans::ClosureUse { capture_kind_span, path_span, .. } = self {
if capture_kind_span == path_span {
err.span_label(capture_kind_span, message);
} else {
let capture_kind_label =
format!("capture is {} because of use here", kind_desc.into());
let path_label = message;
err.span_label(capture_kind_span, capture_kind_label);
err.span_label(path_span, path_label);
}
}
}
@@ -791,10 +826,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
box AggregateKind::Closure(def_id, _)
| box AggregateKind::Generator(def_id, _, _) => {
debug!("move_spans: def_id={:?} places={:?}", def_id, places);
if let Some((args_span, generator_kind, var_span)) =
if let Some((args_span, generator_kind, capture_kind_span, path_span)) =
self.closure_span(*def_id, moved_place, places)
{
return ClosureUse { generator_kind, args_span, var_span };
return ClosureUse {
generator_kind,
args_span,
capture_kind_span,
path_span,
};
}
}
_ => {}
@@ -809,10 +849,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
| FakeReadCause::ForLet(Some(closure_def_id)) => {
debug!("move_spans: def_id={:?} place={:?}", closure_def_id, place);
let places = &[Operand::Move(*place)];
if let Some((args_span, generator_kind, var_span)) =
if let Some((args_span, generator_kind, capture_kind_span, path_span)) =
self.closure_span(closure_def_id, moved_place, places)
{
return ClosureUse { generator_kind, args_span, var_span };
return ClosureUse {
generator_kind,
args_span,
capture_kind_span,
path_span,
};
}
}
_ => {}
@@ -972,10 +1017,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
"borrow_spans: def_id={:?} is_generator={:?} places={:?}",
def_id, is_generator, places
);
if let Some((args_span, generator_kind, var_span)) =
if let Some((args_span, generator_kind, capture_kind_span, path_span)) =
self.closure_span(*def_id, Place::from(target).as_ref(), places)
{
return ClosureUse { generator_kind, args_span, var_span };
return ClosureUse { generator_kind, args_span, capture_kind_span, path_span };
} else {
return OtherUse(use_span);
}
@@ -989,13 +1034,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
OtherUse(use_span)
}
/// Finds the span of a captured variable within a closure or generator.
/// Finds the spans of a captured place within a closure or generator.
/// The first span is the location of the use resulting in the capture kind of the capture
/// The second span is the location the use resulting in the captured path of the capture
fn closure_span(
&self,
def_id: DefId,
target_place: PlaceRef<'tcx>,
places: &[Operand<'tcx>],
) -> Option<(Span, Option<GeneratorKind>, Span)> {
) -> Option<(Span, Option<GeneratorKind>, Span, Span)> {
debug!(
"closure_span: def_id={:?} target_place={:?} places={:?}",
def_id, target_place, places
@@ -1005,13 +1052,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let expr = &self.infcx.tcx.hir().expect_expr(hir_id).kind;
debug!("closure_span: hir_id={:?} expr={:?}", hir_id, expr);
if let hir::ExprKind::Closure(.., body_id, args_span, _) = expr {
for (captured_place, place) in iter::zip(
self.infcx.tcx.typeck(def_id.expect_local()).closure_min_captures_flattened(def_id),
places,
) {
let upvar_hir_id = captured_place.get_root_variable();
//FIXME(project-rfc-2229#8): Use better span from captured_place
let span = self.infcx.tcx.upvars_mentioned(local_did)?[&upvar_hir_id].span;
for (captured_place, place) in self
.infcx
.tcx
.typeck(def_id.expect_local())
.closure_min_captures_flattened(def_id)
.zip(places)
{
match place {
Operand::Copy(place) | Operand::Move(place)
if target_place == place.as_ref() =>
@@ -1020,18 +1067,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let body = self.infcx.tcx.hir().body(*body_id);
let generator_kind = body.generator_kind();
// If we have a more specific span available, point to that.
// We do this even though this span might be part of a borrow error
// message rather than a move error message. Our goal is to point
// to a span that shows why the upvar is used in the closure,
// so a move-related span is as good as any (and potentially better,
// if the overall error is due to a move of the upvar).
let usage_span = match captured_place.info.capture_kind {
ty::UpvarCapture::ByValue(Some(span)) => span,
_ => span,
};
return Some((*args_span, generator_kind, usage_span));
return Some((
*args_span,
generator_kind,
captured_place.get_capture_kind_span(self.infcx.tcx),
captured_place.get_path_span(self.infcx.tcx),
));
}
_ => {}
}

11
compiler/rustc_mir/src/borrow_check/diagnostics/move_errors.rs
View File

@@ -345,10 +345,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
};
let upvar = &self.upvars[upvar_field.unwrap().index()];
// FIXME(project-rfc-2229#8): Improve borrow-check diagnostics in case of precise
// capture.
let upvar_hir_id = upvar.place.get_root_variable();
let upvar_name = upvar.name;
let upvar_name = upvar.place.to_string(self.infcx.tcx);
let upvar_span = self.infcx.tcx.hir().span(upvar_hir_id);
let place_name = self.describe_any_place(move_place.as_ref());
@@ -478,8 +476,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
self.note_type_does_not_implement_copy(err, &place_desc, place_ty, Some(span), "");
use_spans.args_span_label(err, format!("move out of {} occurs here", place_desc));
use_spans
.var_span_label(err, format!("move occurs due to use{}", use_spans.describe()));
use_spans.var_span_label(
err,
format!("move occurs due to use{}", use_spans.describe()),
"moved",
);
}
}
}

3
compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs
View File

@@ -85,7 +85,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
if self.is_upvar_field_projection(access_place.as_ref()).is_some() {
reason = ", as it is not declared as mutable".to_string();
} else {
let name = self.upvars[upvar_index.index()].name;
let name = self.upvars[upvar_index.index()].place.to_string(self.infcx.tcx);
reason = format!(", as `{}` is not declared as mutable", name);
}
}
@@ -195,6 +195,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
"mutable borrow occurs due to use of {} in closure",
self.describe_any_place(access_place.as_ref()),
),
"mutable",
);
borrow_span
}

1
compiler/rustc_mir/src/borrow_check/diagnostics/region_errors.rs
View File

@@ -385,6 +385,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
diag.span_label(*span, message);
// FIXME: This should store a captured_place not a hir id
if let ReturnConstraint::ClosureUpvar(upvar) = kind {
let def_id = match self.regioncx.universal_regions().defining_ty {
DefiningTy::Closure(def_id, _) => def_id,

6
compiler/rustc_mir/src/borrow_check/mod.rs
View File

@@ -74,9 +74,6 @@ crate use region_infer::RegionInferenceContext;
// FIXME(eddyb) perhaps move this somewhere more centrally.
#[derive(Debug)]
crate struct Upvar<'tcx> {
// FIXME(project-rfc_2229#36): print capture precisely here.
name: Symbol,
place: CapturedPlace<'tcx>,
/// If true, the capture is behind a reference.
@@ -159,13 +156,12 @@ fn do_mir_borrowck<'a, 'tcx>(
let upvars: Vec<_> = tables
.closure_min_captures_flattened(def.did.to_def_id())
.map(|captured_place| {
let var_hir_id = captured_place.get_root_variable();
let capture = captured_place.info.capture_kind;
let by_ref = match capture {
ty::UpvarCapture::ByValue(_) => false,
ty::UpvarCapture::ByRef(..) => true,
};
Upvar { name: tcx.hir().name(var_hir_id), place: captured_place.clone(), by_ref }
Upvar { place: captured_place.clone(), by_ref }
})
.collect();