diff options
| -rw-r--r-- | src/librustc_mir/borrow_check/error_reporting.rs | 2 | ||||
| -rw-r--r-- | src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs | 147 | ||||
| -rw-r--r-- | src/librustc_mir/build/expr/stmt.rs | 6 |
3 files changed, 108 insertions, 47 deletions
diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index b9d849966a6..53a190efb58 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -770,7 +770,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { match explanation { BorrowExplanation::UsedLater(..) | BorrowExplanation::UsedLaterInLoop(..) - | BorrowExplanation::UsedLaterWhenDropped(..) => { + | BorrowExplanation::UsedLaterWhenDropped { .. } => { // Only give this note and suggestion if it could be relevant. err.note("consider using a `let` binding to create a longer lived value"); } diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs index d1d6ba12372..e55469436ab 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -12,18 +12,22 @@ use borrow_check::borrow_set::BorrowData; use borrow_check::error_reporting::UseSpans; use borrow_check::nll::region_infer::Cause; use borrow_check::{Context, MirBorrowckCtxt, WriteKind}; -use rustc::ty::{Region, TyCtxt}; -use rustc::mir::{FakeReadCause, Location, Mir, Operand, Place, StatementKind, TerminatorKind}; +use rustc::ty::{self, Region, TyCtxt}; +use rustc::mir::{FakeReadCause, Local, Location, Mir, Operand}; +use rustc::mir::{Place, StatementKind, TerminatorKind}; use rustc_errors::DiagnosticBuilder; use syntax_pos::Span; -use syntax_pos::symbol::Symbol; mod find_use; pub(in borrow_check) enum BorrowExplanation<'tcx> { UsedLater(LaterUseKind, Span), UsedLaterInLoop(LaterUseKind, Span), - UsedLaterWhenDropped(Span, Symbol, bool), + UsedLaterWhenDropped { + drop_loc: Location, + dropped_local: Local, + should_note_order: bool, + }, MustBeValidFor(Region<'tcx>), Unexplained, } @@ -40,7 +44,7 @@ impl<'tcx> BorrowExplanation<'tcx> { pub(in borrow_check) fn add_explanation_to_diagnostic<'cx, 'gcx>( &self, tcx: TyCtxt<'cx, 'gcx, 'tcx>, - _mir: &Mir<'tcx>, + mir: &Mir<'tcx>, err: &mut DiagnosticBuilder<'_>, borrow_desc: &str, ) { @@ -65,23 +69,76 @@ impl<'tcx> BorrowExplanation<'tcx> { }; err.span_label(var_or_use_span, format!("{}{}", borrow_desc, message)); }, - BorrowExplanation::UsedLaterWhenDropped(span, local_name, should_note_order) => { - err.span_label( - span, - format!( - "{}borrow later used here, when `{}` is dropped", - borrow_desc, - local_name, - ), - ); + BorrowExplanation::UsedLaterWhenDropped { drop_loc, dropped_local, + should_note_order } => + { + let local_decl = &mir.local_decls[dropped_local]; + let (dtor_desc, type_desc) = match local_decl.ty.sty { + // 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.item_path_str(adt.did))), + + // Otherwise, just report the whole type (and use + // the intentionally fuzzy phrase "destructor") + ty::Closure(..) => + ("destructor", format!("closure")), + ty::Generator(..) => + ("destructor", format!("generator")), + + _ => ("destructor", format!("type `{}`", local_decl.ty)), + }; + + match local_decl.name { + Some(local_name) => { + 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(mir.source_info(drop_loc).span, message); + + if should_note_order { + err.note( + "values in a scope are dropped \ + in the opposite order they are defined", + ); + } + } + None => { + 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(mir.source_info(drop_loc).span, message); + + if let Some(info) = &local_decl.is_block_tail { + // FIXME: use span_suggestion instead, highlighting the + // whole block tail expression. + let msg = if info.tail_result_is_ignored { + "The temporary is part of an expression at the end of a block. \ + Consider adding semicolon after the expression so its temporaries \ + are dropped sooner, before the local variables declared by the \ + block are dropped." + } else { + "The temporary is part of an expression at the end of a block. \ + Consider forcing this temporary to be dropped sooner, before \ + the block's local variables are dropped. \ + 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." + }; - if should_note_order { - err.note( - "values in a scope are dropped \ - in the opposite order they are defined", - ); + err.note(msg); + } + } } - }, + } + BorrowExplanation::MustBeValidFor(region) => { tcx.note_and_explain_free_region( err, @@ -116,8 +173,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { kind_place: Option<(WriteKind, &Place<'tcx>)>, ) -> BorrowExplanation<'tcx> { debug!( - "explain_why_borrow_contains_point(context={:?}, borrow={:?})", - context, borrow, + "explain_why_borrow_contains_point(context={:?}, borrow={:?}, kind_place={:?})", + context, borrow, kind_place ); let regioncx = &self.nonlexical_regioncx; @@ -154,32 +211,30 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } - Some(Cause::DropVar(local, location)) => match &mir.local_decls[local].name { - Some(local_name) => { - let mut should_note_order = false; - if let Some((WriteKind::StorageDeadOrDrop, place)) = kind_place { - if let Place::Local(borrowed_local) = place { - let dropped_local_scope = mir.local_decls[local].visibility_scope; - let borrowed_local_scope = - mir.local_decls[*borrowed_local].visibility_scope; + Some(Cause::DropVar(local, location)) => { + let mut should_note_order = false; + if mir.local_decls[local].name.is_some() { + if let Some((WriteKind::StorageDeadOrDrop, place)) = kind_place { + if let Place::Local(borrowed_local) = place { + let dropped_local_scope = mir.local_decls[local].visibility_scope; + let borrowed_local_scope = + mir.local_decls[*borrowed_local].visibility_scope; - if mir.is_sub_scope(borrowed_local_scope, dropped_local_scope) - && local != *borrowed_local - { - should_note_order = true; - } - } - } - - BorrowExplanation::UsedLaterWhenDropped( - mir.source_info(location).span, - *local_name, - should_note_order - ) - }, + if mir.is_sub_scope(borrowed_local_scope, dropped_local_scope) + && local != *borrowed_local + { + should_note_order = true; + } + } + } + } - None => BorrowExplanation::Unexplained, - }, + BorrowExplanation::UsedLaterWhenDropped { + drop_loc: location, + dropped_local: local, + should_note_order, + } + } None => if let Some(region) = regioncx.to_error_region(region_sub) { BorrowExplanation::MustBeValidFor(region) diff --git a/src/librustc_mir/build/expr/stmt.rs b/src/librustc_mir/build/expr/stmt.rs index 8cb2b6384b8..d2b39f088b6 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -20,6 +20,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let source_info = this.source_info(expr.span); // Handle a number of expressions that don't need a destination at all. This // avoids needing a mountain of temporary `()` variables. + let expr2 = expr.clone(); match expr.kind { ExprKind::Scope { region_scope, @@ -40,6 +41,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // is better for borrowck interaction with overloaded // operators like x[j] = x[i]. + debug!("stmt_expr Assign block_context.push(SubExpr) : {:?}", expr2); this.block_context.push(BlockFrame::SubExpr); // Generate better code for things that don't need to be @@ -69,6 +71,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let lhs = this.hir.mirror(lhs); let lhs_ty = lhs.ty; + debug!("stmt_expr AssignOp block_context.push(SubExpr) : {:?}", expr2); this.block_context.push(BlockFrame::SubExpr); // As above, RTL. @@ -120,6 +123,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { (break_block, region_scope, break_destination.clone()) }; if let Some(value) = value { + debug!("stmt_expr Break val block_context.push(SubExpr) : {:?}", expr2); this.block_context.push(BlockFrame::SubExpr); unpack!(block = this.into(&destination, block, value)); this.block_context.pop(); @@ -132,6 +136,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ExprKind::Return { value } => { block = match value { Some(value) => { + debug!("stmt_expr Return val block_context.push(SubExpr) : {:?}", expr2); this.block_context.push(BlockFrame::SubExpr); let result = unpack!(this.into(&Place::Local(RETURN_PLACE), block, value)); this.block_context.pop(); @@ -153,6 +158,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { outputs, inputs, } => { + debug!("stmt_expr InlineAsm block_context.push(SubExpr) : {:?}", expr2); this.block_context.push(BlockFrame::SubExpr); let outputs = outputs .into_iter() |