From 91b71f5e9416b570b3e4c1997056c3879e5029af Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 5 Oct 2018 17:05:33 +0200 Subject: [PATCH 1/2] Identify borrows captured by trait objects. This commit enhances `LaterUseKind` detection to identify when a borrow is captured by a trait object which helps explain why there is a borrow error. --- .../borrow_check/nll/explain_borrow/mod.rs | 173 ++++++++++++++---- ...ons-close-over-type-parameter-2.nll.stderr | 2 +- 2 files changed, 137 insertions(+), 38 deletions(-) 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 e55469436abf0..74bbdebe7a3ad 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -13,8 +13,10 @@ use borrow_check::error_reporting::UseSpans; use borrow_check::nll::region_infer::Cause; use borrow_check::{Context, MirBorrowckCtxt, WriteKind}; use rustc::ty::{self, Region, TyCtxt}; -use rustc::mir::{FakeReadCause, Local, Location, Mir, Operand}; -use rustc::mir::{Place, StatementKind, TerminatorKind}; +use rustc::mir::{ + FakeReadCause, Local, Location, Mir, Operand, Place, Rvalue, Statement, StatementKind, + TerminatorKind +}; use rustc_errors::DiagnosticBuilder; use syntax_pos::Span; @@ -34,6 +36,7 @@ pub(in borrow_check) enum BorrowExplanation<'tcx> { #[derive(Clone, Copy)] pub(in borrow_check) enum LaterUseKind { + TraitCapture, ClosureCapture, Call, FakeLetRead, @@ -51,6 +54,7 @@ impl<'tcx> BorrowExplanation<'tcx> { match *self { BorrowExplanation::UsedLater(later_use_kind, var_or_use_span) => { let message = match later_use_kind { + LaterUseKind::TraitCapture => "borrow later captured here by trait object", LaterUseKind::ClosureCapture => "borrow later captured here by closure", LaterUseKind::Call => "borrow later used by call", LaterUseKind::FakeLetRead => "borrow later stored here", @@ -60,9 +64,10 @@ impl<'tcx> BorrowExplanation<'tcx> { }, BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span) => { let message = match later_use_kind { - LaterUseKind::ClosureCapture => { - "borrow captured here by closure, in later iteration of loop" - }, + LaterUseKind::TraitCapture => + "borrow later captured here by trait object, in later iteration of loop", + LaterUseKind::ClosureCapture => + "borrow captured here by closure, in later iteration of loop", LaterUseKind::Call => "borrow used by call, in later iteration of loop", LaterUseKind::FakeLetRead => "borrow later stored here", LaterUseKind::Other => "borrow used here, in later iteration of loop", @@ -200,13 +205,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { .or_else(|| self.borrow_spans(span, location)); if self.is_borrow_location_in_loop(context.loc) { - let later_use = self.later_use_kind(spans, location); + let later_use = self.later_use_kind(borrow, spans, location); BorrowExplanation::UsedLaterInLoop(later_use.0, later_use.1) } 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 = ` patterns. - let later_use = self.later_use_kind(spans, location); + let later_use = self.later_use_kind(borrow, spans, location); BorrowExplanation::UsedLater(later_use.0, later_use.1) } } @@ -316,42 +321,136 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { false } - fn later_use_kind(&self, use_spans: UseSpans, location: Location) -> (LaterUseKind, Span) { - use self::LaterUseKind::*; - - let block = &self.mir.basic_blocks()[location.block]; + /// Determine how the borrow was later used. + fn later_use_kind( + &self, + borrow: &BorrowData<'tcx>, + use_spans: UseSpans, + location: Location + ) -> (LaterUseKind, Span) { match use_spans { - UseSpans::ClosureUse { var_span, .. } => (LaterUseKind::ClosureCapture, var_span), + UseSpans::ClosureUse { var_span, .. } => { + // Used in a closure. + (LaterUseKind::ClosureCapture, var_span) + }, UseSpans::OtherUse(span) => { - (if let Some(stmt) = block.statements.get(location.statement_index) { - match stmt.kind { - StatementKind::FakeRead(FakeReadCause::ForLet, _) => FakeLetRead, - _ => Other, + let block = &self.mir.basic_blocks()[location.block]; + + let kind = if let Some(&Statement { + kind: StatementKind::FakeRead(FakeReadCause::ForLet, _), + .. + }) = block.statements.get(location.statement_index) { + LaterUseKind::FakeLetRead + } else if self.was_captured_by_trait_object(borrow) { + LaterUseKind::TraitCapture + } else if location.statement_index == block.statements.len() { + if let TerminatorKind::Call { + ref func, from_hir_call: true, .. + } = block.terminator().kind { + // Just point to the function, to reduce the chance of overlapping spans. + let function_span = match func { + Operand::Constant(c) => c.span, + Operand::Copy(Place::Local(l)) | Operand::Move(Place::Local(l)) => { + let local_decl = &self.mir.local_decls[*l]; + if local_decl.name.is_none() { + local_decl.source_info.span + } else { + span + } + }, + _ => span, + }; + return (LaterUseKind::Call, function_span); + } else { + LaterUseKind::Other } } else { - assert_eq!(location.statement_index, block.statements.len()); - match block.terminator().kind { - TerminatorKind::Call { ref func, from_hir_call: true, .. } => { - // Just point to the function, to reduce the chance - // of overlapping spans. - let function_span = match func { - Operand::Constant(c) => c.span, - Operand::Copy(Place::Local(l)) | Operand::Move(Place::Local(l)) => { - let local_decl = &self.mir.local_decls[*l]; - if local_decl.name.is_none() { - local_decl.source_info.span - } else { - span - } - }, - _ => span, - }; - return (Call, function_span); - }, - _ => Other, + LaterUseKind::Other + }; + + (kind, span) + } + } + } + + /// Check if a borrowed value was captured by a trait object. + fn was_captured_by_trait_object(&self, borrow: &BorrowData<'tcx>) -> bool { + // In order to check if a value was captured by a trait object, we want to look through + // statements after the reserve location in the current block. We expect the reserve + // location to be a statement assigning to a local. We follow that local in the subsequent + // statements, checking for an assignment of our local (or something intermediate that + // it was assigned into) that results in a trait object. + let location = borrow.reserve_location; + let block = &self.mir[location.block]; + let stmt = block.statements.get(location.statement_index); + debug!( + "was_captured_by_trait_object: location={:?} block={:?} stmt={:?}", + location, block, stmt + ); + let mut target = if let Some(&Statement { + kind: StatementKind::Assign(Place::Local(local), _), + .. + }) = stmt { + local + } else { + return false; + }; + + debug!("was_captured_by_trait_object: target={:?}", target); + for stmt in &block.statements[location.statement_index + 1..] { + debug!("was_captured_by_trait_object: stmt={:?}", stmt); + // Simple case where our target is assigned into another local, and we start + // watching that local instead. + if let StatementKind::Assign( + Place::Local(into), + box Rvalue::Use(operand), + ) = &stmt.kind { + debug!("was_captured_by_trait_object: target={:?} operand={:?}", target, operand); + match operand { + Operand::Copy(Place::Local(from)) | + Operand::Move(Place::Local(from)) if *from == target => target = *into, + _ => {}, + } + } + } + + if let Some(terminator) = &block.terminator { + if let TerminatorKind::Call { + destination: Some((Place::Local(dest), _)), + args, + .. + } = &terminator.kind { + debug!( + "was_captured_by_trait_object: target={:?} dest={:?} args={:?}", + target, dest, args + ); + let mut found_target = false; + for arg in args { + if let Operand::Move(Place::Local(potential)) = arg { + if *potential == target { + found_target = true; + } + } + } + + if found_target { + let local_decl_ty = &self.mir.local_decls[*dest].ty; + debug!("was_captured_by_trait_object: local_decl_ty={:?}", local_decl_ty); + match local_decl_ty.sty { + // `&dyn Trait` + ty::TyKind::Ref(_, ty, _) if ty.is_trait() => return true, + // `Box` + _ if local_decl_ty.is_box() && local_decl_ty.boxed_ty().is_trait() => + return true, + // `dyn Trait` + _ if local_decl_ty.is_trait() => return true, + // Anything else. + _ => return false, } - }, span) + } } } + + false } } diff --git a/src/test/ui/span/regions-close-over-type-parameter-2.nll.stderr b/src/test/ui/span/regions-close-over-type-parameter-2.nll.stderr index f8e5e3914eb3c..11fa447b5489a 100644 --- a/src/test/ui/span/regions-close-over-type-parameter-2.nll.stderr +++ b/src/test/ui/span/regions-close-over-type-parameter-2.nll.stderr @@ -4,7 +4,7 @@ error[E0597]: `tmp0` does not live long enough LL | let tmp1 = &tmp0; | ^^^^^ borrowed value does not live long enough LL | repeater3(tmp1) - | --------------- borrow later used here + | --------------- borrow later captured here by trait object LL | }; | - `tmp0` dropped here while still borrowed From 72911fbbd051c1824f00735ac1b57017ca709a87 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 5 Oct 2018 23:31:33 +0200 Subject: [PATCH 2/2] Update logic to search for casts. This commit updates the captured trait object search logic to look for unsized casts to boxed types rather than for functions that returned trait objects. --- .../borrow_check/nll/explain_borrow/mod.rs | 170 +++++++++++------- src/test/ui/nll/issue-52663-trait-object.rs | 27 +++ .../ui/nll/issue-52663-trait-object.stderr | 13 ++ 3 files changed, 149 insertions(+), 61 deletions(-) create mode 100644 src/test/ui/nll/issue-52663-trait-object.rs create mode 100644 src/test/ui/nll/issue-52663-trait-object.stderr 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 74bbdebe7a3ad..307112f8ba16a 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -14,8 +14,8 @@ use borrow_check::nll::region_infer::Cause; use borrow_check::{Context, MirBorrowckCtxt, WriteKind}; use rustc::ty::{self, Region, TyCtxt}; use rustc::mir::{ - FakeReadCause, Local, Location, Mir, Operand, Place, Rvalue, Statement, StatementKind, - TerminatorKind + CastKind, FakeReadCause, Local, Location, Mir, Operand, Place, Projection, ProjectionElem, + Rvalue, Statement, StatementKind, TerminatorKind }; use rustc_errors::DiagnosticBuilder; use syntax_pos::Span; @@ -65,7 +65,7 @@ impl<'tcx> BorrowExplanation<'tcx> { BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span) => { let message = match later_use_kind { LaterUseKind::TraitCapture => - "borrow later captured here by trait object, in later iteration of loop", + "borrow captured here by trait object, in later iteration of loop", LaterUseKind::ClosureCapture => "borrow captured here by closure, in later iteration of loop", LaterUseKind::Call => "borrow used by call, in later iteration of loop", @@ -373,20 +373,20 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } - /// Check if a borrowed value was captured by a trait object. + /// Check if a borrowed value was captured by a trait object. We do this by + /// looking forward in the MIR from the reserve location and checking if we see + /// a unsized cast to a trait object on our data. fn was_captured_by_trait_object(&self, borrow: &BorrowData<'tcx>) -> bool { - // In order to check if a value was captured by a trait object, we want to look through - // statements after the reserve location in the current block. We expect the reserve - // location to be a statement assigning to a local. We follow that local in the subsequent - // statements, checking for an assignment of our local (or something intermediate that - // it was assigned into) that results in a trait object. + // Start at the reserve location, find the place that we want to see cast to a trait object. let location = borrow.reserve_location; let block = &self.mir[location.block]; let stmt = block.statements.get(location.statement_index); - debug!( - "was_captured_by_trait_object: location={:?} block={:?} stmt={:?}", - location, block, stmt - ); + debug!("was_captured_by_trait_object: location={:?} stmt={:?}", location, stmt); + + // We make a `queue` vector that has the locations we want to visit. As of writing, this + // will only ever have one item at any given time, but by using a vector, we can pop from + // it which simplifies the termination logic. + let mut queue = vec![location]; let mut target = if let Some(&Statement { kind: StatementKind::Assign(Place::Local(local), _), .. @@ -396,61 +396,109 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { return false; }; - debug!("was_captured_by_trait_object: target={:?}", target); - for stmt in &block.statements[location.statement_index + 1..] { - debug!("was_captured_by_trait_object: stmt={:?}", stmt); - // Simple case where our target is assigned into another local, and we start - // watching that local instead. - if let StatementKind::Assign( - Place::Local(into), - box Rvalue::Use(operand), - ) = &stmt.kind { - debug!("was_captured_by_trait_object: target={:?} operand={:?}", target, operand); - match operand { - Operand::Copy(Place::Local(from)) | - Operand::Move(Place::Local(from)) if *from == target => target = *into, - _ => {}, - } - } - } - - if let Some(terminator) = &block.terminator { - if let TerminatorKind::Call { - destination: Some((Place::Local(dest), _)), - args, - .. - } = &terminator.kind { - debug!( - "was_captured_by_trait_object: target={:?} dest={:?} args={:?}", - target, dest, args - ); - let mut found_target = false; - for arg in args { - if let Operand::Move(Place::Local(potential)) = arg { - if *potential == target { - found_target = true; - } + debug!("was_captured_by_trait: target={:?} queue={:?}", target, queue); + while let Some(current_location) = queue.pop() { + debug!("was_captured_by_trait: target={:?}", target); + let block = &self.mir[current_location.block]; + // We need to check the current location to find out if it is a terminator. + let is_terminator = current_location.statement_index == block.statements.len(); + if !is_terminator { + let stmt = &block.statements[current_location.statement_index]; + debug!("was_captured_by_trait_object: stmt={:?}", stmt); + + // The only kind of statement that we care about is assignments... + if let StatementKind::Assign( + place, + box rvalue, + ) = &stmt.kind { + let into = match place { + Place::Local(into) => into, + Place::Projection(box Projection { + base: Place::Local(into), + elem: ProjectionElem::Deref, + }) => into, + _ => { + // Continue at the next location. + queue.push(current_location.successor_within_block()); + continue; + }, + }; + + match rvalue { + // If we see a use, we should check whether it is our data, and if so + // update the place that we're looking for to that new place. + Rvalue::Use(operand) => match operand { + Operand::Copy(Place::Local(from)) | + Operand::Move(Place::Local(from)) if *from == target => { + target = *into; + }, + _ => {}, + }, + // If we see a unsized cast, then if it is our data we should check + // whether it is being cast to a trait object. + Rvalue::Cast(CastKind::Unsize, operand, ty) => match operand { + Operand::Copy(Place::Local(from)) | + Operand::Move(Place::Local(from)) if *from == target => { + debug!("was_captured_by_trait_object: ty={:?}", ty); + // Check the type for a trait object. + match ty.sty { + // `&dyn Trait` + ty::TyKind::Ref(_, ty, _) if ty.is_trait() => return true, + // `Box` + _ if ty.is_box() && ty.boxed_ty().is_trait() => + return true, + // `dyn Trait` + _ if ty.is_trait() => return true, + // Anything else. + _ => return false, + } + }, + _ => return false, + }, + _ => {}, } } - if found_target { - let local_decl_ty = &self.mir.local_decls[*dest].ty; - debug!("was_captured_by_trait_object: local_decl_ty={:?}", local_decl_ty); - match local_decl_ty.sty { - // `&dyn Trait` - ty::TyKind::Ref(_, ty, _) if ty.is_trait() => return true, - // `Box` - _ if local_decl_ty.is_box() && local_decl_ty.boxed_ty().is_trait() => - return true, - // `dyn Trait` - _ if local_decl_ty.is_trait() => return true, - // Anything else. - _ => return false, - } + // Continue at the next location. + queue.push(current_location.successor_within_block()); + } else { + // The only thing we need to do for terminators is progress to the next block. + let terminator = block.terminator(); + debug!("was_captured_by_trait_object: terminator={:?}", terminator); + + match &terminator.kind { + TerminatorKind::Call { + destination: Some((Place::Local(dest), block)), + args, + .. + } => { + debug!( + "was_captured_by_trait_object: target={:?} dest={:?} args={:?}", + target, dest, args + ); + // Check if one of the arguments to this function is the target place. + let found_target = args.iter().any(|arg| { + if let Operand::Move(Place::Local(potential)) = arg { + *potential == target + } else { + false + } + }); + + // If it is, follow this to the next block and update the target. + if found_target { + target = *dest; + queue.push(block.start_location()); + } + }, + _ => {}, } } + + debug!("was_captured_by_trait: queue={:?}", queue); } + // We didn't find anything and ran out of locations to check. false } } diff --git a/src/test/ui/nll/issue-52663-trait-object.rs b/src/test/ui/nll/issue-52663-trait-object.rs new file mode 100644 index 0000000000000..65d73eeae67c4 --- /dev/null +++ b/src/test/ui/nll/issue-52663-trait-object.rs @@ -0,0 +1,27 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(box_syntax)] +#![feature(nll)] + +trait Foo { fn get(&self); } + +impl Foo for A { + fn get(&self) { } +} + +fn main() { + let _ = { + let tmp0 = 3; + let tmp1 = &tmp0; + box tmp1 as Box + }; + //~^^^ ERROR `tmp0` does not live long enough +} diff --git a/src/test/ui/nll/issue-52663-trait-object.stderr b/src/test/ui/nll/issue-52663-trait-object.stderr new file mode 100644 index 0000000000000..035422f245825 --- /dev/null +++ b/src/test/ui/nll/issue-52663-trait-object.stderr @@ -0,0 +1,13 @@ +error[E0597]: `tmp0` does not live long enough + --> $DIR/issue-52663-trait-object.rs:23:20 + | +LL | let tmp1 = &tmp0; + | ^^^^^ borrowed value does not live long enough +LL | box tmp1 as Box + | ------------------------- borrow later captured here by trait object +LL | }; + | - `tmp0` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`.