From 4fff54381bde70cada702981912447f948e76e27 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 22 Sep 2018 00:51:48 +0200 Subject: [PATCH 01/10] Add flag to `mir::LocalDecl` to track whether its a temp from some subexpr a block tail expression. Slightly refactored the `LocalDecl` construction API in the process. --- src/librustc/ich/impls_mir.rs | 1 + src/librustc/mir/mod.rs | 37 ++++++++++++-- src/librustc/mir/visit.rs | 1 + src/librustc_mir/build/block.rs | 39 ++++++++++++-- src/librustc_mir/build/expr/as_temp.rs | 32 +++++++++--- src/librustc_mir/build/expr/into.rs | 22 +++++++- src/librustc_mir/build/expr/stmt.rs | 25 +++++++-- src/librustc_mir/build/matches/mod.rs | 2 + src/librustc_mir/build/mod.rs | 67 +++++++++++++++++++++++++ src/librustc_mir/shim.rs | 1 + src/librustc_mir/transform/generator.rs | 3 ++ 11 files changed, 207 insertions(+), 23 deletions(-) diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index 656f5795e18bd..9312349ee4055 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -29,6 +29,7 @@ impl_stable_hash_for!(struct mir::LocalDecl<'tcx> { source_info, visibility_scope, internal, + is_block_tail, is_user_variable }); impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, var_hir_id, by_ref, mutability }); diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index aec6bb7c3c1ff..5ea097e72433d 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -638,6 +638,18 @@ mod binding_form_impl { } } +#[derive(Clone, Debug, RustcEncodable, RustcDecodable)] +pub struct BlockTailInfo { + /// If `true`, then the value resulting from evaluating this tail + /// expression is ignored by the block's expression context. + /// + /// Examples include `{ ...; tail };` and `let _ = { ...; tail };` + /// but not e.g. `let _x = { ...; tail };` + pub tail_result_is_ignored: bool, +} + +impl_stable_hash_for!(struct BlockTailInfo { tail_result_is_ignored }); + /// A MIR local. /// /// This can be a binding declared by the user, a temporary inserted by the compiler, a function @@ -677,6 +689,12 @@ pub struct LocalDecl<'tcx> { /// generator. pub internal: bool, + /// If this local is a temporary and `is_block_tail` is `Some`, + /// then it is a temporary created for evaluation of some + /// subexpression of some block's tail expression (with no + /// intervening statement context). + pub is_block_tail: Option, + /// Type of this local. pub ty: Ty<'tcx>, @@ -825,10 +843,19 @@ impl<'tcx> LocalDecl<'tcx> { Self::new_local(ty, Mutability::Mut, false, span) } - /// Create a new immutable `LocalDecl` for a temporary. + /// Converts `self` into same `LocalDecl` except tagged as immutable. #[inline] - pub fn new_immutable_temp(ty: Ty<'tcx>, span: Span) -> Self { - Self::new_local(ty, Mutability::Not, false, span) + pub fn immutable(mut self) -> Self { + self.mutability = Mutability::Not; + self + } + + /// Converts `self` into same `LocalDecl` except tagged as internal temporary. + #[inline] + pub fn block_tail(mut self, info: BlockTailInfo) -> Self { + assert!(self.is_block_tail.is_none()); + self.is_block_tail = Some(info); + self } /// Create a new `LocalDecl` for a internal temporary. @@ -856,6 +883,7 @@ impl<'tcx> LocalDecl<'tcx> { visibility_scope: OUTERMOST_SOURCE_SCOPE, internal, is_user_variable: None, + is_block_tail: None, } } @@ -874,6 +902,7 @@ impl<'tcx> LocalDecl<'tcx> { }, visibility_scope: OUTERMOST_SOURCE_SCOPE, internal: false, + is_block_tail: None, name: None, // FIXME maybe we do want some name here? is_user_variable: None, } @@ -2668,6 +2697,7 @@ pub enum ClosureOutlivesSubject<'tcx> { */ CloneTypeFoldableAndLiftImpls! { + BlockTailInfo, Mutability, SourceInfo, UpvarDecl, @@ -2711,6 +2741,7 @@ BraceStructTypeFoldableImpl! { user_ty, name, source_info, + is_block_tail, visibility_scope, } } diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index cbfbed90c905e..7d8227053b373 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -728,6 +728,7 @@ macro_rules! make_mir_visitor { ref $($mutability)* visibility_scope, internal: _, is_user_variable: _, + is_block_tail: _, } = *local_decl; self.visit_ty(ty, TyContext::LocalDecl { diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index c4cb7958fd3c3..b754d63f7183b 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use build::{BlockAnd, BlockAndExtension, Builder}; +use build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; use build::ForGuard::OutsideGuard; use build::matches::ArmHasGuard; use hair::*; @@ -93,6 +93,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let Stmt { kind, opt_destruction_scope } = this.hir.mirror(stmt); match kind { StmtKind::Expr { scope, expr } => { + this.block_context.push(BlockFrame::Statement { ignores_expr_result: true }); unpack!(block = this.in_opt_scope( opt_destruction_scope.map(|de|(de, source_info)), block, |this| { let si = (scope, source_info); @@ -109,6 +110,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { initializer, lint_level } => { + let ignores_expr_result = if let PatternKind::Wild = *pattern.kind { + true + } else { + false + }; + this.block_context.push(BlockFrame::Statement { ignores_expr_result }); + // Enter the remainder scope, i.e. the bindings' destruction scope. this.push_scope((remainder_scope, source_info)); let_scope_stack.push(remainder_scope); @@ -155,19 +163,40 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } } + + let popped = this.block_context.pop(); + assert!(popped.map_or(false, |bf|bf.is_statement())); } + // Then, the block may have an optional trailing expression which is a “return” value - // of the block. + // of the block, which is stored into `destination`. + let tcx = this.hir.tcx(); + let destination_ty = destination.ty(&this.local_decls, tcx).to_ty(tcx); if let Some(expr) = expr { + let tail_result_is_ignored = destination_ty.is_unit() || + match this.block_context.last() { + // no context: conservatively assume result is read + None => false, + + // sub-expression: block result feeds into some computation + Some(BlockFrame::SubExpr) => false, + + // otherwise: use accumualated is_ignored state. + Some(BlockFrame::TailExpr { tail_result_is_ignored: ignored }) | + Some(BlockFrame::Statement { ignores_expr_result: ignored }) => *ignored, + }; + this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored }); + unpack!(block = this.into(destination, block, expr)); + let popped = this.block_context.pop(); + + assert!(popped.map_or(false, |bf|bf.is_tail_expr())); } else { // If a block has no trailing expression, then it is given an implicit return type. // This return type is usually `()`, unless the block is diverging, in which case the // return type is `!`. For the unit type, we need to actually return the unit, but in // the case of `!`, no return value is required, as the block will never return. - let tcx = this.hir.tcx(); - let ty = destination.ty(&this.local_decls, tcx).to_ty(tcx); - if ty.is_unit() { + if destination_ty.is_unit() { // We only want to assign an implicit `()` as the return value of the block if the // block does not diverge. (Otherwise, we may try to assign a unit to a `!`-type.) this.cfg.push_assign_unit(block, source_info, destination); diff --git a/src/librustc_mir/build/expr/as_temp.rs b/src/librustc_mir/build/expr/as_temp.rs index a2dcce6adcb6e..e0bf02c6739e3 100644 --- a/src/librustc_mir/build/expr/as_temp.rs +++ b/src/librustc_mir/build/expr/as_temp.rs @@ -10,7 +10,7 @@ //! See docs in build/expr/mod.rs -use build::{BlockAnd, BlockAndExtension, Builder}; +use build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; use hair::*; use rustc::middle::region; use rustc::mir::*; @@ -59,14 +59,30 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } let expr_ty = expr.ty; - let temp = if mutability == Mutability::Not { - this.local_decls - .push(LocalDecl::new_immutable_temp(expr_ty, expr_span)) - } else { - this.local_decls - .push(LocalDecl::new_temp(expr_ty, expr_span)) - }; + let temp = { + let mut local_decl = LocalDecl::new_temp(expr_ty, expr_span); + if mutability == Mutability::Not { + local_decl = local_decl.immutable(); + } + + debug!("creating temp {:?} with block_context: {:?}", local_decl, this.block_context); + // Find out whether this temp is being created within the + // tail expression of a block whose result is ignored. + for bf in this.block_context.iter().rev() { + match bf { + BlockFrame::SubExpr => continue, + BlockFrame::Statement { .. } => break, + &BlockFrame::TailExpr { tail_result_is_ignored } => { + local_decl = local_decl.block_tail(BlockTailInfo { + tail_result_is_ignored + }); + break; + } + } + } + this.local_decls.push(local_decl) + }; if !expr_ty.is_never() { this.cfg.push( block, diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index f85e37a6ca57b..4f5ed34a46133 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -11,7 +11,7 @@ //! See docs in build/expr/mod.rs use build::expr::category::{Category, RvalueFunc}; -use build::{BlockAnd, BlockAndExtension, Builder}; +use build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; use hair::*; use rustc::mir::*; use rustc::ty; @@ -39,7 +39,17 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let expr_span = expr.span; let source_info = this.source_info(expr_span); - match expr.kind { + let expr_is_block_or_scope = match expr.kind { + ExprKind::Block { .. } => true, + ExprKind::Scope { .. } => true, + _ => false, + }; + + if !expr_is_block_or_scope { + this.block_context.push(BlockFrame::SubExpr); + } + + let block_and = match expr.kind { ExprKind::Scope { region_scope, lint_level, @@ -302,6 +312,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { visibility_scope: source_info.scope, internal: true, is_user_variable: None, + is_block_tail: None, }); let ptr_temp = Place::Local(ptr_temp); let block = unpack!(this.into(&ptr_temp, block, ptr)); @@ -414,6 +425,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { .push_assign(block, source_info, destination, rvalue); block.unit() } + }; + + if !expr_is_block_or_scope { + let popped = this.block_context.pop(); + assert!(popped.is_some()); } + + block_and } } diff --git a/src/librustc_mir/build/expr/stmt.rs b/src/librustc_mir/build/expr/stmt.rs index 32f09599ace82..8cb2b6384b8dd 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -9,7 +9,7 @@ // except according to those terms. use build::scope::BreakableScope; -use build::{BlockAnd, BlockAndExtension, Builder}; +use build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; use hair::*; use rustc::mir::*; @@ -40,19 +40,22 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // is better for borrowck interaction with overloaded // operators like x[j] = x[i]. + this.block_context.push(BlockFrame::SubExpr); + // Generate better code for things that don't need to be // dropped. if this.hir.needs_drop(lhs.ty) { let rhs = unpack!(block = this.as_local_operand(block, rhs)); let lhs = unpack!(block = this.as_place(block, lhs)); unpack!(block = this.build_drop_and_replace(block, lhs_span, lhs, rhs)); - block.unit() } else { let rhs = unpack!(block = this.as_local_rvalue(block, rhs)); let lhs = unpack!(block = this.as_place(block, lhs)); this.cfg.push_assign(block, source_info, &lhs, rhs); - block.unit() } + + this.block_context.pop(); + block.unit() } ExprKind::AssignOp { op, lhs, rhs } => { // FIXME(#28160) there is an interesting semantics @@ -66,6 +69,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let lhs = this.hir.mirror(lhs); let lhs_ty = lhs.ty; + this.block_context.push(BlockFrame::SubExpr); + // As above, RTL. let rhs = unpack!(block = this.as_local_operand(block, rhs)); let lhs = unpack!(block = this.as_place(block, lhs)); @@ -85,6 +90,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ); this.cfg.push_assign(block, source_info, &lhs, result); + this.block_context.pop(); block.unit() } ExprKind::Continue { label } => { @@ -114,7 +120,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { (break_block, region_scope, break_destination.clone()) }; if let Some(value) = value { - unpack!(block = this.into(&destination, block, value)) + this.block_context.push(BlockFrame::SubExpr); + unpack!(block = this.into(&destination, block, value)); + this.block_context.pop(); } else { this.cfg.push_assign_unit(block, source_info, &destination) } @@ -123,7 +131,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } ExprKind::Return { value } => { block = match value { - Some(value) => unpack!(this.into(&Place::Local(RETURN_PLACE), block, value)), + Some(value) => { + this.block_context.push(BlockFrame::SubExpr); + let result = unpack!(this.into(&Place::Local(RETURN_PLACE), block, value)); + this.block_context.pop(); + result + } None => { this.cfg .push_assign_unit(block, source_info, &Place::Local(RETURN_PLACE)); @@ -140,6 +153,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { outputs, inputs, } => { + this.block_context.push(BlockFrame::SubExpr); let outputs = outputs .into_iter() .map(|output| unpack!(block = this.as_place(block, output))) @@ -161,6 +175,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }, }, ); + this.block_context.pop(); block.unit() } _ => { diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index e40ed51f7d354..656c78a46ed78 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -1485,6 +1485,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { source_info, visibility_scope, internal: false, + is_block_tail: None, is_user_variable: Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { binding_mode, // hypothetically, `visit_bindings` could try to unzip @@ -1518,6 +1519,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { visibility_scope, // FIXME: should these secretly injected ref_for_guard's be marked as `internal`? internal: false, + is_block_tail: None, is_user_variable: Some(ClearCrossCrate::Set(BindingForm::RefForGuard)), }); LocalsForNode::ForGuard { diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 3dbd3bbb41573..9e78932bffea6 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -281,6 +281,57 @@ fn liberated_closure_env_ty<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, tcx.liberate_late_bound_regions(closure_def_id, &closure_env_ty) } +#[derive(Debug, PartialEq, Eq)] +pub enum BlockFrame { + /// Evaluation is currently within a statement. + /// + /// Examples include: + /// 1. `EXPR;` + /// 2. `let _ = EXPR;` + /// 3. `let x = EXPR;` + Statement { + /// If true, then statement discards result from evaluating + /// the expression (such as examples 1 and 2 above). + ignores_expr_result: bool + }, + + /// Evaluation is currently within the tail expression of a block. + /// + /// Example: `{ STMT_1; STMT_2; EXPR }` + TailExpr { + /// If true, then the surrounding context of the block ignores + /// the result of evaluating the block's tail expression. + /// + /// Example: `let _ = { STMT_1; EXPR };` + tail_result_is_ignored: bool + }, + + /// Generic mark meaning that the block occurred as a subexpression + /// where the result might be used. + /// + /// Examples: `foo(EXPR)`, `match EXPR { ... }` + SubExpr, +} + +impl BlockFrame { + fn is_tail_expr(&self) -> bool { + match *self { + BlockFrame::TailExpr { .. } => true, + + BlockFrame::Statement { .. } | + BlockFrame::SubExpr => false, + } + } + fn is_statement(&self) -> bool { + match *self { + BlockFrame::Statement { .. } => true, + + BlockFrame::TailExpr { .. } | + BlockFrame::SubExpr => false, + } + } + } + struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { hir: Cx<'a, 'gcx, 'tcx>, cfg: CFG<'tcx>, @@ -292,6 +343,20 @@ struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { /// see the `scope` module for more details scopes: Vec>, + /// the block-context: each time we build the code within an hair::Block, + /// we push a frame here tracking whether we are building a statement or + /// if we are pushing the tail expression of the block. This is used to + /// embed information in generated temps about whether they were created + /// for a block tail expression or not. + /// + /// It would be great if we could fold this into `self.scopes` + /// somehow; but right now I think that is very tightly tied to + /// the code generation in ways that we cannot (or should not) + /// start just throwing new entries onto that vector in order to + /// distinguish the context of EXPR1 from the context of EXPR2 in + /// `{ STMTS; EXPR1 } + EXPR2` + block_context: Vec, + /// The current unsafe block in scope, even if it is hidden by /// a PushUnsafeBlock unpushed_unsafe: Safety, @@ -695,6 +760,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { fn_span: span, arg_count, scopes: vec![], + block_context: vec![], source_scopes: IndexVec::new(), source_scope: OUTERMOST_SOURCE_SCOPE, source_scope_local_data: IndexVec::new(), @@ -781,6 +847,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { name, internal: false, is_user_variable: None, + is_block_tail: None, }); } diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 6c3dd0ea3cc37..4c9d29bb6d070 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -147,6 +147,7 @@ fn temp_decl(mutability: Mutability, ty: Ty, span: Span) -> LocalDecl { visibility_scope: source_info.scope, internal: false, is_user_variable: None, + is_block_tail: None, } } diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 62adbf1bdf7db..c2ae6832cc09f 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -308,6 +308,7 @@ fn replace_result_variable<'tcx>( source_info, visibility_scope: source_info.scope, internal: false, + is_block_tail: None, is_user_variable: None, }; let new_ret_local = Local::new(mir.local_decls.len()); @@ -662,6 +663,7 @@ fn create_generator_drop_shim<'a, 'tcx>( source_info, visibility_scope: source_info.scope, internal: false, + is_block_tail: None, is_user_variable: None, }; @@ -679,6 +681,7 @@ fn create_generator_drop_shim<'a, 'tcx>( source_info, visibility_scope: source_info.scope, internal: false, + is_block_tail: None, is_user_variable: None, }; From a75afd82c9af37aa22b9531c165f05df54c540f4 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 28 Sep 2018 11:50:05 +0200 Subject: [PATCH 02/10] Update doc for `explain_why_borrow_contains_point` to reflect its newer API. --- src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 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 6b5c9f0333e3e..7d6e5dcabb104 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -95,7 +95,7 @@ impl<'tcx> BorrowExplanation<'tcx> { } impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { - /// Adds annotations to `err` explaining *why* the borrow contains the + /// Returns structured explanation for *why* the borrow contains the /// point from `context`. This is key for the "3-point errors" /// [described in the NLL RFC][d]. /// @@ -106,7 +106,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { /// - `kind_place`: if Some, this describes the statement that triggered the error. /// - first half is the kind of write, if any, being performed /// - second half is the place being accessed - /// - `err`: where the error annotations are going to be added /// /// [d]: https://rust-lang.github.io/rfcs/2094-nll.html#leveraging-intuition-framing-errors-in-terms-of-points pub(in borrow_check) fn explain_why_borrow_contains_point( From 74f8263032418f8dd6196c07f086a5d4462d23ad Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 28 Sep 2018 11:50:26 +0200 Subject: [PATCH 03/10] Fix `debug!` to reflect rename of `explain_why_borrow_contains_point`. --- src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7d6e5dcabb104..5092cd4a8b924 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -115,7 +115,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { kind_place: Option<(WriteKind, &Place<'tcx>)>, ) -> BorrowExplanation<'tcx> { debug!( - "find_why_borrow_contains_point(context={:?}, borrow={:?})", + "explain_why_borrow_contains_point(context={:?}, borrow={:?})", context, borrow, ); From 8532c1896df19eb9544ff3c61a684faa0b35df73 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 28 Sep 2018 12:19:28 +0200 Subject: [PATCH 04/10] Alpha-rename `BorrowExplanation::emit` to `BorrowExplanation::add_explanation_to_diagnostic`. (I found it confusing to have calls to an `emit` method in our error_reporting module where that `emit` method *wasn't* the `DiagnosticBuffer::emit` method.) --- src/librustc_mir/borrow_check/error_reporting.rs | 16 ++++++++-------- .../borrow_check/nll/explain_borrow/mod.rs | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 1589d62300cab..45435c64749e2 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -262,7 +262,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { move_spans.var_span_label(&mut err, "move occurs due to use in closure"); self.explain_why_borrow_contains_point(context, borrow, None) - .emit(self.infcx.tcx, &mut err, String::new()); + .add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); err.buffer(&mut self.errors_buffer); } @@ -299,7 +299,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }); self.explain_why_borrow_contains_point(context, borrow, None) - .emit(self.infcx.tcx, &mut err, String::new()); + .add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); err.buffer(&mut self.errors_buffer); } @@ -483,7 +483,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } self.explain_why_borrow_contains_point(context, issued_borrow, None) - .emit(self.infcx.tcx, &mut err, first_borrow_desc.to_string()); + .add_explanation_to_diagnostic(self.infcx.tcx, &mut err, first_borrow_desc.to_string()); err.buffer(&mut self.errors_buffer); } @@ -638,7 +638,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let BorrowExplanation::MustBeValidFor(..) = explanation { } else { - explanation.emit(self.infcx.tcx, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); } } else { err.span_label(borrow_span, "borrowed value does not live long enough"); @@ -649,7 +649,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { borrow_spans.args_span_label(&mut err, "value captured here"); - explanation.emit(self.infcx.tcx, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); } err @@ -709,7 +709,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { _ => {} } - explanation.emit(self.infcx.tcx, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); err.buffer(&mut self.errors_buffer); } @@ -776,7 +776,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } _ => {} } - explanation.emit(self.infcx.tcx, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); borrow_spans.args_span_label(&mut err, "value captured here"); @@ -913,7 +913,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { loan_spans.var_span_label(&mut err, "borrow occurs due to use in closure"); self.explain_why_borrow_contains_point(context, loan, None) - .emit(self.infcx.tcx, &mut err, String::new()); + .add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); err.buffer(&mut self.errors_buffer); } 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 5092cd4a8b924..d2f707ed2e8e9 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -37,7 +37,7 @@ pub(in borrow_check) enum LaterUseKind { } impl<'tcx> BorrowExplanation<'tcx> { - pub(in borrow_check) fn emit<'cx, 'gcx>( + pub(in borrow_check) fn add_explanation_to_diagnostic<'cx, 'gcx>( &self, tcx: TyCtxt<'cx, 'gcx, 'tcx>, err: &mut DiagnosticBuilder<'_>, From 9eebe77a86cb07e7444f07b7b7dac879f5878269 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 28 Sep 2018 12:23:40 +0200 Subject: [PATCH 05/10] Have `add_explanation_to_diagnostic` also take `Mir` as parameter. This is preparation for allowing the `BorrowExplanation` carry things like `mir::Location` or `mir::Local` and still be able to generate usable diagnostic text. --- src/librustc_mir/borrow_check/error_reporting.rs | 16 ++++++++-------- .../borrow_check/nll/explain_borrow/mod.rs | 3 ++- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 45435c64749e2..5dcab038baeb1 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -262,7 +262,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { move_spans.var_span_label(&mut err, "move occurs due to use in closure"); self.explain_why_borrow_contains_point(context, borrow, None) - .add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); + .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); err.buffer(&mut self.errors_buffer); } @@ -299,7 +299,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }); self.explain_why_borrow_contains_point(context, borrow, None) - .add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); + .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); err.buffer(&mut self.errors_buffer); } @@ -483,7 +483,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } self.explain_why_borrow_contains_point(context, issued_borrow, None) - .add_explanation_to_diagnostic(self.infcx.tcx, &mut err, first_borrow_desc.to_string()); + .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, first_borrow_desc.to_string()); err.buffer(&mut self.errors_buffer); } @@ -638,7 +638,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let BorrowExplanation::MustBeValidFor(..) = explanation { } else { - explanation.add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); } } else { err.span_label(borrow_span, "borrowed value does not live long enough"); @@ -649,7 +649,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { borrow_spans.args_span_label(&mut err, "value captured here"); - explanation.add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); } err @@ -709,7 +709,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { _ => {} } - explanation.add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); err.buffer(&mut self.errors_buffer); } @@ -776,7 +776,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } _ => {} } - explanation.add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); borrow_spans.args_span_label(&mut err, "value captured here"); @@ -913,7 +913,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { loan_spans.var_span_label(&mut err, "borrow occurs due to use in closure"); self.explain_why_borrow_contains_point(context, loan, None) - .add_explanation_to_diagnostic(self.infcx.tcx, &mut err, String::new()); + .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); err.buffer(&mut self.errors_buffer); } 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 d2f707ed2e8e9..3319113aec074 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -13,7 +13,7 @@ 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, Operand, Place, StatementKind, TerminatorKind}; +use rustc::mir::{FakeReadCause, Location, Mir, Operand, Place, StatementKind, TerminatorKind}; use rustc_errors::DiagnosticBuilder; use syntax_pos::Span; use syntax_pos::symbol::Symbol; @@ -40,6 +40,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>, err: &mut DiagnosticBuilder<'_>, borrow_desc: String, ) { From 504ab34e6298bf7c743d1df0cd3f48522355552d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 5 Oct 2018 11:00:05 +0200 Subject: [PATCH 06/10] Replaced `String` with `&str` in API for `add_explanation_to_diagnostic`. --- src/librustc_mir/borrow_check/error_reporting.rs | 16 ++++++++-------- .../borrow_check/nll/explain_borrow/mod.rs | 12 ++++++------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 5dcab038baeb1..b9d849966a604 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -262,7 +262,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { move_spans.var_span_label(&mut err, "move occurs due to use in closure"); self.explain_why_borrow_contains_point(context, borrow, None) - .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); + .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, ""); err.buffer(&mut self.errors_buffer); } @@ -299,7 +299,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }); self.explain_why_borrow_contains_point(context, borrow, None) - .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); + .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, ""); err.buffer(&mut self.errors_buffer); } @@ -483,7 +483,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } self.explain_why_borrow_contains_point(context, issued_borrow, None) - .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, first_borrow_desc.to_string()); + .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, first_borrow_desc); err.buffer(&mut self.errors_buffer); } @@ -638,7 +638,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let BorrowExplanation::MustBeValidFor(..) = explanation { } else { - explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, ""); } } else { err.span_label(borrow_span, "borrowed value does not live long enough"); @@ -649,7 +649,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { borrow_spans.args_span_label(&mut err, "value captured here"); - explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, ""); } err @@ -709,7 +709,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { _ => {} } - explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, ""); err.buffer(&mut self.errors_buffer); } @@ -776,7 +776,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } _ => {} } - explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); + explanation.add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, ""); borrow_spans.args_span_label(&mut err, "value captured here"); @@ -913,7 +913,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { loan_spans.var_span_label(&mut err, "borrow occurs due to use in closure"); self.explain_why_borrow_contains_point(context, loan, None) - .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, String::new()); + .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, ""); err.buffer(&mut self.errors_buffer); } 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 3319113aec074..d1d6ba123727c 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -42,20 +42,20 @@ impl<'tcx> BorrowExplanation<'tcx> { tcx: TyCtxt<'cx, 'gcx, 'tcx>, _mir: &Mir<'tcx>, err: &mut DiagnosticBuilder<'_>, - borrow_desc: String, + borrow_desc: &str, ) { match *self { BorrowExplanation::UsedLater(later_use_kind, var_or_use_span) => { - let message = borrow_desc + match later_use_kind { + let message = match later_use_kind { LaterUseKind::ClosureCapture => "borrow later captured here by closure", LaterUseKind::Call => "borrow later used by call", LaterUseKind::FakeLetRead => "borrow later stored here", LaterUseKind::Other => "borrow later used here", }; - err.span_label(var_or_use_span, message); + err.span_label(var_or_use_span, format!("{}{}", borrow_desc, message)); }, BorrowExplanation::UsedLaterInLoop(later_use_kind, var_or_use_span) => { - let message = borrow_desc + match later_use_kind { + let message = match later_use_kind { LaterUseKind::ClosureCapture => { "borrow captured here by closure, in later iteration of loop" }, @@ -63,7 +63,7 @@ impl<'tcx> BorrowExplanation<'tcx> { LaterUseKind::FakeLetRead => "borrow later stored here", LaterUseKind::Other => "borrow used here, in later iteration of loop", }; - err.span_label(var_or_use_span, message); + err.span_label(var_or_use_span, format!("{}{}", borrow_desc, message)); }, BorrowExplanation::UsedLaterWhenDropped(span, local_name, should_note_order) => { err.span_label( @@ -85,7 +85,7 @@ impl<'tcx> BorrowExplanation<'tcx> { BorrowExplanation::MustBeValidFor(region) => { tcx.note_and_explain_free_region( err, - &(borrow_desc + "borrowed value must be valid for "), + &format!("{}{}", borrow_desc, "borrowed value must be valid for "), region, "...", ); From 80bc17108eedeb5f529ef380337e8ad48babab64 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 28 Sep 2018 15:25:02 +0200 Subject: [PATCH 07/10] Improve diagnostics for borrow-check errors that result from drops of temporary r-values. Changed `BorrowExplanation::UsedLaterWhenDropped` to handle both named locals and also unnamed (aka temporaries). If the dropped temporary does not implement `Drop`, then we print its full type; but when the dropped temporary is itself an ADT `D` that implements `Drop`, then diagnostic points the user directly at `D`. --- .../borrow_check/error_reporting.rs | 2 +- .../borrow_check/nll/explain_borrow/mod.rs | 147 ++++++++++++------ 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 b9d849966a604..53a190efb5835 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 d1d6ba123727c..e55469436abf0 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 8cb2b6384b8dd..d2b39f088b65e 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() From 37f100312183364d52fe1468dc81a693783e4387 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 29 Sep 2018 23:30:43 +0200 Subject: [PATCH 08/10] Updates to .stderr output in ui tests from earlier changes. --- .../dropck-eyepatch-extern-crate.nll.stderr | 2 +- .../dropck/dropck-eyepatch-reorder.nll.stderr | 2 +- src/test/ui/dropck/dropck-eyepatch.nll.stderr | 2 +- src/test/ui/dropck/dropck-union.nll.stderr | 2 +- src/test/ui/error-codes/E0597.nll.stderr | 2 +- src/test/ui/generator/borrowing.nll.stderr | 11 +++++++-- src/test/ui/generator/dropck.nll.stderr | 4 ++-- src/test/ui/issues/issue-18783.nll.stderr | 4 ++-- src/test/ui/issues/issue-47646.stderr | 8 ++++++- src/test/ui/nll/drop-no-may-dangle.stderr | 4 ++-- ...ialized-drop-implicit-fragment-drop.stderr | 2 +- ...aybe-initialized-drop-with-fragment.stderr | 2 +- ...d-drop-with-uninitialized-fragments.stderr | 2 +- src/test/ui/nll/maybe-initialized-drop.stderr | 2 +- .../span/destructor-restrictions.nll.stderr | 11 +++++++-- .../ui/span/dropck-object-cycle.nll.stderr | 2 +- .../span/dropck_arr_cycle_checked.nll.stderr | 6 ++--- .../dropck_direct_cycle_with_drop.nll.stderr | 4 ++-- .../ui/span/dropck_misc_variants.nll.stderr | 4 ++-- .../span/dropck_vec_cycle_checked.nll.stderr | 6 ++--- ...locals-die-before-temps-of-body.nll.stderr | 23 +++++++++++++++---- ...opck-child-has-items-via-parent.nll.stderr | 2 +- ...ue-24805-dropck-trait-has-items.nll.stderr | 6 ++--- .../issue-24895-copy-clone-dropck.nll.stderr | 2 +- src/test/ui/span/issue-25199.nll.stderr | 2 +- src/test/ui/span/issue-26656.nll.stderr | 2 +- src/test/ui/span/issue-29106.nll.stderr | 4 ++-- .../ui/span/issue28498-reject-ex1.nll.stderr | 2 +- ...ssue28498-reject-lifetime-param.nll.stderr | 2 +- .../issue28498-reject-passed-to-fn.nll.stderr | 2 +- .../issue28498-reject-trait-bound.nll.stderr | 2 +- ...-close-over-borrowed-ref-in-obj.nll.stderr | 2 +- .../send-is-not-static-std-sync.nll.stderr | 2 +- ...-must-not-hide-type-from-dropck.nll.stderr | 4 ++-- .../vec_refs_data_with_early_death.nll.stderr | 4 ++-- ...-closures-failed-recursive-fn-1.nll.stderr | 2 +- 36 files changed, 90 insertions(+), 55 deletions(-) diff --git a/src/test/ui/dropck/dropck-eyepatch-extern-crate.nll.stderr b/src/test/ui/dropck/dropck-eyepatch-extern-crate.nll.stderr index b59f628c746bb..e0b7fc4eb9054 100644 --- a/src/test/ui/dropck/dropck-eyepatch-extern-crate.nll.stderr +++ b/src/test/ui/dropck/dropck-eyepatch-extern-crate.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `c_shortest` dropped here while still borrowed - | borrow later used here, when `dt` is dropped + | borrow might be used here, when `dt` is dropped and runs the `Drop` code for type `other::Dt` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/dropck/dropck-eyepatch-reorder.nll.stderr b/src/test/ui/dropck/dropck-eyepatch-reorder.nll.stderr index ad460fcf55e57..97c1caa87f5c5 100644 --- a/src/test/ui/dropck/dropck-eyepatch-reorder.nll.stderr +++ b/src/test/ui/dropck/dropck-eyepatch-reorder.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `c_shortest` dropped here while still borrowed - | borrow later used here, when `dt` is dropped + | borrow might be used here, when `dt` is dropped and runs the `Drop` code for type `Dt` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/dropck/dropck-eyepatch.nll.stderr b/src/test/ui/dropck/dropck-eyepatch.nll.stderr index 864fb2f2ea6f4..4a6e42ef94a85 100644 --- a/src/test/ui/dropck/dropck-eyepatch.nll.stderr +++ b/src/test/ui/dropck/dropck-eyepatch.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `c_shortest` dropped here while still borrowed - | borrow later used here, when `dt` is dropped + | borrow might be used here, when `dt` is dropped and runs the `Drop` code for type `Dt` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/dropck/dropck-union.nll.stderr b/src/test/ui/dropck/dropck-union.nll.stderr index ffb322b85dc34..b828a16756101 100644 --- a/src/test/ui/dropck/dropck-union.nll.stderr +++ b/src/test/ui/dropck/dropck-union.nll.stderr @@ -7,7 +7,7 @@ LL | } | - | | | `v` dropped here while still borrowed - | borrow later used here, when `v` is dropped + | borrow might be used here, when `v` is dropped and runs the `Drop` code for type `Wrap` error: aborting due to previous error diff --git a/src/test/ui/error-codes/E0597.nll.stderr b/src/test/ui/error-codes/E0597.nll.stderr index 31ba24b0004fb..54a46f612b9ed 100644 --- a/src/test/ui/error-codes/E0597.nll.stderr +++ b/src/test/ui/error-codes/E0597.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `y` dropped here while still borrowed - | borrow later used here, when `x` is dropped + | borrow might be used here, when `x` is dropped and runs the `Drop` code for type `Foo` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/generator/borrowing.nll.stderr b/src/test/ui/generator/borrowing.nll.stderr index 2488df7772b3f..9d1a52a833543 100644 --- a/src/test/ui/generator/borrowing.nll.stderr +++ b/src/test/ui/generator/borrowing.nll.stderr @@ -2,10 +2,17 @@ error[E0597]: `a` does not live long enough --> $DIR/borrowing.rs:18:18 | LL | unsafe { (|| yield &a).resume() } - | ^^^^^^^^^^^^^ borrowed value does not live long enough + | ^^^^^^^^^^^^^ + | | + | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... LL | //~^ ERROR: `a` does not live long enough LL | }; - | - `a` dropped here while still borrowed + | -- ... and the borrow might be used here, when that temporary is dropped and runs the destructor for generator + | | + | `a` dropped here while still borrowed + | + = note: 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. error[E0597]: `a` does not live long enough --> $DIR/borrowing.rs:24:9 diff --git a/src/test/ui/generator/dropck.nll.stderr b/src/test/ui/generator/dropck.nll.stderr index ef7e64ffd97ae..2b95a5caf6da0 100644 --- a/src/test/ui/generator/dropck.nll.stderr +++ b/src/test/ui/generator/dropck.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `*cell` dropped here while still borrowed - | borrow later used here, when `gen` is dropped + | borrow might be used here, when `gen` is dropped and runs the destructor for generator | = note: values in a scope are dropped in the opposite order they are defined @@ -27,7 +27,7 @@ LL | } | - | | | `ref_` dropped here while still borrowed - | borrow later used here, when `gen` is dropped + | borrow might be used here, when `gen` is dropped and runs the destructor for generator | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/issues/issue-18783.nll.stderr b/src/test/ui/issues/issue-18783.nll.stderr index 2531279f7508b..023021eccb931 100644 --- a/src/test/ui/issues/issue-18783.nll.stderr +++ b/src/test/ui/issues/issue-18783.nll.stderr @@ -11,7 +11,7 @@ LL | c.push(Box::new(|| y = 0)); | second mutable borrow occurs here LL | //~^ ERROR cannot borrow `y` as mutable more than once at a time LL | } - | - first borrow later used here, when `c` is dropped + | - first borrow might be used here, when `c` is dropped and runs the destructor for type `std::cell::RefCell>>` error[E0499]: cannot borrow `y` as mutable more than once at a time --> $DIR/issue-18783.rs:26:29 @@ -26,7 +26,7 @@ LL | Push::push(&c, Box::new(|| y = 0)); | second mutable borrow occurs here LL | //~^ ERROR cannot borrow `y` as mutable more than once at a time LL | } - | - first borrow later used here, when `c` is dropped + | - first borrow might be used here, when `c` is dropped and runs the destructor for type `std::cell::RefCell>>` error: aborting due to 2 previous errors diff --git a/src/test/ui/issues/issue-47646.stderr b/src/test/ui/issues/issue-47646.stderr index 4bdce85d18b5f..116cb3be6b50c 100644 --- a/src/test/ui/issues/issue-47646.stderr +++ b/src/test/ui/issues/issue-47646.stderr @@ -3,9 +3,15 @@ error[E0502]: cannot borrow `heap` as immutable because it is also borrowed as m | LL | let borrow = heap.peek_mut(); | ---- mutable borrow occurs here -... +LL | +LL | match (borrow, ()) { + | ------------ a temporary with access to the mutable borrow is created here ... +LL | (Some(_), ()) => { LL | println!("{:?}", heap); //~ ERROR cannot borrow `heap` as immutable | ^^^^ immutable borrow occurs here +... +LL | }; + | - ... and the mutable borrow might be used here, when that temporary is dropped and runs the destructor for type `(std::option::Option>, ())` error: aborting due to previous error diff --git a/src/test/ui/nll/drop-no-may-dangle.stderr b/src/test/ui/nll/drop-no-may-dangle.stderr index a35271bdcfeff..60862d0f22952 100644 --- a/src/test/ui/nll/drop-no-may-dangle.stderr +++ b/src/test/ui/nll/drop-no-may-dangle.stderr @@ -8,7 +8,7 @@ LL | v[0] += 1; //~ ERROR cannot assign to `v[..]` because it is borrowe | ^^^^^^^^^ assignment to borrowed `v[..]` occurs here ... LL | } - | - borrow later used here, when `p` is dropped + | - borrow might be used here, when `p` is dropped and runs the `Drop` code for type `WrapMayNotDangle` error[E0506]: cannot assign to `v[..]` because it is borrowed --> $DIR/drop-no-may-dangle.rs:33:5 @@ -19,7 +19,7 @@ LL | let p: WrapMayNotDangle<&usize> = WrapMayNotDangle { value: &v[0] }; LL | v[0] += 1; //~ ERROR cannot assign to `v[..]` because it is borrowed | ^^^^^^^^^ assignment to borrowed `v[..]` occurs here LL | } - | - borrow later used here, when `p` is dropped + | - borrow might be used here, when `p` is dropped and runs the `Drop` code for type `WrapMayNotDangle` error: aborting due to 2 previous errors diff --git a/src/test/ui/nll/maybe-initialized-drop-implicit-fragment-drop.stderr b/src/test/ui/nll/maybe-initialized-drop-implicit-fragment-drop.stderr index 6fc26d502d30d..fc5417ac80c44 100644 --- a/src/test/ui/nll/maybe-initialized-drop-implicit-fragment-drop.stderr +++ b/src/test/ui/nll/maybe-initialized-drop-implicit-fragment-drop.stderr @@ -8,7 +8,7 @@ LL | x = 1; //~ ERROR cannot assign to `x` because it is borrowed [E0506] | ^^^^^ assignment to borrowed `x` occurs here LL | // FIXME ^ Should not error in the future with implicit dtors, only manually implemented ones LL | } - | - borrow later used here, when `foo` is dropped + | - borrow might be used here, when `foo` is dropped and runs the destructor for type `Foo<'_>` error: aborting due to previous error diff --git a/src/test/ui/nll/maybe-initialized-drop-with-fragment.stderr b/src/test/ui/nll/maybe-initialized-drop-with-fragment.stderr index 54be12f289545..6ae026fb169b0 100644 --- a/src/test/ui/nll/maybe-initialized-drop-with-fragment.stderr +++ b/src/test/ui/nll/maybe-initialized-drop-with-fragment.stderr @@ -7,7 +7,7 @@ LL | let wrap = Wrap { p: &mut x }; LL | x = 1; //~ ERROR cannot assign to `x` because it is borrowed [E0506] | ^^^^^ assignment to borrowed `x` occurs here LL | } - | - borrow later used here, when `foo` is dropped + | - borrow might be used here, when `foo` is dropped and runs the destructor for type `Foo<'_>` error: aborting due to previous error diff --git a/src/test/ui/nll/maybe-initialized-drop-with-uninitialized-fragments.stderr b/src/test/ui/nll/maybe-initialized-drop-with-uninitialized-fragments.stderr index ee926e4279318..dd16e9a0e840c 100644 --- a/src/test/ui/nll/maybe-initialized-drop-with-uninitialized-fragments.stderr +++ b/src/test/ui/nll/maybe-initialized-drop-with-uninitialized-fragments.stderr @@ -8,7 +8,7 @@ LL | x = 1; //~ ERROR cannot assign to `x` because it is borrowed [E0506] | ^^^^^ assignment to borrowed `x` occurs here LL | // FIXME ^ This currently errors and it should not. LL | } - | - borrow later used here, when `foo` is dropped + | - borrow might be used here, when `foo` is dropped and runs the destructor for type `Foo<'_>` error: aborting due to previous error diff --git a/src/test/ui/nll/maybe-initialized-drop.stderr b/src/test/ui/nll/maybe-initialized-drop.stderr index cc842c29ccb15..e8f427826ba33 100644 --- a/src/test/ui/nll/maybe-initialized-drop.stderr +++ b/src/test/ui/nll/maybe-initialized-drop.stderr @@ -6,7 +6,7 @@ LL | let wrap = Wrap { p: &mut x }; LL | x = 1; //~ ERROR cannot assign to `x` because it is borrowed [E0506] | ^^^^^ assignment to borrowed `x` occurs here LL | } - | - borrow later used here, when `wrap` is dropped + | - borrow might be used here, when `wrap` is dropped and runs the `Drop` code for type `Wrap` error: aborting due to previous error diff --git a/src/test/ui/span/destructor-restrictions.nll.stderr b/src/test/ui/span/destructor-restrictions.nll.stderr index 22f2f13f1f7cf..99a839f76d33c 100644 --- a/src/test/ui/span/destructor-restrictions.nll.stderr +++ b/src/test/ui/span/destructor-restrictions.nll.stderr @@ -2,9 +2,16 @@ error[E0597]: `*a` does not live long enough --> $DIR/destructor-restrictions.rs:18:10 | LL | *a.borrow() + 1 - | ^ borrowed value does not live long enough + | ^--------- + | | + | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... LL | }; //~^ ERROR `*a` does not live long enough - | - `*a` dropped here while still borrowed + | -- ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::cell::Ref<'_, i32>` + | | + | `*a` dropped here while still borrowed + | + = note: 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. error: aborting due to previous error diff --git a/src/test/ui/span/dropck-object-cycle.nll.stderr b/src/test/ui/span/dropck-object-cycle.nll.stderr index 08e4b9ec9faa2..52d4007f3f8f6 100644 --- a/src/test/ui/span/dropck-object-cycle.nll.stderr +++ b/src/test/ui/span/dropck-object-cycle.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `*m` dropped here while still borrowed - | borrow later used here, when `m` is dropped + | borrow might be used here, when `m` is dropped and runs the destructor for type `std::boxed::Box>` error: aborting due to previous error diff --git a/src/test/ui/span/dropck_arr_cycle_checked.nll.stderr b/src/test/ui/span/dropck_arr_cycle_checked.nll.stderr index 74db695e7e560..32abfd81f53a6 100644 --- a/src/test/ui/span/dropck_arr_cycle_checked.nll.stderr +++ b/src/test/ui/span/dropck_arr_cycle_checked.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `b2` dropped here while still borrowed - | borrow later used here, when `b1` is dropped + | borrow might be used here, when `b1` is dropped and runs the destructor for type `B<'_>` | = note: values in a scope are dropped in the opposite order they are defined @@ -22,7 +22,7 @@ LL | } | - | | | `b3` dropped here while still borrowed - | borrow later used here, when `b1` is dropped + | borrow might be used here, when `b1` is dropped and runs the destructor for type `B<'_>` | = note: values in a scope are dropped in the opposite order they are defined @@ -36,7 +36,7 @@ LL | } | - | | | `b1` dropped here while still borrowed - | borrow later used here, when `b1` is dropped + | borrow might be used here, when `b1` is dropped and runs the destructor for type `B<'_>` error: aborting due to 3 previous errors diff --git a/src/test/ui/span/dropck_direct_cycle_with_drop.nll.stderr b/src/test/ui/span/dropck_direct_cycle_with_drop.nll.stderr index baf3cef2ae83c..0c5c49c80de5d 100644 --- a/src/test/ui/span/dropck_direct_cycle_with_drop.nll.stderr +++ b/src/test/ui/span/dropck_direct_cycle_with_drop.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `d2` dropped here while still borrowed - | borrow later used here, when `d1` is dropped + | borrow might be used here, when `d1` is dropped and runs the `Drop` code for type `D` | = note: values in a scope are dropped in the opposite order they are defined @@ -22,7 +22,7 @@ LL | } | - | | | `d1` dropped here while still borrowed - | borrow later used here, when `d1` is dropped + | borrow might be used here, when `d1` is dropped and runs the `Drop` code for type `D` error: aborting due to 2 previous errors diff --git a/src/test/ui/span/dropck_misc_variants.nll.stderr b/src/test/ui/span/dropck_misc_variants.nll.stderr index 27a52360bb701..fb68262b0ae2a 100644 --- a/src/test/ui/span/dropck_misc_variants.nll.stderr +++ b/src/test/ui/span/dropck_misc_variants.nll.stderr @@ -7,7 +7,7 @@ LL | } | - | | | `bomb` dropped here while still borrowed - | borrow later used here, when `_w` is dropped + | borrow might be used here, when `_w` is dropped and runs the destructor for type `Wrap<&[&str]>` | = note: values in a scope are dropped in the opposite order they are defined @@ -21,7 +21,7 @@ LL | } | - | | | `v` dropped here while still borrowed - | borrow later used here, when `_w` is dropped + | borrow might be used here, when `_w` is dropped and runs the destructor for closure | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/dropck_vec_cycle_checked.nll.stderr b/src/test/ui/span/dropck_vec_cycle_checked.nll.stderr index f7ff4e5169fd4..ad62936b870c8 100644 --- a/src/test/ui/span/dropck_vec_cycle_checked.nll.stderr +++ b/src/test/ui/span/dropck_vec_cycle_checked.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `c2` dropped here while still borrowed - | borrow later used here, when `c1` is dropped + | borrow might be used here, when `c1` is dropped and runs the destructor for type `C<'_>` | = note: values in a scope are dropped in the opposite order they are defined @@ -22,7 +22,7 @@ LL | } | - | | | `c3` dropped here while still borrowed - | borrow later used here, when `c1` is dropped + | borrow might be used here, when `c1` is dropped and runs the destructor for type `C<'_>` | = note: values in a scope are dropped in the opposite order they are defined @@ -36,7 +36,7 @@ LL | } | - | | | `c1` dropped here while still borrowed - | borrow later used here, when `c1` is dropped + | borrow might be used here, when `c1` is dropped and runs the destructor for type `C<'_>` error: aborting due to 3 previous errors diff --git a/src/test/ui/span/issue-23338-locals-die-before-temps-of-body.nll.stderr b/src/test/ui/span/issue-23338-locals-die-before-temps-of-body.nll.stderr index fe0187f386378..9e82884b95440 100644 --- a/src/test/ui/span/issue-23338-locals-die-before-temps-of-body.nll.stderr +++ b/src/test/ui/span/issue-23338-locals-die-before-temps-of-body.nll.stderr @@ -2,17 +2,32 @@ error[E0597]: `y` does not live long enough --> $DIR/issue-23338-locals-die-before-temps-of-body.rs:20:5 | LL | y.borrow().clone() - | ^ borrowed value does not live long enough + | ^--------- + | | + | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... LL | } - | - `y` dropped here while still borrowed + | - + | | + | `y` dropped here while still borrowed + | ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::cell::Ref<'_, std::string::String>` + | + = note: 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. error[E0597]: `y` does not live long enough --> $DIR/issue-23338-locals-die-before-temps-of-body.rs:27:9 | LL | y.borrow().clone() - | ^ borrowed value does not live long enough + | ^--------- + | | + | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... LL | }; - | - `y` dropped here while still borrowed + | -- ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::cell::Ref<'_, std::string::String>` + | | + | `y` dropped here while still borrowed + | + = note: 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. error: aborting due to 2 previous errors diff --git a/src/test/ui/span/issue-24805-dropck-child-has-items-via-parent.nll.stderr b/src/test/ui/span/issue-24805-dropck-child-has-items-via-parent.nll.stderr index b9834a3d4379e..fb995990f167b 100644 --- a/src/test/ui/span/issue-24805-dropck-child-has-items-via-parent.nll.stderr +++ b/src/test/ui/span/issue-24805-dropck-child-has-items-via-parent.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `d1` dropped here while still borrowed - | borrow later used here, when `_d` is dropped + | borrow might be used here, when `_d` is dropped and runs the `Drop` code for type `D_Child` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/issue-24805-dropck-trait-has-items.nll.stderr b/src/test/ui/span/issue-24805-dropck-trait-has-items.nll.stderr index cd3d658aa8726..5eaff66e72ea2 100644 --- a/src/test/ui/span/issue-24805-dropck-trait-has-items.nll.stderr +++ b/src/test/ui/span/issue-24805-dropck-trait-has-items.nll.stderr @@ -7,7 +7,7 @@ LL | } | - | | | `d1` dropped here while still borrowed - | borrow later used here, when `_d` is dropped + | borrow might be used here, when `_d` is dropped and runs the `Drop` code for type `D_HasSelfMethod` | = note: values in a scope are dropped in the opposite order they are defined @@ -20,7 +20,7 @@ LL | } | - | | | `d1` dropped here while still borrowed - | borrow later used here, when `_d` is dropped + | borrow might be used here, when `_d` is dropped and runs the `Drop` code for type `D_HasMethodWithSelfArg` | = note: values in a scope are dropped in the opposite order they are defined @@ -33,7 +33,7 @@ LL | } | - | | | `d1` dropped here while still borrowed - | borrow later used here, when `_d` is dropped + | borrow might be used here, when `_d` is dropped and runs the `Drop` code for type `D_HasType` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/issue-24895-copy-clone-dropck.nll.stderr b/src/test/ui/span/issue-24895-copy-clone-dropck.nll.stderr index 54c6ac43f347c..26c74b653a11b 100644 --- a/src/test/ui/span/issue-24895-copy-clone-dropck.nll.stderr +++ b/src/test/ui/span/issue-24895-copy-clone-dropck.nll.stderr @@ -7,7 +7,7 @@ LL | } | - | | | `d1` dropped here while still borrowed - | borrow later used here, when `d2` is dropped + | borrow might be used here, when `d2` is dropped and runs the `Drop` code for type `D` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/issue-25199.nll.stderr b/src/test/ui/span/issue-25199.nll.stderr index a81b591d288eb..9801f92ce6446 100644 --- a/src/test/ui/span/issue-25199.nll.stderr +++ b/src/test/ui/span/issue-25199.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `container` dropped here while still borrowed - | borrow later used here, when `container` is dropped + | borrow might be used here, when `container` is dropped and runs the destructor for type `Container<'_>` error: aborting due to previous error diff --git a/src/test/ui/span/issue-26656.nll.stderr b/src/test/ui/span/issue-26656.nll.stderr index b6c2882812088..79d9dab17769b 100644 --- a/src/test/ui/span/issue-26656.nll.stderr +++ b/src/test/ui/span/issue-26656.nll.stderr @@ -7,7 +7,7 @@ LL | } | - | | | `ticking` dropped here while still borrowed - | borrow later used here, when `zook` is dropped + | borrow might be used here, when `zook` is dropped and runs the `Drop` code for type `Zook` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/issue-29106.nll.stderr b/src/test/ui/span/issue-29106.nll.stderr index a1451866e677c..77bb311c61a40 100644 --- a/src/test/ui/span/issue-29106.nll.stderr +++ b/src/test/ui/span/issue-29106.nll.stderr @@ -7,7 +7,7 @@ LL | } | - | | | `x` dropped here while still borrowed - | borrow later used here, when `y` is dropped + | borrow might be used here, when `y` is dropped and runs the `Drop` code for type `std::sync::Arc` | = note: values in a scope are dropped in the opposite order they are defined @@ -20,7 +20,7 @@ LL | } | - | | | `x` dropped here while still borrowed - | borrow later used here, when `y` is dropped + | borrow might be used here, when `y` is dropped and runs the `Drop` code for type `std::rc::Rc` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/issue28498-reject-ex1.nll.stderr b/src/test/ui/span/issue28498-reject-ex1.nll.stderr index 0f6f6e381d801..bc3a31b843fbc 100644 --- a/src/test/ui/span/issue28498-reject-ex1.nll.stderr +++ b/src/test/ui/span/issue28498-reject-ex1.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | here, drop of `foo` needs exclusive access to `foo.data`, because the type `Foo>` implements the `Drop` trait - | borrow later used here, when `foo` is dropped + | borrow might be used here, when `foo` is dropped and runs the `Drop` code for type `Foo` | = note: consider using a `let` binding to create a longer lived value diff --git a/src/test/ui/span/issue28498-reject-lifetime-param.nll.stderr b/src/test/ui/span/issue28498-reject-lifetime-param.nll.stderr index 72acc54bffb32..21508303afc21 100644 --- a/src/test/ui/span/issue28498-reject-lifetime-param.nll.stderr +++ b/src/test/ui/span/issue28498-reject-lifetime-param.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `first_dropped` dropped here while still borrowed - | borrow later used here, when `foo1` is dropped + | borrow might be used here, when `foo1` is dropped and runs the `Drop` code for type `Foo` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/issue28498-reject-passed-to-fn.nll.stderr b/src/test/ui/span/issue28498-reject-passed-to-fn.nll.stderr index a39050a519911..752f96988db0e 100644 --- a/src/test/ui/span/issue28498-reject-passed-to-fn.nll.stderr +++ b/src/test/ui/span/issue28498-reject-passed-to-fn.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `first_dropped` dropped here while still borrowed - | borrow later used here, when `foo1` is dropped + | borrow might be used here, when `foo1` is dropped and runs the `Drop` code for type `Foo` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/issue28498-reject-trait-bound.nll.stderr b/src/test/ui/span/issue28498-reject-trait-bound.nll.stderr index fcffea25702b9..7bf55c8e9973d 100644 --- a/src/test/ui/span/issue28498-reject-trait-bound.nll.stderr +++ b/src/test/ui/span/issue28498-reject-trait-bound.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `first_dropped` dropped here while still borrowed - | borrow later used here, when `foo1` is dropped + | borrow might be used here, when `foo1` is dropped and runs the `Drop` code for type `Foo` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/regions-close-over-borrowed-ref-in-obj.nll.stderr b/src/test/ui/span/regions-close-over-borrowed-ref-in-obj.nll.stderr index 6f7fbdcf42145..fd789e79e9133 100644 --- a/src/test/ui/span/regions-close-over-borrowed-ref-in-obj.nll.stderr +++ b/src/test/ui/span/regions-close-over-borrowed-ref-in-obj.nll.stderr @@ -7,7 +7,7 @@ LL | let ss: &isize = &id(1); LL | } | - temporary value is freed at the end of this statement LL | } - | - borrow later used here, when `blah` is dropped + | - borrow might be used here, when `blah` is dropped and runs the destructor for type `std::boxed::Box` | = note: consider using a `let` binding to create a longer lived value diff --git a/src/test/ui/span/send-is-not-static-std-sync.nll.stderr b/src/test/ui/span/send-is-not-static-std-sync.nll.stderr index 878ae36386371..c17f502b77902 100644 --- a/src/test/ui/span/send-is-not-static-std-sync.nll.stderr +++ b/src/test/ui/span/send-is-not-static-std-sync.nll.stderr @@ -62,7 +62,7 @@ LL | } | - `z` dropped here while still borrowed ... LL | } - | - borrow later used here, when `tx` is dropped + | - borrow might be used here, when `tx` is dropped and runs the `Drop` code for type `std::sync::mpsc::Sender` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/span/vec-must-not-hide-type-from-dropck.nll.stderr b/src/test/ui/span/vec-must-not-hide-type-from-dropck.nll.stderr index 292c007f51225..7ad7a1b0bcdb7 100644 --- a/src/test/ui/span/vec-must-not-hide-type-from-dropck.nll.stderr +++ b/src/test/ui/span/vec-must-not-hide-type-from-dropck.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `c2` dropped here while still borrowed - | borrow later used here, when `c1` is dropped + | borrow might be used here, when `c1` is dropped and runs the destructor for type `C<'_>` | = note: values in a scope are dropped in the opposite order they are defined @@ -22,7 +22,7 @@ LL | } | - | | | `c1` dropped here while still borrowed - | borrow later used here, when `c1` is dropped + | borrow might be used here, when `c1` is dropped and runs the destructor for type `C<'_>` error: aborting due to 2 previous errors diff --git a/src/test/ui/span/vec_refs_data_with_early_death.nll.stderr b/src/test/ui/span/vec_refs_data_with_early_death.nll.stderr index 60905367063a4..562544c79165d 100644 --- a/src/test/ui/span/vec_refs_data_with_early_death.nll.stderr +++ b/src/test/ui/span/vec_refs_data_with_early_death.nll.stderr @@ -8,7 +8,7 @@ LL | } | - | | | `x` dropped here while still borrowed - | borrow later used here, when `v` is dropped + | borrow might be used here, when `v` is dropped and runs the `Drop` code for type `Bag` | = note: values in a scope are dropped in the opposite order they are defined @@ -22,7 +22,7 @@ LL | } | - | | | `y` dropped here while still borrowed - | borrow later used here, when `v` is dropped + | borrow might be used here, when `v` is dropped and runs the `Drop` code for type `Bag` | = note: values in a scope are dropped in the opposite order they are defined diff --git a/src/test/ui/unboxed-closures/unboxed-closures-failed-recursive-fn-1.nll.stderr b/src/test/ui/unboxed-closures/unboxed-closures-failed-recursive-fn-1.nll.stderr index afd90237d16ae..ad01f84cd2c14 100644 --- a/src/test/ui/unboxed-closures/unboxed-closures-failed-recursive-fn-1.nll.stderr +++ b/src/test/ui/unboxed-closures/unboxed-closures-failed-recursive-fn-1.nll.stderr @@ -10,7 +10,7 @@ LL | } | - | | | `factorial` dropped here while still borrowed - | borrow later used here, when `factorial` is dropped + | borrow might be used here, when `factorial` is dropped and runs the destructor for type `std::option::Option u32>>` error[E0506]: cannot assign to `factorial` because it is borrowed --> $DIR/unboxed-closures-failed-recursive-fn-1.rs:30:5 From 056cfffd1ba69e2e80d87c728d8475944dfbe050 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 3 Oct 2018 17:47:23 +0200 Subject: [PATCH 09/10] Unit tests for issue #54556. Some were also taken from issues #21114, #46413. --- src/test/ui/nll/issue-21114-ebfull.rs | 20 ++++ src/test/ui/nll/issue-21114-kixunil.rs | 19 +++ .../ui/nll/issue-54556-niconii.nll.stderr | 20 ++++ src/test/ui/nll/issue-54556-niconii.rs | 31 +++++ src/test/ui/nll/issue-54556-niconii.stderr | 14 +++ .../ui/nll/issue-54556-stephaneyfx.nll.stderr | 19 +++ src/test/ui/nll/issue-54556-stephaneyfx.rs | 35 ++++++ .../ui/nll/issue-54556-stephaneyfx.stderr | 14 +++ ...-54556-temps-in-tail-diagnostic.nll.stderr | 19 +++ .../issue-54556-temps-in-tail-diagnostic.rs | 23 ++++ ...ssue-54556-temps-in-tail-diagnostic.stderr | 14 +++ ...ssue-54556-used-vs-unused-tails.nll.stderr | 113 ++++++++++++++++++ .../nll/issue-54556-used-vs-unused-tails.rs | 48 ++++++++ .../issue-54556-used-vs-unused-tails.stderr | 86 +++++++++++++ .../ui/nll/issue-54556-wrap-it-up.nll.stderr | 14 +++ src/test/ui/nll/issue-54556-wrap-it-up.rs | 28 +++++ src/test/ui/nll/issue-54556-wrap-it-up.stderr | 12 ++ 17 files changed, 529 insertions(+) create mode 100644 src/test/ui/nll/issue-21114-ebfull.rs create mode 100644 src/test/ui/nll/issue-21114-kixunil.rs create mode 100644 src/test/ui/nll/issue-54556-niconii.nll.stderr create mode 100644 src/test/ui/nll/issue-54556-niconii.rs create mode 100644 src/test/ui/nll/issue-54556-niconii.stderr create mode 100644 src/test/ui/nll/issue-54556-stephaneyfx.nll.stderr create mode 100644 src/test/ui/nll/issue-54556-stephaneyfx.rs create mode 100644 src/test/ui/nll/issue-54556-stephaneyfx.stderr create mode 100644 src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.nll.stderr create mode 100644 src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.rs create mode 100644 src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.stderr create mode 100644 src/test/ui/nll/issue-54556-used-vs-unused-tails.nll.stderr create mode 100644 src/test/ui/nll/issue-54556-used-vs-unused-tails.rs create mode 100644 src/test/ui/nll/issue-54556-used-vs-unused-tails.stderr create mode 100644 src/test/ui/nll/issue-54556-wrap-it-up.nll.stderr create mode 100644 src/test/ui/nll/issue-54556-wrap-it-up.rs create mode 100644 src/test/ui/nll/issue-54556-wrap-it-up.stderr diff --git a/src/test/ui/nll/issue-21114-ebfull.rs b/src/test/ui/nll/issue-21114-ebfull.rs new file mode 100644 index 0000000000000..f5738968746ee --- /dev/null +++ b/src/test/ui/nll/issue-21114-ebfull.rs @@ -0,0 +1,20 @@ +// (this works, but only in NLL) +// compile-pass +#![feature(nll)] + +use std::collections::HashMap; +use std::sync::Mutex; + +fn i_used_to_be_able_to(foo: &Mutex>) -> Vec<(usize, usize)> { + let mut foo = foo.lock().unwrap(); + + foo.drain().collect() +} + +fn but_after_nightly_update_now_i_gotta(foo: &Mutex>) -> Vec<(usize, usize)> { + let mut foo = foo.lock().unwrap(); + + return foo.drain().collect(); +} + +fn main() {} diff --git a/src/test/ui/nll/issue-21114-kixunil.rs b/src/test/ui/nll/issue-21114-kixunil.rs new file mode 100644 index 0000000000000..2add951b70bc0 --- /dev/null +++ b/src/test/ui/nll/issue-21114-kixunil.rs @@ -0,0 +1,19 @@ +// (this works, but only in NLL) +// compile-pass +#![feature(nll)] + +fn from_stdin(min: u64) -> Vec { + use std::io::BufRead; + + let stdin = std::io::stdin(); + let stdin = stdin.lock(); + + stdin.lines() + .map(Result::unwrap) + .map(|val| val.parse()) + .map(Result::unwrap) + .filter(|val| *val >= min) + .collect() +} + +fn main() {} diff --git a/src/test/ui/nll/issue-54556-niconii.nll.stderr b/src/test/ui/nll/issue-54556-niconii.nll.stderr new file mode 100644 index 0000000000000..40cd04de5ecc1 --- /dev/null +++ b/src/test/ui/nll/issue-54556-niconii.nll.stderr @@ -0,0 +1,20 @@ +error[E0597]: `counter` does not live long enough + --> $DIR/issue-54556-niconii.rs:22:20 + | +LL | if let Ok(_) = counter.lock() { } + | ^^^^^^^------- + | | + | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... +... +LL | } + | - + | | + | `counter` dropped here while still borrowed + | ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::result::Result, ()>` + | + = note: 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. + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-54556-niconii.rs b/src/test/ui/nll/issue-54556-niconii.rs new file mode 100644 index 0000000000000..49b063f44f427 --- /dev/null +++ b/src/test/ui/nll/issue-54556-niconii.rs @@ -0,0 +1,31 @@ +// This is a reduction of a concrete test illustrating a case that was +// annoying to Rust developer niconii (see comment thread on #21114). +// +// With resolving issue #54556, pnkfelix hopes that the new diagnostic +// output produced by NLL helps to *explain* the semantic significance +// of temp drop order, and thus why inserting a semi-colon after the +// `if let` expression in `main` works. + +struct Mutex; +struct MutexGuard<'a>(&'a Mutex); + +impl Drop for Mutex { fn drop(&mut self) { println!("Mutex::drop"); } } +impl<'a> Drop for MutexGuard<'a> { fn drop(&mut self) { println!("MutexGuard::drop"); } } + +impl Mutex { + fn lock(&self) -> Result { Ok(MutexGuard(self)) } +} + +fn main() { + let counter = Mutex; + + if let Ok(_) = counter.lock() { } + + // With this code as written, the dynamic semantics here implies + // that `Mutex::drop` for `counter` runs *before* + // `MutexGuard::drop`, which would be unsound since `MutexGuard` + // still has a reference to `counter`. + // + // The goal of #54556 is to explain that within a compiler + // diagnostic. +} diff --git a/src/test/ui/nll/issue-54556-niconii.stderr b/src/test/ui/nll/issue-54556-niconii.stderr new file mode 100644 index 0000000000000..2d0de26ab309a --- /dev/null +++ b/src/test/ui/nll/issue-54556-niconii.stderr @@ -0,0 +1,14 @@ +error[E0597]: `counter` does not live long enough + --> $DIR/issue-54556-niconii.rs:22:20 + | +LL | if let Ok(_) = counter.lock() { } + | ^^^^^^^ borrowed value does not live long enough +... +LL | } + | - `counter` dropped here while still borrowed + | + = note: values in a scope are dropped in the opposite order they are created + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-54556-stephaneyfx.nll.stderr b/src/test/ui/nll/issue-54556-stephaneyfx.nll.stderr new file mode 100644 index 0000000000000..0bf76485eef2a --- /dev/null +++ b/src/test/ui/nll/issue-54556-stephaneyfx.nll.stderr @@ -0,0 +1,19 @@ +error[E0597]: `stmt` does not live long enough + --> $DIR/issue-54556-stephaneyfx.rs:27:21 + | +LL | let rows = Rows(&stmt); + | ^^^^^ borrowed value does not live long enough +LL | rows.map(|row| row).next() + | ------------------- a temporary with access to the borrow is created here ... +... +LL | } + | - + | | + | `stmt` dropped here while still borrowed + | ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `std::iter::Map, [closure@$DIR/issue-54556-stephaneyfx.rs:28:14: 28:23]>` + | + = note: 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. + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-54556-stephaneyfx.rs b/src/test/ui/nll/issue-54556-stephaneyfx.rs new file mode 100644 index 0000000000000..10a4e21497c73 --- /dev/null +++ b/src/test/ui/nll/issue-54556-stephaneyfx.rs @@ -0,0 +1,35 @@ +// This is a reduction of a concrete test illustrating a case that was +// annoying to Rust developer stephaneyfx (see issue #46413). +// +// With resolving issue #54556, pnkfelix hopes that the new diagnostic +// output produced by NLL helps to *explain* the semantic significance +// of temp drop order, and thus why storing the result in `x` and then +// returning `x` works. + +pub struct Statement; + +pub struct Rows<'stmt>(&'stmt Statement); + +impl<'stmt> Drop for Rows<'stmt> { + fn drop(&mut self) {} +} + +impl<'stmt> Iterator for Rows<'stmt> { + type Item = String; + + fn next(&mut self) -> Option { + None + } +} + +fn get_names() -> Option { + let stmt = Statement; + let rows = Rows(&stmt); + rows.map(|row| row).next() + // let x = rows.map(|row| row).next(); + // x + // + // Removing the map works too as does removing the Drop impl. +} + +fn main() {} diff --git a/src/test/ui/nll/issue-54556-stephaneyfx.stderr b/src/test/ui/nll/issue-54556-stephaneyfx.stderr new file mode 100644 index 0000000000000..4e581a516b2d3 --- /dev/null +++ b/src/test/ui/nll/issue-54556-stephaneyfx.stderr @@ -0,0 +1,14 @@ +error[E0597]: `stmt` does not live long enough + --> $DIR/issue-54556-stephaneyfx.rs:27:22 + | +LL | let rows = Rows(&stmt); + | ^^^^ borrowed value does not live long enough +... +LL | } + | - `stmt` dropped here while still borrowed + | + = note: values in a scope are dropped in the opposite order they are created + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.nll.stderr b/src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.nll.stderr new file mode 100644 index 0000000000000..513dca7950af9 --- /dev/null +++ b/src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.nll.stderr @@ -0,0 +1,19 @@ +error[E0597]: `_thing1` does not live long enough + --> $DIR/issue-54556-temps-in-tail-diagnostic.rs:5:11 + | +LL | D(&_thing1).end() + | --^^^^^^^^- + | | | + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... +LL | } + | - `_thing1` dropped here while still borrowed +LL | +LL | ; + | - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | + = note: 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. + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.rs b/src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.rs new file mode 100644 index 0000000000000..63b04333de4be --- /dev/null +++ b/src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.rs @@ -0,0 +1,23 @@ +fn main() { + { + let mut _thing1 = D(Box::new("thing1")); + // D("other").next(&_thing1).end() + D(&_thing1).end() + } + + ; +} + +#[derive(Debug)] +struct D(T); + +impl Drop for D { + fn drop(&mut self) { + println!("dropping {:?})", self); + } +} + +impl D { + fn next(&self, _other: U) -> D { D(_other) } + fn end(&self) { } +} diff --git a/src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.stderr b/src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.stderr new file mode 100644 index 0000000000000..a74970f71182a --- /dev/null +++ b/src/test/ui/nll/issue-54556-temps-in-tail-diagnostic.stderr @@ -0,0 +1,14 @@ +error[E0597]: `_thing1` does not live long enough + --> $DIR/issue-54556-temps-in-tail-diagnostic.rs:5:12 + | +LL | D(&_thing1).end() + | ^^^^^^^ borrowed value does not live long enough +LL | } + | - `_thing1` dropped here while still borrowed +LL | +LL | ; + | - borrowed value needs to live until here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-54556-used-vs-unused-tails.nll.stderr b/src/test/ui/nll/issue-54556-used-vs-unused-tails.nll.stderr new file mode 100644 index 0000000000000..9911fc9729190 --- /dev/null +++ b/src/test/ui/nll/issue-54556-used-vs-unused-tails.nll.stderr @@ -0,0 +1,113 @@ +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:10:55 + | +LL | { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // suggest `;` + | --^^^^- - - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | | | + | | | `_t1` dropped here while still borrowed + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... + | + = note: 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. + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:12:55 + | +LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } } ; // suggest `;` + | --^^^^- - - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | | | + | | | `_t1` dropped here while still borrowed + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... + | + = note: 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. + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:14:55 + | +LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() }; } // suggest `;` + | --^^^^- -- ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | | | + | | | `_t1` dropped here while still borrowed + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... + | + = note: 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. + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:16:55 + | +LL | let _ = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // suggest `;` + | --^^^^- - - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | | | + | | | `_t1` dropped here while still borrowed + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... + | + = note: 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. + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:18:55 + | +LL | let _u = { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } ; // suggest `;` + | --^^^^- - - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | | | + | | | `_t1` dropped here while still borrowed + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... + | + = note: 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. + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:20:55 + | +LL | let _x = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // `let x = ...; x` + | --^^^^- - - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | | | + | | | `_t1` dropped here while still borrowed + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... + | + = note: 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. + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:24:55 + | +LL | _y = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // `let x = ...; x` + | --^^^^- - - ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | | | + | | | `_t1` dropped here while still borrowed + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... + | + = note: 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. + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:30:55 + | +LL | fn f_local_ref() { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } // suggest `;` + | --^^^^- - + | | | | + | | | `_t1` dropped here while still borrowed + | | | ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... + | + = note: 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. + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:32:55 + | +LL | fn f() -> String { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } // `let x = ...; x` + | --^^^^- - + | | | | + | | | `_t1` dropped here while still borrowed + | | | ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `D` + | | borrowed value does not live long enough + | a temporary with access to the borrow is created here ... + | + = note: 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. + +error: aborting due to 9 previous errors + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-54556-used-vs-unused-tails.rs b/src/test/ui/nll/issue-54556-used-vs-unused-tails.rs new file mode 100644 index 0000000000000..64e4f75724aad --- /dev/null +++ b/src/test/ui/nll/issue-54556-used-vs-unused-tails.rs @@ -0,0 +1,48 @@ +// Ths test case is exploring the space of how blocs with tail +// expressions and statements can be composed, trying to keep each +// case on one line so that we can compare them via a vertical scan +// with the human eye. + +// Each comment on the right side of the line is summarizing the +// expected suggestion from the diagnostic for issue #54556. + +fn main() { + { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // suggest `;` + + { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } } ; // suggest `;` + + { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() }; } // suggest `;` + + let _ = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // suggest `;` + + let _u = { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } ; // suggest `;` + + let _x = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // `let x = ...; x` + let _x = { let mut _t1 = D(Box::new("t1")); let x = D(&_t1).end(); x } ; // no error + + let mut _y; + _y = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // `let x = ...; x` + _y = { let mut _t1 = D(Box::new("t1")); let x = D(&_t1).end(); x } ; // no error +} + +fn f_param_ref(_t1: D>) { D(&_t1).unit() } // no error + +fn f_local_ref() { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } // suggest `;` + +fn f() -> String { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } // `let x = ...; x` + + +#[derive(Debug)] +struct D(T); + +impl Drop for D { + fn drop(&mut self) { + println!("dropping {:?})", self); + } +} + +impl D { + fn next(&self, _other: U) -> D { D(_other) } + fn end(&self) -> String { format!("End({:?})", self.0) } + fn unit(&self) { } +} diff --git a/src/test/ui/nll/issue-54556-used-vs-unused-tails.stderr b/src/test/ui/nll/issue-54556-used-vs-unused-tails.stderr new file mode 100644 index 0000000000000..c75707b2aee17 --- /dev/null +++ b/src/test/ui/nll/issue-54556-used-vs-unused-tails.stderr @@ -0,0 +1,86 @@ +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:10:56 + | +LL | { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // suggest `;` + | ^^^ - - borrowed value needs to live until here + | | | + | | `_t1` dropped here while still borrowed + | borrowed value does not live long enough + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:12:56 + | +LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } } ; // suggest `;` + | ^^^ - - borrowed value needs to live until here + | | | + | | `_t1` dropped here while still borrowed + | borrowed value does not live long enough + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:14:56 + | +LL | { { let mut _t1 = D(Box::new("t1")); D(&_t1).end() }; } // suggest `;` + | ^^^ -- borrowed value needs to live until here + | | | + | | `_t1` dropped here while still borrowed + | borrowed value does not live long enough + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:16:56 + | +LL | let _ = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // suggest `;` + | ^^^ - - borrowed value needs to live until here + | | | + | | `_t1` dropped here while still borrowed + | borrowed value does not live long enough + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:18:56 + | +LL | let _u = { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } ; // suggest `;` + | ^^^ - - borrowed value needs to live until here + | | | + | | `_t1` dropped here while still borrowed + | borrowed value does not live long enough + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:20:56 + | +LL | let _x = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // `let x = ...; x` + | ^^^ - - borrowed value needs to live until here + | | | + | | `_t1` dropped here while still borrowed + | borrowed value does not live long enough + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:24:56 + | +LL | _y = { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } ; // `let x = ...; x` + | ^^^ - - borrowed value needs to live until here + | | | + | | `_t1` dropped here while still borrowed + | borrowed value does not live long enough + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:30:56 + | +LL | fn f_local_ref() { let mut _t1 = D(Box::new("t1")); D(&_t1).unit() } // suggest `;` + | ^^^ - `_t1` dropped here while still borrowed + | | + | borrowed value does not live long enough + | + = note: values in a scope are dropped in the opposite order they are created + +error[E0597]: `_t1` does not live long enough + --> $DIR/issue-54556-used-vs-unused-tails.rs:32:56 + | +LL | fn f() -> String { let mut _t1 = D(Box::new("t1")); D(&_t1).end() } // `let x = ...; x` + | ^^^ - `_t1` dropped here while still borrowed + | | + | borrowed value does not live long enough + | + = note: values in a scope are dropped in the opposite order they are created + +error: aborting due to 9 previous errors + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-54556-wrap-it-up.nll.stderr b/src/test/ui/nll/issue-54556-wrap-it-up.nll.stderr new file mode 100644 index 0000000000000..a13e59fa48b5c --- /dev/null +++ b/src/test/ui/nll/issue-54556-wrap-it-up.nll.stderr @@ -0,0 +1,14 @@ +error[E0506]: cannot assign to `x` because it is borrowed + --> $DIR/issue-54556-wrap-it-up.rs:27:5 + | +LL | let wrap = Wrap { p: &mut x }; + | ------ borrow of `x` occurs here +... +LL | x = 1; //~ ERROR cannot assign to `x` because it is borrowed [E0506] + | ^^^^^ assignment to borrowed `x` occurs here +LL | } + | - borrow might be used here, when `foo` is dropped and runs the destructor for type `Foo<'_>` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0506`. diff --git a/src/test/ui/nll/issue-54556-wrap-it-up.rs b/src/test/ui/nll/issue-54556-wrap-it-up.rs new file mode 100644 index 0000000000000..11dbef0d86460 --- /dev/null +++ b/src/test/ui/nll/issue-54556-wrap-it-up.rs @@ -0,0 +1,28 @@ +// This is testing how the diagnostic from issue #54556 behaves when +// the destructor code is attached to a place held in a field of the +// temporary being dropped. +// +// Eventually it would be nice if the diagnostic would actually report +// that specific place and its type that implements the `Drop` trait. +// But for the short term, it is acceptable to just print out the +// whole type of the temporary. + +#![allow(warnings)] + +struct Wrap<'p> { p: &'p mut i32 } + +impl<'p> Drop for Wrap<'p> { + fn drop(&mut self) { + *self.p += 1; + } +} + +struct Foo<'p> { a: String, b: Wrap<'p> } + +fn main() { + let mut x = 0; + let wrap = Wrap { p: &mut x }; + let s = String::from("str"); + let foo = Foo { a: s, b: wrap }; + x = 1; //~ ERROR cannot assign to `x` because it is borrowed [E0506] +} diff --git a/src/test/ui/nll/issue-54556-wrap-it-up.stderr b/src/test/ui/nll/issue-54556-wrap-it-up.stderr new file mode 100644 index 0000000000000..a0c19b9638798 --- /dev/null +++ b/src/test/ui/nll/issue-54556-wrap-it-up.stderr @@ -0,0 +1,12 @@ +error[E0506]: cannot assign to `x` because it is borrowed + --> $DIR/issue-54556-wrap-it-up.rs:27:5 + | +LL | let wrap = Wrap { p: &mut x }; + | - borrow of `x` occurs here +... +LL | x = 1; //~ ERROR cannot assign to `x` because it is borrowed [E0506] + | ^^^^^ assignment to borrowed `x` occurs here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0506`. From 704877f2ad7e3424a5c0286aed8697ba498fb234 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 5 Oct 2018 12:26:29 +0200 Subject: [PATCH 10/10] Add doc comment explaining what `BlockTailInfo` is. --- src/librustc/mir/mod.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 5ea097e72433d..2ae06e9ab0de7 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -638,6 +638,14 @@ mod binding_form_impl { } } +/// `BlockTailInfo` is attached to the `LocalDecl` for temporaries +/// created during evaluation of expressions in a block tail +/// expression; that is, a block like `{ STMT_1; STMT_2; EXPR }`. +/// +/// It is used to improve diagnostics when such temporaries are +/// involved in borrow_check errors, e.g. explanations of where the +/// temporaries come from, when their destructors are run, and/or how +/// one might revise the code to satisfy the borrow checker's rules. #[derive(Clone, Debug, RustcEncodable, RustcDecodable)] pub struct BlockTailInfo { /// If `true`, then the value resulting from evaluating this tail