Skip to content

Commit

Permalink
Rollup merge of rust-lang#84358 - sexxi-goose:print_captures_borrowck…
Browse files Browse the repository at this point in the history 
…_rebased, r=nikomatsakis

Update closure capture error logging for disjoint captures for disjoint captures

Improved error logging when `#![feature(capture_disjoint_fields)]` is used.

Closes rust-lang/project-rfc-2229#8
Closes rust-lang/project-rfc-2229#36
Closes rust-lang/project-rfc-2229#39
Closes rust-lang#76005
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored May 2, 2021
2 parents 3a1cd0e + 404cc33 commit 35c6902
Show file tree
Hide file tree
Showing 57 changed files with 534 additions and 152 deletions.
11 changes: 11 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,15 @@ impl BorrowKind {
BorrowKind::Mut { allow_two_phase_borrow } => allow_two_phase_borrow,
}
}

pub fn describe_mutability(&self) -> String {
match *self {
BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => {
"immutable".to_string()
}
BorrowKind::Mut { .. } => "mutable".to_string(),
}
}
}

///////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -2369,6 +2378,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
};
let mut struct_fmt = fmt.debug_struct(&name);

// FIXME(project-rfc-2229#48): This should be a list of capture names/places
if let Some(upvars) = tcx.upvars_mentioned(def_id) {
for (&var_id, place) in iter::zip(upvars.keys(), places) {
let var_name = tcx.hir().name(var_id);
Expand All @@ -2388,6 +2398,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
let name = format!("[generator@{:?}]", tcx.hir().span(hir_id));
let mut struct_fmt = fmt.debug_struct(&name);

// FIXME(project-rfc-2229#48): This should be a list of capture names/places
if let Some(upvars) = tcx.upvars_mentioned(def_id) {
for (&var_id, place) in iter::zip(upvars.keys(), places) {
let var_name = tcx.hir().name(var_id);
Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_middle/src/ty/closure.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ pub struct CapturedPlace<'tcx> {
}

impl CapturedPlace<'tcx> {
pub fn to_string(&self, tcx: TyCtxt<'tcx>) -> String {
place_to_string_for_capture(tcx, &self.place)
}

/// Returns the hir-id of the root variable for the captured place.
/// e.g., if `a.b.c` was captured, would return the hir-id for `a`.
pub fn get_root_variable(&self) -> hir::HirId {
Expand All @@ -168,6 +172,22 @@ impl CapturedPlace<'tcx> {
}
}

/// Return span pointing to use that resulted in selecting the captured path
pub fn get_path_span(&self, tcx: TyCtxt<'tcx>) -> Span {
if let Some(path_expr_id) = self.info.path_expr_id {
tcx.hir().span(path_expr_id)
} else if let Some(capture_kind_expr_id) = self.info.capture_kind_expr_id {
tcx.hir().span(capture_kind_expr_id)
} else {
// Fallback on upvars mentioned if neither path or capture expr id is captured

// Safe to unwrap since we know this place is captured by the closure, therefore the closure must have upvars.
tcx.upvars_mentioned(self.get_closure_local_def_id()).unwrap()
[&self.get_root_variable()]
.span
}
}

/// Return span pointing to use that resulted in selecting the current capture kind
pub fn get_capture_kind_span(&self, tcx: TyCtxt<'tcx>) -> Span {
if let Some(capture_kind_expr_id) = self.info.capture_kind_expr_id {
Expand Down
48 changes: 33 additions & 15 deletions compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
);
Expand Down Expand Up @@ -255,6 +255,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
partially_str,
move_spans.describe()
),
"moved",
);
}
}
Expand Down Expand Up @@ -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()),
);
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -468,18 +472,24 @@ 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()),
borrow_span,
&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(
Expand Down Expand Up @@ -591,6 +601,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
desc_place,
borrow_spans.describe(),
),
"immutable",
);

return err;
Expand Down Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -688,6 +701,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
desc_place,
borrow_spans.describe(),
),
gen_borrow_kind.describe_mutability(),
);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand Down
71 changes: 54 additions & 17 deletions compiler/rustc_mir/src/borrow_check/diagnostics/explain_borrow.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -67,22 +67,39 @@ 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",
LaterUseKind::Call => "used by call",
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(|path_span| path_span == var_or_use_span).unwrap_or(true) {
if borrow_span.map(|sp| !sp.overlaps(var_or_use_span)).unwrap_or(true) {
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"
Expand All @@ -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(|path_span| path_span == var_or_use_span).unwrap_or(true) {
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(|sp| !sp.overlaps(var_or_use_span)).unwrap_or(true) {
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,
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -542,15 +579,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}
};
return (LaterUseKind::Call, function_span);
return (LaterUseKind::Call, function_span, None);
} else {
LaterUseKind::Other
}
} else {
LaterUseKind::Other
};

(kind, span)
(kind, span, None)
}
}
}
Expand Down
Loading

0 comments on commit 35c6902

Please sign in to comment.