Skip to content

[NLL] Rework checking for borrows conflicting with drops #54509

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.

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

Merged
merged 3 commits into from
Sep 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
108 changes: 83 additions & 25 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use borrow_check::{WriteKind, StorageDeadOrDrop};
use borrow_check::WriteKind;
use borrow_check::prefixes::IsPrefixOf;
use borrow_check::nll::explain_borrow::BorrowExplanation;
use rustc::middle::region::ScopeTree;
use rustc::mir::{
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, FakeReadCause, Field, Local,
LocalDecl, LocalKind, Location, Operand, Place, ProjectionElem, Rvalue, Statement,
StatementKind, TerminatorKind, VarBindingForm,
LocalDecl, LocalKind, Location, Operand, Place, PlaceProjection, ProjectionElem, Rvalue,
Statement, StatementKind, TerminatorKind, VarBindingForm,
};
use rustc::hir;
use rustc::hir::def_id::DefId;
Expand Down Expand Up @@ -452,13 +452,21 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
self.access_place_error_reported
.insert((root_place.clone(), borrow_span));

if let Some(WriteKind::StorageDeadOrDrop(StorageDeadOrDrop::Destructor)) = kind {
if let StorageDeadOrDrop::Destructor(dropped_ty)
= self.classify_drop_access_kind(&borrow.borrowed_place)
{
// If a borrow of path `B` conflicts with drop of `D` (and
// we're not in the uninteresting case where `B` is a
// prefix of `D`), then report this as a more interesting
// destructor conflict.
if !borrow.borrowed_place.is_prefix_of(place_span.0) {
self.report_borrow_conflicts_with_destructor(context, borrow, place_span, kind);
self.report_borrow_conflicts_with_destructor(
context,
borrow,
place_span,
kind,
dropped_ty,
);
return;
}
}
Expand Down Expand Up @@ -566,6 +574,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
borrow: &BorrowData<'tcx>,
(place, drop_span): (&Place<'tcx>, Span),
kind: Option<WriteKind>,
dropped_ty: ty::Ty<'tcx>,
) {
debug!(
"report_borrow_conflicts_with_destructor(\
Expand All @@ -579,28 +588,19 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

let mut err = self.infcx.tcx.cannot_borrow_across_destructor(borrow_span, Origin::Mir);

let (what_was_dropped, dropped_ty) = {
let desc = match self.describe_place(place) {
Some(name) => format!("`{}`", name.as_str()),
None => format!("temporary value"),
};
let ty = place.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx);
(desc, ty)
let what_was_dropped = match self.describe_place(place) {
Some(name) => format!("`{}`", name.as_str()),
None => format!("temporary value"),
};

let label = match dropped_ty.sty {
ty::Adt(adt, _) if adt.has_dtor(self.infcx.tcx) && !adt.is_box() => {
match self.describe_place(&borrow.borrowed_place) {
Some(borrowed) =>
format!("here, drop of {D} needs exclusive access to `{B}`, \
because the type `{T}` implements the `Drop` trait",
D=what_was_dropped, T=dropped_ty, B=borrowed),
None =>
format!("here is drop of {D}; whose type `{T}` implements the `Drop` trait",
D=what_was_dropped, T=dropped_ty),
}
}
_ => format!("drop of {D} occurs here", D=what_was_dropped),
let label = match self.describe_place(&borrow.borrowed_place) {
Some(borrowed) =>
format!("here, drop of {D} needs exclusive access to `{B}`, \
because the type `{T}` implements the `Drop` trait",
D=what_was_dropped, T=dropped_ty, B=borrowed),
None =>
format!("here is drop of {D}; whose type `{T}` implements the `Drop` trait",
D=what_was_dropped, T=dropped_ty),
};
err.span_label(drop_span, label);

Expand Down Expand Up @@ -880,6 +880,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

pub(super) struct IncludingDowncast(bool);

/// Which case a StorageDeadOrDrop is for.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
enum StorageDeadOrDrop<'tcx> {
LocalStorageDead,
BoxedStorageDead,
Destructor(ty::Ty<'tcx>),
}

impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// End-user visible description of `place` if one can be found. If the
// place is a temporary for instance, None will be returned.
Expand Down Expand Up @@ -1167,6 +1175,56 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}

fn classify_drop_access_kind(&self, place: &Place<'tcx>) -> StorageDeadOrDrop<'tcx> {
let tcx = self.infcx.tcx;
match place {
Place::Local(_)
| Place::Static(_)
| Place::Promoted(_) => StorageDeadOrDrop::LocalStorageDead,
Place::Projection(box PlaceProjection { base, elem }) => {
let base_access = self.classify_drop_access_kind(base);
match elem {
ProjectionElem::Deref => {
match base_access {
StorageDeadOrDrop::LocalStorageDead
| StorageDeadOrDrop::BoxedStorageDead => {
assert!(base.ty(self.mir, tcx).to_ty(tcx).is_box(),
"Drop of value behind a reference or raw pointer");
StorageDeadOrDrop::BoxedStorageDead
}
StorageDeadOrDrop::Destructor(_) => {
base_access
}
}
}
ProjectionElem::Field(..)
| ProjectionElem::Downcast(..) => {
let base_ty = base.ty(self.mir, tcx).to_ty(tcx);
match base_ty.sty {
ty::Adt(def, _) if def.has_dtor(tcx) => {
// Report the outermost adt with a destructor
match base_access {
StorageDeadOrDrop::Destructor(_) => {
base_access
}
StorageDeadOrDrop::LocalStorageDead
| StorageDeadOrDrop::BoxedStorageDead => {
StorageDeadOrDrop::Destructor(base_ty)
}
}
}
_ => base_access,
}
}

ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. }
| ProjectionElem::Index(_) => base_access,
}
}
}
}

/// Annotate argument and return type of function and closure with (synthesized) lifetime for
/// borrow of local value that does not live long enough.
fn annotate_argument_and_return_for_borrow(
Expand Down
Loading