From 20df0e949133fc7bb45b5a630b78f24720e55fe5 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 16 Mar 2017 14:41:48 +0100 Subject: [PATCH 01/11] Add `-Z span_free_rvalues`. This is solely a hack to make comparing test output plausible; it makes closures print as [closure@node_id] instead of [closure@span-with-host-path] in debug printouts. --- src/librustc/mir/mod.rs | 6 +++++- src/librustc/session/config.rs | 2 ++ src/librustc/util/ppaux.rs | 6 +++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 80c42917196db..2b2d96351f6c9 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1224,7 +1224,11 @@ impl<'tcx> Debug for Rvalue<'tcx> { AggregateKind::Closure(def_id, _) => ty::tls::with(|tcx| { if let Some(node_id) = tcx.hir.as_local_node_id(def_id) { - let name = format!("[closure@{:?}]", tcx.hir.span(node_id)); + let name = if tcx.sess.opts.debugging_opts.span_free_formats { + format!("[closure@{:?}]", node_id) + } else { + format!("[closure@{:?}]", tcx.hir.span(node_id)) + }; let mut struct_fmt = fmt.debug_struct(&name); tcx.with_freevars(node_id, |freevars| { diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 589489b49b4fd..99435ccf929c0 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -893,6 +893,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, DB_OPTIONS, db_type_desc, dbsetters, verbose: bool = (false, parse_bool, [UNTRACKED], "in general, enable more debug printouts"), + span_free_formats: bool = (false, parse_bool, [UNTRACKED], + "when debug-printing compiler state, do not include spans"), // o/w tests have closure@path time_passes: bool = (false, parse_bool, [UNTRACKED], "measure time of each rustc pass"), count_llvm_insns: bool = (false, parse_bool, diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index 15fbeb5108fdb..39f8a1d81b882 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -789,7 +789,11 @@ impl<'tcx> fmt::Display for ty::TypeVariants<'tcx> { write!(f, "[closure")?; if let Some(node_id) = tcx.hir.as_local_node_id(did) { - write!(f, "@{:?}", tcx.hir.span(node_id))?; + if tcx.sess.opts.debugging_opts.span_free_formats { + write!(f, "@{:?}", node_id)?; + } else { + write!(f, "@{:?}", tcx.hir.span(node_id))?; + } let mut sep = " "; tcx.with_freevars(node_id, |freevars| { for (freevar, upvar_ty) in freevars.iter().zip(upvar_tys) { From 609813bbc292e32d257a2c205f783babe07a5c60 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 23 Mar 2017 11:08:08 +0100 Subject: [PATCH 02/11] `-Z identify_regions` toggles rendering of (previously hidden) unnamed regions. Unlike `-Z verbose`, it is succinct. It uniquely identifies regions when displaying them, and distinguishes code extents from user-specified lifetimes in the output by leveraging a syntactic restriction: you cannot write a lifetime that starts with a numeric character. For example, it prints 'ce for the more verbose `ReScope(CodeExtent())`. --- src/librustc/mir/mod.rs | 14 ++++++++++++-- src/librustc/session/config.rs | 2 ++ src/librustc/util/ppaux.rs | 23 +++++++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 2b2d96351f6c9..e212f1c100694 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1176,12 +1176,22 @@ impl<'tcx> Debug for Rvalue<'tcx> { UnaryOp(ref op, ref a) => write!(fmt, "{:?}({:?})", op, a), Discriminant(ref lval) => write!(fmt, "discriminant({:?})", lval), NullaryOp(ref op, ref t) => write!(fmt, "{:?}({:?})", op, t), - Ref(_, borrow_kind, ref lv) => { + Ref(region, borrow_kind, ref lv) => { let kind_str = match borrow_kind { BorrowKind::Shared => "", BorrowKind::Mut | BorrowKind::Unique => "mut ", }; - write!(fmt, "&{}{:?}", kind_str, lv) + + // When identifying regions, add trailing space if + // necessary. + let region = if ppaux::identify_regions() { + let mut region = format!("{}", region); + if region.len() > 0 { region.push(' '); } + region + } else { + "".to_owned() + }; + write!(fmt, "&{}{}{:?}", region, kind_str, lv) } Aggregate(ref kind, ref lvs) => { diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 99435ccf929c0..283b18f967ff3 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -895,6 +895,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "in general, enable more debug printouts"), span_free_formats: bool = (false, parse_bool, [UNTRACKED], "when debug-printing compiler state, do not include spans"), // o/w tests have closure@path + identify_regions: bool = (false, parse_bool, [UNTRACKED], + "make unnamed regions display as '# (where # is some non-ident unique id)"), time_passes: bool = (false, parse_bool, [UNTRACKED], "measure time of each rustc pass"), count_llvm_insns: bool = (false, parse_bool, diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index 39f8a1d81b882..1fa6357719666 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -8,8 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use hir::BodyId; use hir::def_id::DefId; use hir::map::definitions::DefPathData; +use middle::region::{CodeExtent, BlockRemainder}; use ty::subst::{self, Subst}; use ty::{BrAnon, BrEnv, BrFresh, BrNamed}; use ty::{TyBool, TyChar, TyAdt}; @@ -32,6 +34,10 @@ pub fn verbose() -> bool { ty::tls::with(|tcx| tcx.sess.verbose()) } +pub fn identify_regions() -> bool { + ty::tls::with(|tcx| tcx.sess.opts.debugging_opts.identify_regions) +} + fn fn_sig(f: &mut fmt::Formatter, inputs: &[Ty], variadic: bool, @@ -519,6 +525,23 @@ impl fmt::Display for ty::RegionKind { ty::ReSkolemized(_, br) => { write!(f, "{}", br) } + ty::ReScope(code_extent) if identify_regions() => { + match code_extent { + CodeExtent::Misc(node_id) => + write!(f, "'{}mce", node_id.as_u32()), + CodeExtent::CallSiteScope(BodyId { node_id }) => + write!(f, "'{}cce", node_id.as_u32()), + CodeExtent::ParameterScope(BodyId { node_id }) => + write!(f, "'{}pce", node_id.as_u32()), + CodeExtent::DestructionScope(node_id) => + write!(f, "'{}dce", node_id.as_u32()), + CodeExtent::Remainder(BlockRemainder { block, first_statement_index }) => + write!(f, "'{}_{}rce", block, first_statement_index), + } + } + ty::ReVar(region_vid) if identify_regions() => { + write!(f, "'{}rv", region_vid.index) + } ty::ReScope(_) | ty::ReVar(_) | ty::ReErased => Ok(()), From cbed41a174aad44e069bec09bf1e502591c132ae Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 23 May 2017 13:18:20 +0200 Subject: [PATCH 03/11] Add destruction extents around blocks and statements in HAIR. --- src/librustc_mir/build/block.rs | 52 ++++++++++++++++++------------- src/librustc_mir/build/scope.rs | 17 ++++++++++ src/librustc_mir/hair/cx/block.rs | 11 +++++++ src/librustc_mir/hair/mod.rs | 2 ++ 4 files changed, 60 insertions(+), 22 deletions(-) diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index d81de954dbf15..1933ef29b539b 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -21,21 +21,24 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ast_block: &'tcx hir::Block, source_info: SourceInfo) -> BlockAnd<()> { - let Block { extent, span, stmts, expr, targeted_by_break } = self.hir.mirror(ast_block); - self.in_scope(extent, block, move |this| { - if targeted_by_break { - // This is a `break`-able block (currently only `catch { ... }`) - let exit_block = this.cfg.start_new_block(); - let block_exit = this.in_breakable_scope(None, exit_block, - destination.clone(), |this| { + let Block { extent, opt_destruction_extent, span, stmts, expr, targeted_by_break } = + self.hir.mirror(ast_block); + self.in_opt_scope(opt_destruction_extent, block, move |this| { + this.in_scope(extent, block, move |this| { + if targeted_by_break { + // This is a `break`-able block (currently only `catch { ... }`) + let exit_block = this.cfg.start_new_block(); + let block_exit = this.in_breakable_scope( + None, exit_block, destination.clone(), |this| { + this.ast_block_stmts(destination, block, span, stmts, expr) + }); + this.cfg.terminate(unpack!(block_exit), source_info, + TerminatorKind::Goto { target: exit_block }); + exit_block.unit() + } else { this.ast_block_stmts(destination, block, span, stmts, expr) - }); - this.cfg.terminate(unpack!(block_exit), source_info, - TerminatorKind::Goto { target: exit_block }); - exit_block.unit() - } else { - this.ast_block_stmts(destination, block, span, stmts, expr) - } + } + }) }) } @@ -67,12 +70,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let mut let_extent_stack = Vec::with_capacity(8); let outer_visibility_scope = this.visibility_scope; for stmt in stmts { - let Stmt { span: _, kind } = this.hir.mirror(stmt); + let Stmt { span: _, kind, opt_destruction_extent } = this.hir.mirror(stmt); match kind { StmtKind::Expr { scope, expr } => { - unpack!(block = this.in_scope(scope, block, |this| { - let expr = this.hir.mirror(expr); - this.stmt_expr(block, expr) + unpack!(block = this.in_opt_scope(opt_destruction_extent, block, |this| { + this.in_scope(scope, block, |this| { + let expr = this.hir.mirror(expr); + this.stmt_expr(block, expr) + }) })); } StmtKind::Let { remainder_scope, init_scope, pattern, initializer } => { @@ -89,10 +94,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Evaluate the initializer, if present. if let Some(init) = initializer { - unpack!(block = this.in_scope(init_scope, block, move |this| { - // FIXME #30046 ^~~~ - this.expr_into_pattern(block, pattern, init) - })); + unpack!(block = this.in_opt_scope( + opt_destruction_extent, block, move |this| { + this.in_scope(init_scope, block, move |this| { + // FIXME #30046 ^~~~ + this.expr_into_pattern(block, pattern, init) + }) + })); } else { this.visit_bindings(&pattern, &mut |this, _, _, node, span, _| { this.storage_live_binding(block, node, span); diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index a99e7b4be5768..d9c303369ccd2 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -269,6 +269,23 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { res } + pub fn in_opt_scope(&mut self, + opt_extent: Option, + mut block: BasicBlock, + f: F) + -> BlockAnd + where F: FnOnce(&mut Builder<'a, 'gcx, 'tcx>) -> BlockAnd + { + debug!("in_opt_scope(opt_extent={:?}, block={:?})", opt_extent, block); + if let Some(extent) = opt_extent { self.push_scope(extent); } + let rv = unpack!(block = f(self)); + if let Some(extent) = opt_extent { + unpack!(block = self.pop_scope(extent, block)); + } + debug!("in_scope: exiting opt_extent={:?} block={:?}", opt_extent, block); + block.and(rv) + } + /// Convenience wrapper that pushes a scope and then executes `f` /// to build its contents, popping the scope afterwards. pub fn in_scope(&mut self, diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index 47c50b78f3acc..fad070ca8d8f9 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -22,9 +22,14 @@ impl<'tcx> Mirror<'tcx> for &'tcx hir::Block { // We have to eagerly translate the "spine" of the statements // in order to get the lexical scoping correctly. let stmts = mirror_stmts(cx, self.id, &*self.stmts); + let opt_def_id = cx.tcx.hir.opt_local_def_id(self.id); + let opt_destruction_extent = opt_def_id.and_then(|def_id| { + cx.tcx.region_maps(def_id).opt_destruction_extent(self.id) + }); Block { targeted_by_break: self.targeted_by_break, extent: CodeExtent::Misc(self.id), + opt_destruction_extent: opt_destruction_extent, span: self.span, stmts: stmts, expr: self.expr.to_ref(), @@ -37,7 +42,11 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, stmts: &'tcx [hir::Stmt]) -> Vec> { let mut result = vec![]; + let opt_def_id = cx.tcx.hir.opt_local_def_id(block_id); for (index, stmt) in stmts.iter().enumerate() { + let opt_dxn_ext = opt_def_id.and_then(|def_id| { + cx.tcx.region_maps(def_id).opt_destruction_extent(stmt.node.id()) + }); match stmt.node { hir::StmtExpr(ref expr, id) | hir::StmtSemi(ref expr, id) => { @@ -47,6 +56,7 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, scope: CodeExtent::Misc(id), expr: expr.to_ref(), }, + opt_destruction_extent: opt_dxn_ext, }))) } hir::StmtDecl(ref decl, id) => { @@ -69,6 +79,7 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, pattern: pattern, initializer: local.init.to_ref(), }, + opt_destruction_extent: opt_dxn_ext, }))); } } diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 044096699b1a4..bb11cce748751 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -33,6 +33,7 @@ pub use rustc_const_eval::pattern::{BindingMode, Pattern, PatternKind, FieldPatt pub struct Block<'tcx> { pub targeted_by_break: bool, pub extent: CodeExtent, + pub opt_destruction_extent: Option, pub span: Span, pub stmts: Vec>, pub expr: Option>, @@ -47,6 +48,7 @@ pub enum StmtRef<'tcx> { pub struct Stmt<'tcx> { pub span: Span, pub kind: StmtKind<'tcx>, + pub opt_destruction_extent: Option, } #[derive(Clone, Debug)] From a658bb21e4b8742f227b07bfc3c26a0a34381285 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 26 May 2017 18:21:03 +0200 Subject: [PATCH 04/11] Paired source_info with extent; thread both through to pts where EndRegion will need construction. --- src/librustc_mir/build/block.rs | 26 ++++++++++++----------- src/librustc_mir/build/expr/as_lvalue.rs | 2 +- src/librustc_mir/build/expr/as_operand.rs | 2 ++ src/librustc_mir/build/expr/as_rvalue.rs | 3 ++- src/librustc_mir/build/expr/as_temp.rs | 5 +++-- src/librustc_mir/build/expr/into.rs | 1 + src/librustc_mir/build/expr/stmt.rs | 8 +++---- src/librustc_mir/build/mod.rs | 5 +++-- src/librustc_mir/build/scope.rs | 20 ++++++++--------- 9 files changed, 40 insertions(+), 32 deletions(-) diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 1933ef29b539b..f3f366bb79238 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -23,8 +23,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { -> BlockAnd<()> { let Block { extent, opt_destruction_extent, span, stmts, expr, targeted_by_break } = self.hir.mirror(ast_block); - self.in_opt_scope(opt_destruction_extent, block, move |this| { - this.in_scope(extent, block, move |this| { + self.in_opt_scope(opt_destruction_extent.map(|de|(de, source_info)), block, move |this| { + this.in_scope((extent, source_info), block, move |this| { if targeted_by_break { // This is a `break`-able block (currently only `catch { ... }`) let exit_block = this.cfg.start_new_block(); @@ -69,16 +69,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // First we build all the statements in the block. let mut let_extent_stack = Vec::with_capacity(8); let outer_visibility_scope = this.visibility_scope; + let source_info = this.source_info(span); for stmt in stmts { let Stmt { span: _, kind, opt_destruction_extent } = this.hir.mirror(stmt); match kind { StmtKind::Expr { scope, expr } => { - unpack!(block = this.in_opt_scope(opt_destruction_extent, block, |this| { - this.in_scope(scope, block, |this| { - let expr = this.hir.mirror(expr); - this.stmt_expr(block, expr) - }) - })); + unpack!(block = this.in_opt_scope( + opt_destruction_extent.map(|de|(de, source_info)), block, |this| { + this.in_scope((scope, source_info), block, |this| { + let expr = this.hir.mirror(expr); + this.stmt_expr(block, expr) + }) + })); } StmtKind::Let { remainder_scope, init_scope, pattern, initializer } => { let tcx = this.hir.tcx(); @@ -95,9 +97,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Evaluate the initializer, if present. if let Some(init) = initializer { unpack!(block = this.in_opt_scope( - opt_destruction_extent, block, move |this| { - this.in_scope(init_scope, block, move |this| { - // FIXME #30046 ^~~~ + opt_destruction_extent.map(|de|(de, source_info)), block, move |this| { + this.in_scope((init_scope, source_info), block, move |this| { + // FIXME #30046 ^~~~ this.expr_into_pattern(block, pattern, init) }) })); @@ -126,7 +128,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Finally, we pop all the let scopes before exiting out from the scope of block // itself. for extent in let_extent_stack.into_iter().rev() { - unpack!(block = this.pop_scope(extent, block)); + unpack!(block = this.pop_scope((extent, source_info), block)); } // Restore the original visibility scope. this.visibility_scope = outer_visibility_scope; diff --git a/src/librustc_mir/build/expr/as_lvalue.rs b/src/librustc_mir/build/expr/as_lvalue.rs index df2841a668269..04c23215463dd 100644 --- a/src/librustc_mir/build/expr/as_lvalue.rs +++ b/src/librustc_mir/build/expr/as_lvalue.rs @@ -40,7 +40,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let source_info = this.source_info(expr_span); match expr.kind { ExprKind::Scope { extent, value } => { - this.in_scope(extent, block, |this| this.as_lvalue(block, value)) + this.in_scope((extent, source_info), block, |this| this.as_lvalue(block, value)) } ExprKind::Field { lhs, name } => { let lvalue = unpack!(block = this.as_lvalue(block, lhs)); diff --git a/src/librustc_mir/build/expr/as_operand.rs b/src/librustc_mir/build/expr/as_operand.rs index 5178963179d6f..4679e0bb0a5c3 100644 --- a/src/librustc_mir/build/expr/as_operand.rs +++ b/src/librustc_mir/build/expr/as_operand.rs @@ -56,6 +56,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let this = self; if let ExprKind::Scope { extent, value } = expr.kind { + let source_info = this.source_info(expr.span); + let extent = (extent, source_info); return this.in_scope(extent, block, |this| { this.as_operand(block, scope, value) }); diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index 2884b60fdd8a9..2512291f1a44f 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -59,6 +59,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { match expr.kind { ExprKind::Scope { extent, value } => { + let extent = (extent, source_info); this.in_scope(extent, block, |this| this.as_rvalue(block, scope, value)) } ExprKind::Repeat { value, count } => { @@ -99,7 +100,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // to start, malloc some memory of suitable type (thus far, uninitialized): let box_ = Rvalue::NullaryOp(NullOp::Box, value.ty); this.cfg.push_assign(block, source_info, &result, box_); - this.in_scope(value_extents, block, |this| { + this.in_scope((value_extents, source_info), block, |this| { // schedule a shallow free of that memory, lest we unwind: this.schedule_box_free(expr_span, value_extents, &result, value.ty); // initialize the box contents: diff --git a/src/librustc_mir/build/expr/as_temp.rs b/src/librustc_mir/build/expr/as_temp.rs index 17d74571ce482..faaa46a4a8fd9 100644 --- a/src/librustc_mir/build/expr/as_temp.rs +++ b/src/librustc_mir/build/expr/as_temp.rs @@ -39,14 +39,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { block, temp_lifetime, expr); let this = self; + let expr_span = expr.span; + let source_info = this.source_info(expr_span); if let ExprKind::Scope { extent, value } = expr.kind { - return this.in_scope(extent, block, |this| { + return this.in_scope((extent, source_info), block, |this| { this.as_temp(block, temp_lifetime, value) }); } let expr_ty = expr.ty.clone(); - let expr_span = expr.span; let temp = this.temp(expr_ty.clone(), expr_span); let source_info = this.source_info(expr_span); diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index d456bc3ded390..97d63ac6d1901 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -39,6 +39,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { match expr.kind { ExprKind::Scope { extent, value } => { + let extent = (extent, source_info); this.in_scope(extent, block, |this| this.into(destination, block, value)) } ExprKind::Block { body: ast_block } => { diff --git a/src/librustc_mir/build/expr/stmt.rs b/src/librustc_mir/build/expr/stmt.rs index 3c7ab373651d2..3120ac2190824 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -24,7 +24,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { match expr.kind { ExprKind::Scope { extent, value } => { let value = this.hir.mirror(value); - this.in_scope(extent, block, |this| this.stmt_expr(block, value)) + this.in_scope((extent, source_info), block, |this| this.stmt_expr(block, value)) } ExprKind::Assign { lhs, rhs } => { let lhs = this.hir.mirror(lhs); @@ -81,7 +81,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { *this.find_breakable_scope(expr_span, label); let continue_block = continue_block.expect( "Attempted to continue in non-continuable breakable block"); - this.exit_scope(expr_span, extent, block, continue_block); + this.exit_scope(expr_span, (extent, source_info), block, continue_block); this.cfg.start_new_block().unit() } ExprKind::Break { label, value } => { @@ -99,7 +99,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } else { this.cfg.push_assign_unit(block, source_info, &destination) } - this.exit_scope(expr_span, extent, block, break_block); + this.exit_scope(expr_span, (extent, source_info), block, break_block); this.cfg.start_new_block().unit() } ExprKind::Return { value } => { @@ -116,7 +116,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }; let extent = this.extent_of_return_scope(); let return_block = this.return_block(); - this.exit_scope(expr_span, extent, block, return_block); + this.exit_scope(expr_span, (extent, source_info), block, return_block); this.cfg.start_new_block().unit() } ExprKind::InlineAsm { asm, outputs, inputs } => { diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 56c0e18d6f91a..eb1414d42e179 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -339,8 +339,9 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>, let call_site_extent = CodeExtent::CallSiteScope(body.id()); let arg_extent = CodeExtent::ParameterScope(body.id()); let mut block = START_BLOCK; - unpack!(block = builder.in_scope(call_site_extent, block, |builder| { - unpack!(block = builder.in_scope(arg_extent, block, |builder| { + let source_info = builder.source_info(span); + unpack!(block = builder.in_scope((call_site_extent, source_info), block, |builder| { + unpack!(block = builder.in_scope((arg_extent, source_info), block, |builder| { builder.args_and_body(block, &arguments, arg_extent, &body.value) })); // Attribute epilogue to function's closing brace diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index d9c303369ccd2..84b69bbf610f5 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -270,14 +270,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } pub fn in_opt_scope(&mut self, - opt_extent: Option, + opt_extent: Option<(CodeExtent, SourceInfo)>, mut block: BasicBlock, f: F) -> BlockAnd where F: FnOnce(&mut Builder<'a, 'gcx, 'tcx>) -> BlockAnd { debug!("in_opt_scope(opt_extent={:?}, block={:?})", opt_extent, block); - if let Some(extent) = opt_extent { self.push_scope(extent); } + if let Some(extent) = opt_extent { self.push_scope(extent.0); } let rv = unpack!(block = f(self)); if let Some(extent) = opt_extent { unpack!(block = self.pop_scope(extent, block)); @@ -289,14 +289,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// Convenience wrapper that pushes a scope and then executes `f` /// to build its contents, popping the scope afterwards. pub fn in_scope(&mut self, - extent: CodeExtent, + extent: (CodeExtent, SourceInfo), mut block: BasicBlock, f: F) -> BlockAnd where F: FnOnce(&mut Builder<'a, 'gcx, 'tcx>) -> BlockAnd { debug!("in_scope(extent={:?}, block={:?})", extent, block); - self.push_scope(extent); + self.push_scope(extent.0); let rv = unpack!(block = f(self)); unpack!(block = self.pop_scope(extent, block)); debug!("in_scope: exiting extent={:?} block={:?}", extent, block); @@ -324,7 +324,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// drops onto the end of `block` that are needed. This must /// match 1-to-1 with `push_scope`. pub fn pop_scope(&mut self, - extent: CodeExtent, + extent: (CodeExtent, SourceInfo), mut block: BasicBlock) -> BlockAnd<()> { debug!("pop_scope({:?}, {:?})", extent, block); @@ -332,7 +332,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // to make sure all the `cached_block`s are filled in. self.diverge_cleanup(); let scope = self.scopes.pop().unwrap(); - assert_eq!(scope.extent, extent); + assert_eq!(scope.extent, extent.0); unpack!(block = build_scope_drops(&mut self.cfg, &scope, &self.scopes, @@ -348,11 +348,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// module comment for details. pub fn exit_scope(&mut self, span: Span, - extent: CodeExtent, + extent: (CodeExtent, SourceInfo), mut block: BasicBlock, target: BasicBlock) { debug!("exit_scope(extent={:?}, block={:?}, target={:?})", extent, block, target); - let scope_count = 1 + self.scopes.iter().rev().position(|scope| scope.extent == extent) + let scope_count = 1 + self.scopes.iter().rev().position(|scope| scope.extent == extent.0) .unwrap_or_else(||{ span_bug!(span, "extent {:?} does not enclose", extent) }); @@ -363,7 +363,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let mut rest = &mut self.scopes[(len - scope_count)..]; while let Some((scope, rest_)) = {rest}.split_last_mut() { rest = rest_; - block = if let Some(&e) = scope.cached_exits.get(&(target, extent)) { + block = if let Some(&e) = scope.cached_exits.get(&(target, extent.0)) { self.cfg.terminate(block, scope.source_info(span), TerminatorKind::Goto { target: e }); return; @@ -371,7 +371,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let b = self.cfg.start_new_block(); self.cfg.terminate(block, scope.source_info(span), TerminatorKind::Goto { target: b }); - scope.cached_exits.insert((target, extent), b); + scope.cached_exits.insert((target, extent.0), b); b }; unpack!(block = build_scope_drops(&mut self.cfg, From 7c0c4cde80ef871858d5e9dcef506157fcdcc21c Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 26 May 2017 18:54:23 +0200 Subject: [PATCH 05/11] Pass span through diverge_cleanup down to build_diverge_scope where it can be used for building the diverge path's terminator. --- src/librustc_mir/build/expr/into.rs | 2 +- src/librustc_mir/build/matches/test.rs | 2 +- src/librustc_mir/build/scope.rs | 13 +++++++------ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 97d63ac6d1901..b7abc707a380d 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -234,7 +234,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { .collect(); let success = this.cfg.start_new_block(); - let cleanup = this.diverge_cleanup(); + let cleanup = this.diverge_cleanup(expr_span); this.cfg.terminate(block, source_info, TerminatorKind::Call { func: fun, args: args, diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index 28386fa598ce6..f4d43e041ae87 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -306,7 +306,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let bool_ty = self.hir.bool_ty(); let eq_result = self.temp(bool_ty, test.span); let eq_block = self.cfg.start_new_block(); - let cleanup = self.diverge_cleanup(); + let cleanup = self.diverge_cleanup(test.span); self.cfg.terminate(block, source_info, TerminatorKind::Call { func: Operand::Constant(box Constant { span: test.span, diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 84b69bbf610f5..28828d45b2e96 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -330,7 +330,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { debug!("pop_scope({:?}, {:?})", extent, block); // We need to have `cached_block`s available for all the drops, so we call diverge_cleanup // to make sure all the `cached_block`s are filled in. - self.diverge_cleanup(); + self.diverge_cleanup(extent.1.span); let scope = self.scopes.pop().unwrap(); assert_eq!(scope.extent, extent.0); unpack!(block = build_scope_drops(&mut self.cfg, @@ -607,7 +607,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// This path terminates in Resume. Returns the start of the path. /// See module comment for more details. None indicates there’s no /// cleanup to do at this point. - pub fn diverge_cleanup(&mut self) -> Option { + pub fn diverge_cleanup(&mut self, span: Span) -> Option { if !self.scopes.iter().any(|scope| scope.needs_cleanup) { return None; } @@ -641,7 +641,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }; for scope in scopes.iter_mut().filter(|s| s.needs_cleanup) { - target = build_diverge_scope(hir.tcx(), cfg, &unit_temp, scope, target); + target = build_diverge_scope(hir.tcx(), cfg, &unit_temp, span, scope, target); } Some(target) } @@ -657,7 +657,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } let source_info = self.source_info(span); let next_target = self.cfg.start_new_block(); - let diverge_target = self.diverge_cleanup(); + let diverge_target = self.diverge_cleanup(span); self.cfg.terminate(block, source_info, TerminatorKind::Drop { location: location, @@ -675,7 +675,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { value: Operand<'tcx>) -> BlockAnd<()> { let source_info = self.source_info(span); let next_target = self.cfg.start_new_block(); - let diverge_target = self.diverge_cleanup(); + let diverge_target = self.diverge_cleanup(span); self.cfg.terminate(block, source_info, TerminatorKind::DropAndReplace { location: location, @@ -698,7 +698,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let source_info = self.source_info(span); let success_block = self.cfg.start_new_block(); - let cleanup = self.diverge_cleanup(); + let cleanup = self.diverge_cleanup(span); self.cfg.terminate(block, source_info, TerminatorKind::Assert { @@ -767,6 +767,7 @@ fn build_scope_drops<'tcx>(cfg: &mut CFG<'tcx>, fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, cfg: &mut CFG<'tcx>, unit_temp: &Lvalue<'tcx>, + span: Span, scope: &mut Scope<'tcx>, mut target: BasicBlock) -> BasicBlock From 1d315cf7da85911dfa239331fab30607ce2d1dce Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 17 Feb 2017 13:38:42 +0100 Subject: [PATCH 06/11] Add `EndRegion` statement kind to MIR. * Emit `EndRegion` for every code-extent for which we observe a borrow. To do this, we needed to thread source info back through to `fn in_scope`, which makes this commit a bit more painful than one might have expected. * There is `end_region` emission in `Builder::pop_scope` and in `Builder::exit_scope`; the first handles falling out of a scope normally, the second handles e.g. `break`. * Remove `EndRegion` statements during the erase_regions mir transformation. * Preallocate the terminator block, and throw an `Unreachable` marker on it from the outset. Then overwrite that Terminator as necessary on demand. * Instead of marking the scope as needs_cleanup after seeing a borrow, just treat every scope in the chain as being part of the diverge_block (after any *one* of them has separately signalled that it needs cleanup, e.g. due to having a destructor to run). * Allow for resume terminators to be patched when looking up drop flags. (In particular, `MirPatch::new` has an explicit code path, presumably previously unreachable, that patches up such resume terminators.) * Make `Scope` implement `Debug` trait. * Expanded a stray comment: we do not emit StorageDead on diverging paths, but that end behavior might not be desirable. --- src/librustc/ich/impls_mir.rs | 3 ++ src/librustc/mir/mod.rs | 21 ++++++++++ src/librustc/mir/visit.rs | 1 + .../borrowck/mir/dataflow/impls.rs | 1 + .../borrowck/mir/dataflow/sanity_check.rs | 1 + .../borrowck/mir/elaborate_drops.rs | 5 +++ .../borrowck/mir/gather_moves.rs | 1 + src/librustc_borrowck/borrowck/mir/mod.rs | 1 + src/librustc_mir/build/block.rs | 3 +- src/librustc_mir/build/cfg.rs | 11 +++++ src/librustc_mir/build/expr/as_temp.rs | 1 - src/librustc_mir/build/scope.rs | 40 +++++++++++++++---- src/librustc_mir/transform/erase_regions.rs | 9 +++++ src/librustc_mir/transform/qualify_consts.rs | 1 + src/librustc_mir/transform/type_check.rs | 1 + src/librustc_mir/util/patch.rs | 1 + src/librustc_passes/mir_stats.rs | 1 + src/librustc_trans/mir/constant.rs | 1 + src/librustc_trans/mir/statement.rs | 1 + 19 files changed, 94 insertions(+), 10 deletions(-) diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index ae2bea3027d44..cb017b7f8864d 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -226,6 +226,9 @@ for mir::StatementKind<'tcx> { mir::StatementKind::StorageDead(ref lvalue) => { lvalue.hash_stable(hcx, hasher); } + mir::StatementKind::EndRegion(ref extents) => { + extents.hash_stable(hcx, hasher); + } mir::StatementKind::Nop => {} mir::StatementKind::InlineAsm { ref asm, ref outputs, ref inputs } => { asm.hash_stable(hcx, hasher); diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index e212f1c100694..c8d03e7b30588 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -12,6 +12,7 @@ use graphviz::IntoCow; use middle::const_val::ConstVal; +use middle::region::CodeExtent; use rustc_const_math::{ConstUsize, ConstInt, ConstMathErr}; use rustc_data_structures::indexed_vec::{IndexVec, Idx}; use rustc_data_structures::control_flow_graph::dominators::{Dominators, dominators}; @@ -804,6 +805,10 @@ pub enum StatementKind<'tcx> { inputs: Vec> }, + /// Mark one terminating point of an extent (i.e. static region). + /// (The starting point(s) arise implicitly from borrows.) + EndRegion(CodeExtent), + /// No-op. Useful for deleting instructions without affecting statement indices. Nop, } @@ -813,6 +818,8 @@ impl<'tcx> Debug for Statement<'tcx> { use self::StatementKind::*; match self.kind { Assign(ref lv, ref rv) => write!(fmt, "{:?} = {:?}", lv, rv), + // (reuse lifetime rendering policy from ppaux.) + EndRegion(ref ce) => write!(fmt, "EndRegion({})", ty::ReScope(*ce)), StorageLive(ref lv) => write!(fmt, "StorageLive({:?})", lv), StorageDead(ref lv) => write!(fmt, "StorageDead({:?})", lv), SetDiscriminant{lvalue: ref lv, variant_index: index} => { @@ -1472,6 +1479,13 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> { outputs: outputs.fold_with(folder), inputs: inputs.fold_with(folder) }, + + // Note for future: If we want to expose the extents + // during the fold, we need to either generalize EndRegion + // to carry `[ty::Region]`, or extend the `TypeFolder` + // trait with a `fn fold_extent`. + EndRegion(ref extent) => EndRegion(extent.clone()), + Nop => Nop, }; Statement { @@ -1490,6 +1504,13 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> { StorageDead(ref lvalue) => lvalue.visit_with(visitor), InlineAsm { ref outputs, ref inputs, .. } => outputs.visit_with(visitor) || inputs.visit_with(visitor), + + // Note for future: If we want to expose the extents + // during the visit, we need to either generalize EndRegion + // to carry `[ty::Region]`, or extend the `TypeVisitor` + // trait with a `fn visit_extent`. + EndRegion(ref _extent) => false, + Nop => false, } } diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 780ce736bfd3c..ac1c0306f701c 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -325,6 +325,7 @@ macro_rules! make_mir_visitor { ref $($mutability)* rvalue) => { self.visit_assign(block, lvalue, rvalue, location); } + StatementKind::EndRegion(_) => {} StatementKind::SetDiscriminant{ ref $($mutability)* lvalue, .. } => { self.visit_lvalue(lvalue, LvalueContext::Store, location); } diff --git a/src/librustc_borrowck/borrowck/mir/dataflow/impls.rs b/src/librustc_borrowck/borrowck/mir/dataflow/impls.rs index da8aa231ccf15..1a1ac7f9c74d3 100644 --- a/src/librustc_borrowck/borrowck/mir/dataflow/impls.rs +++ b/src/librustc_borrowck/borrowck/mir/dataflow/impls.rs @@ -474,6 +474,7 @@ impl<'a, 'tcx> BitDenotation for MovingOutStatements<'a, 'tcx> { mir::StatementKind::StorageLive(_) | mir::StatementKind::StorageDead(_) | mir::StatementKind::InlineAsm { .. } | + mir::StatementKind::EndRegion(_) | mir::StatementKind::Nop => {} } } diff --git a/src/librustc_borrowck/borrowck/mir/dataflow/sanity_check.rs b/src/librustc_borrowck/borrowck/mir/dataflow/sanity_check.rs index 44e3b38ea3857..2c55460fb301b 100644 --- a/src/librustc_borrowck/borrowck/mir/dataflow/sanity_check.rs +++ b/src/librustc_borrowck/borrowck/mir/dataflow/sanity_check.rs @@ -105,6 +105,7 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>, mir::StatementKind::StorageLive(_) | mir::StatementKind::StorageDead(_) | mir::StatementKind::InlineAsm { .. } | + mir::StatementKind::EndRegion(_) | mir::StatementKind::Nop => continue, mir::StatementKind::SetDiscriminant{ .. } => span_bug!(stmt.source_info.span, diff --git a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs b/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs index b03d34819f637..7acfc2c3bbaff 100644 --- a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs +++ b/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs @@ -585,6 +585,11 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { // drop elaboration should handle that by itself continue } + TerminatorKind::Resume => { + // We can replace resumes with gotos + // jumping to a canonical resume. + continue + } TerminatorKind::DropAndReplace { .. } => { // this contains the move of the source and // the initialization of the destination. We diff --git a/src/librustc_borrowck/borrowck/mir/gather_moves.rs b/src/librustc_borrowck/borrowck/mir/gather_moves.rs index b03d2a775df71..a0ecdcc8e2ff7 100644 --- a/src/librustc_borrowck/borrowck/mir/gather_moves.rs +++ b/src/librustc_borrowck/borrowck/mir/gather_moves.rs @@ -413,6 +413,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { "SetDiscriminant should not exist during borrowck"); } StatementKind::InlineAsm { .. } | + StatementKind::EndRegion(_) | StatementKind::Nop => {} } } diff --git a/src/librustc_borrowck/borrowck/mir/mod.rs b/src/librustc_borrowck/borrowck/mir/mod.rs index 2b39d2a256e1f..e3b99b9d4bd43 100644 --- a/src/librustc_borrowck/borrowck/mir/mod.rs +++ b/src/librustc_borrowck/borrowck/mir/mod.rs @@ -394,6 +394,7 @@ fn drop_flag_effects_for_location<'a, 'tcx, F>( mir::StatementKind::StorageLive(_) | mir::StatementKind::StorageDead(_) | mir::StatementKind::InlineAsm { .. } | + mir::StatementKind::EndRegion(_) | mir::StatementKind::Nop => {} }, None => { diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index f3f366bb79238..865174aa272ec 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -71,7 +71,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let outer_visibility_scope = this.visibility_scope; let source_info = this.source_info(span); for stmt in stmts { - let Stmt { span: _, kind, opt_destruction_extent } = this.hir.mirror(stmt); + let Stmt { span, kind, opt_destruction_extent } = this.hir.mirror(stmt); match kind { StmtKind::Expr { scope, expr } => { unpack!(block = this.in_opt_scope( @@ -122,7 +122,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { if let Some(expr) = expr { unpack!(block = this.into(destination, block, expr)); } else { - let source_info = this.source_info(span); this.cfg.push_assign_unit(block, source_info, destination); } // Finally, we pop all the let scopes before exiting out from the scope of block diff --git a/src/librustc_mir/build/cfg.rs b/src/librustc_mir/build/cfg.rs index 40a78933aad2d..c20f8bde78386 100644 --- a/src/librustc_mir/build/cfg.rs +++ b/src/librustc_mir/build/cfg.rs @@ -14,6 +14,7 @@ //! Routines for manipulating the control-flow graph. use build::CFG; +use rustc::middle::region::CodeExtent; use rustc::mir::*; impl<'tcx> CFG<'tcx> { @@ -43,6 +44,16 @@ impl<'tcx> CFG<'tcx> { self.block_data_mut(block).statements.push(statement); } + pub fn push_end_region(&mut self, + block: BasicBlock, + source_info: SourceInfo, + extent: CodeExtent) { + self.push(block, Statement { + source_info: source_info, + kind: StatementKind::EndRegion(extent), + }); + } + pub fn push_assign(&mut self, block: BasicBlock, source_info: SourceInfo, diff --git a/src/librustc_mir/build/expr/as_temp.rs b/src/librustc_mir/build/expr/as_temp.rs index faaa46a4a8fd9..9be306d2848b3 100644 --- a/src/librustc_mir/build/expr/as_temp.rs +++ b/src/librustc_mir/build/expr/as_temp.rs @@ -49,7 +49,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let expr_ty = expr.ty.clone(); let temp = this.temp(expr_ty.clone(), expr_span); - let source_info = this.source_info(expr_span); if !expr_ty.is_never() && temp_lifetime.is_some() { this.cfg.push(block, Statement { diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 28828d45b2e96..469fd5750a2f9 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -94,10 +94,11 @@ use rustc::ty::subst::{Kind, Subst}; use rustc::ty::{Ty, TyCtxt}; use rustc::mir::*; use rustc::mir::transform::MirSource; -use syntax_pos::Span; +use syntax_pos::{Span}; use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::fx::FxHashMap; +#[derive(Debug)] pub struct Scope<'tcx> { /// The visibility scope this scope was created in. visibility_scope: VisibilityScope, @@ -114,7 +115,7 @@ pub struct Scope<'tcx> { /// * pollutting the cleanup MIR with StorageDead creates /// landing pads even though there's no actual destructors /// * freeing up stack space has no effect during unwinding - needs_cleanup: bool, + pub(super) needs_cleanup: bool, /// set of lvalues to drop when exiting this scope. This starts /// out empty but grows as variables are declared during the @@ -141,6 +142,7 @@ pub struct Scope<'tcx> { cached_exits: FxHashMap<(BasicBlock, CodeExtent), BasicBlock>, } +#[derive(Debug)] struct DropData<'tcx> { /// span where drop obligation was incurred (typically where lvalue was declared) span: Span, @@ -152,6 +154,7 @@ struct DropData<'tcx> { kind: DropKind } +#[derive(Debug)] enum DropKind { Value { /// The cached block for the cleanups-on-diverge path. This block @@ -163,6 +166,7 @@ enum DropKind { Storage } +#[derive(Debug)] struct FreeData<'tcx> { /// span where free obligation was incurred span: Span, @@ -338,6 +342,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { &self.scopes, block, self.arg_count)); + + self.cfg.push_end_region(block, extent.1, scope.extent); block.unit() } @@ -379,6 +385,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { rest, block, self.arg_count)); + + // End all regions for scopes out of which we are breaking. + self.cfg.push_end_region(block, extent.1, scope.extent); + if let Some(ref free_data) = scope.free { let next = self.cfg.start_new_block(); let free = build_free(self.hir.tcx(), &tmp, free_data, next); @@ -640,7 +650,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { resumeblk }; - for scope in scopes.iter_mut().filter(|s| s.needs_cleanup) { + for scope in scopes.iter_mut() { target = build_diverge_scope(hir.tcx(), cfg, &unit_temp, span, scope, target); } Some(target) @@ -775,9 +785,9 @@ fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, // Build up the drops in **reverse** order. The end result will // look like: // - // [drops[n]] -...-> [drops[0]] -> [Free] -> [target] - // | | - // +------------------------------------+ + // [EndRegion Block] -> [drops[n]] -...-> [drops[0]] -> [Free] -> [target] + // | | + // +---------------------------------------------------------+ // code for scope // // The code in this function reads from right to left. At each @@ -807,9 +817,16 @@ fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, // Next, build up the drops. Here we iterate the vector in // *forward* order, so that we generate drops[0] first (right to // left in diagram above). - for drop_data in &mut scope.drops { + for (j, drop_data) in scope.drops.iter_mut().enumerate() { + debug!("build_diverge_scope drop_data[{}]: {:?}", j, drop_data); // Only full value drops are emitted in the diverging path, // not StorageDead. + // + // Note: This may not actually be what we desire (are we + // "freeing" stack storage as we unwind, or merely observing a + // frozen stack)? In particular, the intent may have been to + // match the behavior of clang, but on inspection eddyb says + // this is not what clang does. let cached_block = match drop_data.kind { DropKind::Value { ref mut cached_block } => cached_block, DropKind::Storage => continue @@ -829,6 +846,15 @@ fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, }; } + // Finally, push the EndRegion block, used by mir-borrowck. (Block + // becomes trivial goto after pass that removes all EndRegions.) + { + let block = cfg.start_new_cleanup_block(); + cfg.push_end_region(block, source_info(span), scope.extent); + cfg.terminate(block, source_info(span), TerminatorKind::Goto { target: target }); + target = block + } + target } diff --git a/src/librustc_mir/transform/erase_regions.rs b/src/librustc_mir/transform/erase_regions.rs index fa88eca6ec3f0..e809695c18043 100644 --- a/src/librustc_mir/transform/erase_regions.rs +++ b/src/librustc_mir/transform/erase_regions.rs @@ -65,6 +65,15 @@ impl<'a, 'tcx> MutVisitor<'tcx> for EraseRegionsVisitor<'a, 'tcx> { substs: &mut ClosureSubsts<'tcx>) { *substs = self.tcx.erase_regions(substs); } + + fn visit_statement(&mut self, + _block: BasicBlock, + statement: &mut Statement<'tcx>, + _location: Location) { + if let StatementKind::EndRegion(_) = statement.kind { + statement.kind = StatementKind::Nop; + } + } } pub struct EraseRegions; diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index d60e761bc0b94..041d78d3c24b0 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -907,6 +907,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { StatementKind::StorageLive(_) | StatementKind::StorageDead(_) | StatementKind::InlineAsm {..} | + StatementKind::EndRegion(_) | StatementKind::Nop => {} } }); diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index e23f0705b6a02..efde39ad6a4c1 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -413,6 +413,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } StatementKind::InlineAsm { .. } | + StatementKind::EndRegion(_) | StatementKind::Nop => {} } } diff --git a/src/librustc_mir/util/patch.rs b/src/librustc_mir/util/patch.rs index 7898d93c22e3b..ac121131eb999 100644 --- a/src/librustc_mir/util/patch.rs +++ b/src/librustc_mir/util/patch.rs @@ -46,6 +46,7 @@ impl<'tcx> MirPatch<'tcx> { for (bb, block) in mir.basic_blocks().iter_enumerated() { if let TerminatorKind::Resume = block.terminator().kind { if block.statements.len() > 0 { + assert!(resume_stmt_block.is_none()); resume_stmt_block = Some(bb); } else { resume_block = Some(bb); diff --git a/src/librustc_passes/mir_stats.rs b/src/librustc_passes/mir_stats.rs index e29da3a649655..4dd38cc515c77 100644 --- a/src/librustc_passes/mir_stats.rs +++ b/src/librustc_passes/mir_stats.rs @@ -125,6 +125,7 @@ impl<'a, 'tcx> mir_visit::Visitor<'tcx> for StatCollector<'a, 'tcx> { self.record("Statement", statement); self.record(match statement.kind { StatementKind::Assign(..) => "StatementKind::Assign", + StatementKind::EndRegion(..) => "StatementKind::EndRegion", StatementKind::SetDiscriminant { .. } => "StatementKind::SetDiscriminant", StatementKind::StorageLive(..) => "StatementKind::StorageLive", StatementKind::StorageDead(..) => "StatementKind::StorageDead", diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index 4967ef2f7908b..16ef32ccf5777 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -284,6 +284,7 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { } mir::StatementKind::StorageLive(_) | mir::StatementKind::StorageDead(_) | + mir::StatementKind::EndRegion(_) | mir::StatementKind::Nop => {} mir::StatementKind::InlineAsm { .. } | mir::StatementKind::SetDiscriminant{ .. } => { diff --git a/src/librustc_trans/mir/statement.rs b/src/librustc_trans/mir/statement.rs index 52c2afca4748b..170a76a49497b 100644 --- a/src/librustc_trans/mir/statement.rs +++ b/src/librustc_trans/mir/statement.rs @@ -86,6 +86,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> { asm::trans_inline_asm(&bcx, asm, outputs, input_vals); bcx } + mir::StatementKind::EndRegion(_) | mir::StatementKind::Nop => bcx, } } From 0a5211e80951eeb89e6f8e9e10b31398904d8d76 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 22 May 2017 13:06:51 +0200 Subject: [PATCH 07/11] Add post-pass to remove EndRegions of unborrowed extents. --- src/librustc_driver/driver.rs | 3 + .../transform/clean_end_regions.rs | 84 +++++++++++++++++++ src/librustc_mir/transform/mod.rs | 1 + 3 files changed, 88 insertions(+) create mode 100644 src/librustc_mir/transform/clean_end_regions.rs diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index bca82ff9a46df..b1620ed53c718 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -912,6 +912,9 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, let mut passes = Passes::new(); passes.push_hook(mir::transform::dump_mir::DumpMir); + // Remove all `EndRegion` statements that are not involved in borrows. + passes.push_pass(MIR_CONST, mir::transform::clean_end_regions::CleanEndRegions); + // What we need to do constant evaluation. passes.push_pass(MIR_CONST, mir::transform::simplify::SimplifyCfg::new("initial")); passes.push_pass(MIR_CONST, mir::transform::type_check::TypeckMir); diff --git a/src/librustc_mir/transform/clean_end_regions.rs b/src/librustc_mir/transform/clean_end_regions.rs new file mode 100644 index 0000000000000..36125f9454365 --- /dev/null +++ b/src/librustc_mir/transform/clean_end_regions.rs @@ -0,0 +1,84 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! This module provides one pass, `CleanEndRegions`, that reduces the +//! set of `EndRegion` statements in the MIR. +//! +//! The "pass" is actually implemented as two traversals (aka visits) +//! of the input MIR. The first traversal, `GatherBorrowedRegions`, +//! finds all of the regions in the MIR that are involved in a borrow. +//! +//! The second traversal, `DeleteTrivialEndRegions`, walks over the +//! MIR and removes any `EndRegion` that is applied to a region that +//! was not seen in the previous pass. + +use rustc_data_structures::fx::FxHashSet; + +use rustc::middle::region::CodeExtent; +use rustc::mir::transform::{MirPass, MirSource}; +use rustc::mir::{BasicBlock, Location, Mir, Rvalue, Statement, StatementKind}; +use rustc::mir::visit::{MutVisitor, Visitor}; +use rustc::ty::{RegionKind, TyCtxt}; + +pub struct CleanEndRegions; + +struct GatherBorrowedRegions { + seen_regions: FxHashSet, +} + +struct DeleteTrivialEndRegions<'a> { + seen_regions: &'a FxHashSet, +} + +impl MirPass for CleanEndRegions { + fn run_pass<'a, 'tcx>(&self, + _tcx: TyCtxt<'a, 'tcx, 'tcx>, + _source: MirSource, + mir: &mut Mir<'tcx>) { + let mut gather = GatherBorrowedRegions { seen_regions: FxHashSet() }; + gather.visit_mir(mir); + + let mut delete = DeleteTrivialEndRegions { seen_regions: &mut gather.seen_regions }; + delete.visit_mir(mir); + } +} + +impl<'tcx> Visitor<'tcx> for GatherBorrowedRegions { + fn visit_rvalue(&mut self, + rvalue: &Rvalue<'tcx>, + location: Location) { + if let Rvalue::Ref(r, _, _) = *rvalue { + if let RegionKind::ReScope(ce) = *r { + self.seen_regions.insert(ce); + } + } + self.super_rvalue(rvalue, location); + } +} + +impl<'a, 'tcx> MutVisitor<'tcx> for DeleteTrivialEndRegions<'a> { + fn visit_statement(&mut self, + block: BasicBlock, + statement: &mut Statement<'tcx>, + location: Location) { + let mut delete_it = false; + + if let StatementKind::EndRegion(ref extent) = statement.kind { + if !self.seen_regions.contains(extent) { + delete_it = true; + } + } + + if delete_it { + statement.kind = StatementKind::Nop; + } + self.super_statement(block, statement, location); + } +} diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index fcea5d4c86047..4594c611d596f 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -24,6 +24,7 @@ use syntax::ast; use syntax_pos::{DUMMY_SP, Span}; use transform; +pub mod clean_end_regions; pub mod simplify_branches; pub mod simplify; pub mod erase_regions; From 9dd55276a6249110ce6aa52e450612f716f27149 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 23 Mar 2017 11:03:17 +0100 Subject: [PATCH 08/11] Leverage mir-opt to encode tests for `EndRegion`. The tests use `-Z identify_regions` so one can eyeball output for sanity. The tests with closures use `-Z span_free_formats` so that host-specific paths do not get embedded into the dumped MIR. The tests check against MIR dump output immediately prior to borrowck (determined by hand to be the dump from after the "qualify-consts" pass) since that is when `EndRegion` will be most relevant in the near term. --- src/test/mir-opt/README.md | 27 ++++++++- src/test/mir-opt/end_region_1.rs | 38 +++++++++++++ src/test/mir-opt/end_region_2.rs | 66 ++++++++++++++++++++++ src/test/mir-opt/end_region_3.rs | 69 +++++++++++++++++++++++ src/test/mir-opt/end_region_4.rs | 75 ++++++++++++++++++++++++ src/test/mir-opt/end_region_5.rs | 80 ++++++++++++++++++++++++++ src/test/mir-opt/end_region_6.rs | 83 +++++++++++++++++++++++++++ src/test/mir-opt/end_region_7.rs | 97 ++++++++++++++++++++++++++++++++ src/test/mir-opt/end_region_8.rs | 86 ++++++++++++++++++++++++++++ src/test/mir-opt/end_region_9.rs | 85 ++++++++++++++++++++++++++++ 10 files changed, 705 insertions(+), 1 deletion(-) create mode 100644 src/test/mir-opt/end_region_1.rs create mode 100644 src/test/mir-opt/end_region_2.rs create mode 100644 src/test/mir-opt/end_region_3.rs create mode 100644 src/test/mir-opt/end_region_4.rs create mode 100644 src/test/mir-opt/end_region_5.rs create mode 100644 src/test/mir-opt/end_region_6.rs create mode 100644 src/test/mir-opt/end_region_7.rs create mode 100644 src/test/mir-opt/end_region_8.rs create mode 100644 src/test/mir-opt/end_region_9.rs diff --git a/src/test/mir-opt/README.md b/src/test/mir-opt/README.md index 9144e9757f6a7..28a124e3c61c8 100644 --- a/src/test/mir-opt/README.md +++ b/src/test/mir-opt/README.md @@ -22,7 +22,32 @@ All the test information is in comments so the test is runnable. For each $file_name, compiletest expects [$expected_line_0, ..., $expected_line_N] to appear in the dumped MIR in order. Currently it allows -other non-matched lines before, after and in-between. +other non-matched lines before, after and in-between. Note that this includes +lines that end basic blocks or begin new ones; it is good practice +in your tests to include the terminator for each of your basic blocks as an +internal sanity check guarding against a test like: + +``` +bb0: { + StorageLive(_1); + _1 = const true; + StorageDead(_1); +} +``` + +that will inadvertantly pattern-matching against: + +``` +bb0: { + StorageLive(_1); + _1 = const true; + goto -> bb1 +} +bb1: { + StorageDead(_1); + return; +} +``` Lines match ignoring whitespace, and the prefix "//" is removed. diff --git a/src/test/mir-opt/end_region_1.rs b/src/test/mir-opt/end_region_1.rs new file mode 100644 index 0000000000000..55dac4440275f --- /dev/null +++ b/src/test/mir-opt/end_region_1.rs @@ -0,0 +1,38 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z identify_regions +// ignore-tidy-linelength + +// This is just about the simplest program that exhibits an EndRegion. + +fn main() { + let a = 3; + let b = &a; +} + +// END RUST SOURCE +// START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// let mut _0: (); +// let _1: i32; +// let _2: &'6_1rce i32; +// +// bb0: { +// StorageLive(_1); +// _1 = const 3i32; +// StorageLive(_2); +// _2 = &'6_1rce _1; +// _0 = (); +// StorageDead(_2); +// EndRegion('6_1rce); +// StorageDead(_1); +// return; +// } +// END rustc.node4.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_2.rs b/src/test/mir-opt/end_region_2.rs new file mode 100644 index 0000000000000..a1386ec47a13b --- /dev/null +++ b/src/test/mir-opt/end_region_2.rs @@ -0,0 +1,66 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z identify_regions +// ignore-tidy-linelength + +// We will EndRegion for borrows in a loop that occur before break but +// not those after break. + +fn main() { + loop { + let a = true; + let b = &a; + if a { break; } + let c = &a; + } +} + +// END RUST SOURCE +// START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// let mut _0: (); +// let _2: bool; +// let _3: &'7_1rce bool; +// let _7: &'7_3rce bool; +// let mut _4: (); +// let mut _5: bool; +// bb0: { +// goto -> bb1; +// } +// bb1: { +// StorageLive(_2); +// _2 = const true; +// StorageLive(_3); +// _3 = &'7_1rce _2; +// StorageLive(_5); +// _5 = _2; +// switchInt(_5) -> [0u8: bb3, otherwise: bb2]; +// } +// bb2: { +// _0 = (); +// StorageDead(_5); +// StorageDead(_3); +// EndRegion('7_1rce); +// StorageDead(_2); +// return; +// } +// bb3: { +// StorageDead(_5); +// StorageLive(_7); +// _7 = &'7_3rce _2; +// _1 = (); +// StorageDead(_7); +// EndRegion('7_3rce); +// StorageDead(_3); +// EndRegion('7_1rce); +// StorageDead(_2); +// goto -> bb1; +// } +// END rustc.node4.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_3.rs b/src/test/mir-opt/end_region_3.rs new file mode 100644 index 0000000000000..b3d2809e76ceb --- /dev/null +++ b/src/test/mir-opt/end_region_3.rs @@ -0,0 +1,69 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z identify_regions +// ignore-tidy-linelength + +// Binding the borrow's subject outside the loop does not increase the +// scope of the borrow. + +fn main() { + let mut a; + loop { + a = true; + let b = &a; + if a { break; } + let c = &a; + } +} + +// END RUST SOURCE +// START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// let mut _0: (); +// let mut _1: bool; +// let _3: &'9_1rce bool; +// let _7: &'9_3rce bool; +// let mut _2: (); +// let mut _4: (); +// let mut _5: bool; +// +// bb0: { +// StorageLive(_1); +// goto -> bb1; +// } +// bb1: { +// _1 = const true; +// StorageLive(_3); +// _3 = &'9_1rce _1; +// StorageLive(_5); +// _5 = _1; +// switchInt(_5) -> [0u8: bb3, otherwise: bb2]; +// } +// bb2: { +// _0 = (); +// StorageDead(_5); +// StorageDead(_3); +// EndRegion('9_1rce); +// StorageDead(_1); +// return; +// } +// bb3: { +// _4 = (); +// StorageDead(_5); +// StorageLive(_7); +// _7 = &'9_3rce _1; +// _2 = (); +// StorageDead(_7); +// EndRegion('9_3rce); +// StorageDead(_3); +// EndRegion('9_1rce); +// goto -> bb1; +// } +// END rustc.node4.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_4.rs b/src/test/mir-opt/end_region_4.rs new file mode 100644 index 0000000000000..16ade9f96fd1e --- /dev/null +++ b/src/test/mir-opt/end_region_4.rs @@ -0,0 +1,75 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z identify_regions +// ignore-tidy-linelength + +// Unwinding should EndRegion for in-scope borrows: Direct borrows. + +fn main() { + let d = D(0); + let a = 0; + let b = &a; + foo(*b); + let c = &a; +} + +struct D(i32); +impl Drop for D { fn drop(&mut self) { println!("dropping D({})", self.0); } } + +fn foo(i: i32) { + if i > 0 { panic!("im positive"); } +} + +// END RUST SOURCE +// START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// let mut _0: (); +// let _1: D; +// let _3: i32; +// let _4: &'6_2rce i32; +// let _7: &'6_4rce i32; +// let mut _5: (); +// let mut _6: i32; +// +// bb0: { +// StorageLive(_1); +// _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_3); +// _3 = const 0i32; +// StorageLive(_4); +// _4 = &'6_2rce _3; +// StorageLive(_6); +// _6 = (*_4); +// _5 = const foo(_6) -> [return: bb2, unwind: bb3]; +// } +// bb1: { +// resume; +// } +// bb2: { +// StorageDead(_6); +// StorageLive(_7); +// _7 = &'6_4rce _3; +// _0 = (); +// StorageDead(_7); +// EndRegion('6_4rce); +// StorageDead(_4); +// EndRegion('6_2rce); +// StorageDead(_3); +// drop(_1) -> bb4; +// } +// bb3: { +// EndRegion('6_2rce); +// drop(_1) -> bb1; +// } +// bb4: { +// StorageDead(_1); +// return; +// } +// END rustc.node4.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_5.rs b/src/test/mir-opt/end_region_5.rs new file mode 100644 index 0000000000000..513632a4cdf38 --- /dev/null +++ b/src/test/mir-opt/end_region_5.rs @@ -0,0 +1,80 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z identify_regions -Z span_free_formats +// ignore-tidy-linelength + +// Unwinding should EndRegion for in-scope borrows: Borrowing via by-ref closure. + +fn main() { + let d = D(0); + foo(|| -> i32 { d.0 }); +} + +struct D(i32); +impl Drop for D { fn drop(&mut self) { println!("dropping D({})", self.0); } } + +fn foo(f: F) where F: FnOnce() -> i32 { + if f() > 0 { panic!("im positive"); } +} + +// END RUST SOURCE +// START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// fn main() -> () { +// let mut _0: (); +// let _1: D; +// let mut _2: (); +// let mut _3: (); +// let mut _4: [closure@NodeId(18) d: &'19mce D]; +// let mut _5: &'19mce D; +// +// bb0: { +// StorageLive(_1); +// _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_4); +// StorageLive(_5); +// _5 = &'19mce _1; +// _4 = [closure@NodeId(18)] { d: _5 }; +// StorageDead(_5); +// _3 = const foo(_4) -> [return: bb2, unwind: bb3]; +// } +// bb1: { +// resume; +// } +// bb2: { +// StorageDead(_4); +// EndRegion('19mce); +// _0 = (); +// drop(_1) -> bb4; +// } +// bb3: { +// EndRegion('19mce); +// drop(_1) -> bb1; +// } +// bb4: { +// StorageDead(_1); +// return; +// } +// } +// END rustc.node4.SimplifyCfg-qualify-consts.after.mir + +// START rustc.node18.SimplifyCfg-qualify-consts.after.mir +// fn main::{{closure}}(_1: [closure@NodeId(18) d:&'19mce D]) -> i32 { +// let mut _0: i32; +// let mut _2: i32; +// +// bb0: { +// StorageLive(_2); +// _2 = ((*(_1.0: &'19mce D)).0: i32); +// _0 = _2; +// StorageDead(_2); +// return; +// } +// END rustc.node18.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_6.rs b/src/test/mir-opt/end_region_6.rs new file mode 100644 index 0000000000000..e82556f3ce4bb --- /dev/null +++ b/src/test/mir-opt/end_region_6.rs @@ -0,0 +1,83 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z identify_regions -Z span_free_formats +// ignore-tidy-linelength + +// Unwinding should EndRegion for in-scope borrows: 2nd borrow within by-ref closure. + +fn main() { + let d = D(0); + foo(|| -> i32 { let r = &d; r.0 }); +} + +struct D(i32); +impl Drop for D { fn drop(&mut self) { println!("dropping D({})", self.0); } } + +fn foo(f: F) where F: FnOnce() -> i32 { + if f() > 0 { panic!("im positive"); } +} + +// END RUST SOURCE +// START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// let mut _0: (); +// let _1: D; +// let mut _2: (); +// let mut _3: (); +// let mut _4: [closure@NodeId(22) d:&'23mce D]; +// let mut _5: &'23mce D; +// +// bb0: { +// StorageLive(_1); +// _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_4); +// StorageLive(_5); +// _5 = &'23mce _1; +// _4 = [closure@NodeId(22)] { d: _5 }; +// StorageDead(_5); +// _3 = const foo(_4) -> [return: bb2, unwind: bb3]; +// } +// bb1: { +// resume; +// } +// bb2: { +// StorageDead(_4); +// EndRegion('23mce); +// _0 = (); +// drop(_1) -> bb4; +// } +// bb3: { +// EndRegion('23mce); +// drop(_1) -> bb1; +// } +// bb4: { +// StorageDead(_1); +// return; +// } +// END rustc.node4.SimplifyCfg-qualify-consts.after.mir + +// START rustc.node22.SimplifyCfg-qualify-consts.after.mir +// fn main::{{closure}}(_1: [closure@NodeId(22) d:&'23mce D]) -> i32 { +// let mut _0: i32; +// let _2: &'14_0rce D; +// let mut _3: i32; +// +// bb0: { +// StorageLive(_2); +// _2 = &'14_0rce (*(_1.0: &'23mce D)); +// StorageLive(_3); +// _3 = ((*_2).0: i32); +// _0 = _3; +// StorageDead(_3); +// StorageDead(_2); +// EndRegion('14_0rce); +// return; +// } +// END rustc.node22.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_7.rs b/src/test/mir-opt/end_region_7.rs new file mode 100644 index 0000000000000..3fbd3f368659d --- /dev/null +++ b/src/test/mir-opt/end_region_7.rs @@ -0,0 +1,97 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z identify_regions -Z span_free_formats +// ignore-tidy-linelength + +// Unwinding should EndRegion for in-scope borrows: Borrow of moved data. + +fn main() { + let d = D(0); + foo(move || -> i32 { let r = &d; r.0 }); +} + +struct D(i32); +impl Drop for D { fn drop(&mut self) { println!("dropping D({})", self.0); } } + +fn foo(f: F) where F: FnOnce() -> i32 { + if f() > 0 { panic!("im positive"); } +} + +// END RUST SOURCE +// START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// fn main() -> () { +// let mut _0: (); +// let _1: D; +// let mut _2: (); +// let mut _3: (); +// let mut _4: [closure@NodeId(22) d:D]; +// let mut _5: D; +// +// bb0: { +// StorageLive(_1); +// _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_4); +// StorageLive(_5); +// _5 = _1; +// _4 = [closure@NodeId(22)] { d: _5 }; +// drop(_5) -> [return: bb4, unwind: bb3]; +// } +// bb1: { +// resume; +// } +// bb2: { +// drop(_1) -> bb1; +// } +// bb3: { +// drop(_4) -> bb2; +// } +// bb4: { +// StorageDead(_5); +// _3 = const foo(_4) -> [return: bb5, unwind: bb3]; +// } +// bb5: { +// drop(_4) -> [return: bb6, unwind: bb2]; +// } +// bb6: { +// StorageDead(_4); +// _0 = (); +// drop(_1) -> bb7; +// } +// bb7: { +// StorageDead(_1); +// return; +// } +// } +// END rustc.node4.SimplifyCfg-qualify-consts.after.mir + +// START rustc.node22.SimplifyCfg-qualify-consts.after.mir +// fn main::{{closure}}(_1: [closure@NodeId(22) d:D]) -> i32 { +// let mut _0: i32; +// let _2: &'14_0rce D; +// let mut _3: (); +// let mut _4: i32; +// +// bb0: { +// StorageLive(_2); +// _2 = &'14_0rce (_1.0: D); +// StorageLive(_4); +// _4 = ((*_2).0: i32); +// _0 = _4; +// StorageDead(_4); +// StorageDead(_2); +// EndRegion('14_0rce); +// drop(_1) -> bb1; +// } +// bb1: { +// return; +// } +// } +// END rustc.node22.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_8.rs b/src/test/mir-opt/end_region_8.rs new file mode 100644 index 0000000000000..7fb3f0b91181a --- /dev/null +++ b/src/test/mir-opt/end_region_8.rs @@ -0,0 +1,86 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z identify_regions -Z span_free_formats +// ignore-tidy-linelength + +// Unwinding should EndRegion for in-scope borrows: Move of borrow into closure. + +fn main() { + let d = D(0); + let r = &d; + foo(move || -> i32 { r.0 }); +} + +struct D(i32); +impl Drop for D { fn drop(&mut self) { println!("dropping D({})", self.0); } } + +fn foo(f: F) where F: FnOnce() -> i32 { + if f() > 0 { panic!("im positive"); } +} + +// END RUST SOURCE +// START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// fn main() -> () { +// let mut _0: (); +// let _1: D; +// let _3: &'6_1rce D; +// let mut _2: (); +// let mut _4: (); +// let mut _5: [closure@NodeId(22) r:&'6_1rce D]; +// let mut _6: &'6_1rce D; +// +// bb0: { +// StorageLive(_1); +// _1 = D::{{constructor}}(const 0i32,); +// StorageLive(_3); +// _3 = &'6_1rce _1; +// StorageLive(_5); +// StorageLive(_6); +// _6 = _3; +// _5 = [closure@NodeId(22)] { r: _6 }; +// StorageDead(_6); +// _4 = const foo(_5) -> [return: bb2, unwind: bb3]; +// } +// bb1: { +// resume; +// } +// bb2: { +// StorageDead(_5); +// _0 = (); +// StorageDead(_3); +// EndRegion('6_1rce); +// drop(_1) -> bb4; +// } +// bb3: { +// EndRegion('6_1rce); +// drop(_1) -> bb1; +// } +// bb4: { +// StorageDead(_1); +// return; +// } +// } +// END rustc.node4.SimplifyCfg-qualify-consts.after.mir + +// START rustc.node22.SimplifyCfg-qualify-consts.after.mir +// fn main::{{closure}}(_1: [closure@NodeId(22) r:&'6_1rce D]) -> i32 { +// let mut _0: i32; +// let mut _2: i32; +// +// bb0: { +// StorageLive(_2); +// _2 = ((*(_1.0: &'6_1rce D)).0: i32); +// _0 = _2; +// StorageDead(_2); +// return; +// } +// } +// END rustc.node22.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/end_region_9.rs b/src/test/mir-opt/end_region_9.rs new file mode 100644 index 0000000000000..deff984e4d0de --- /dev/null +++ b/src/test/mir-opt/end_region_9.rs @@ -0,0 +1,85 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-flags: -Z identify_regions -Z span_free_formats +// ignore-tidy-linelength + +// This test models a scenario that arielb1 found during review. +// Namely, any filtering of EndRegions must ensure to continue to emit +// any necessary EndRegions that occur earlier in the source than the +// first borrow involving that region. +// +// It is tricky to actually construct examples of this, which is the +// main reason that I am keeping this test even though I have now +// removed the pre-filter that motivated the test in the first place. + +fn main() { + let mut second_iter = false; + let x = 3; + 'a: loop { + let mut y; + loop { + if second_iter { + break 'a; // want to generate `EndRegion('a)` here + } else { + y = &/*'a*/ x; + } + second_iter = true; + } + } +} + +// END RUST SOURCE +// START rustc.node4.SimplifyCfg-qualify-consts.after.mir +// fn main() -> () { +// let mut _0: (); +// let mut _1: bool; +// let _2: i32; +// let mut _4: &'13_0rce i32; +// let mut _3: (); +// let mut _5: !; +// let mut _6: (); +// let mut _7: bool; +// let mut _8: !; +// +// bb0: { +// StorageLive(_1); +// _1 = const false; +// StorageLive(_2); +// _2 = const 3i32; +// StorageLive(_4); +// goto -> bb1; +// } +// +// bb1: { +// StorageLive(_7); +// _7 = _1; +// switchInt(_7) -> [0u8: bb3, otherwise: bb2]; +// } +// +// bb2: { +// _0 = (); +// StorageDead(_7); +// StorageDead(_4); +// EndRegion('13_0rce); +// StorageDead(_2); +// StorageDead(_1); +// return; +// } +// +// bb3: { +// _4 = &'13_0rce _2; +// _6 = (); +// StorageDead(_7); +// _1 = const true; +// _3 = (); +// goto -> bb1; +// } +// } From 5eff019779c5875c53c7fc39a72603cf8825e38d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 29 May 2017 11:46:02 +0200 Subject: [PATCH 09/11] Update basic_assignment test to reflect small changes to codegen. --- src/test/mir-opt/basic_assignment.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/test/mir-opt/basic_assignment.rs b/src/test/mir-opt/basic_assignment.rs index e4eb1aeaf9be2..ef5158a403a93 100644 --- a/src/test/mir-opt/basic_assignment.rs +++ b/src/test/mir-opt/basic_assignment.rs @@ -50,7 +50,7 @@ fn main() { // StorageLive(_6); // StorageLive(_7); // _7 = _4; -// replace(_6 <- _7) -> [return: bb5, unwind: bb4]; +// replace(_6 <- _7) -> [return: bb6, unwind: bb7]; // } // bb1: { // resume; @@ -59,24 +59,30 @@ fn main() { // drop(_4) -> bb1; // } // bb3: { -// drop(_6) -> bb2; +// goto -> bb2; // } // bb4: { -// drop(_7) -> bb3; +// drop(_6) -> bb3; // } // bb5: { -// drop(_7) -> [return: bb6, unwind: bb3]; +// goto -> bb4; // } // bb6: { +// drop(_7) -> [return: bb8, unwind: bb4]; +// } +// bb7: { +// drop(_7) -> bb5; +// } +// bb8: { // StorageDead(_7); // _0 = (); -// drop(_6) -> [return: bb7, unwind: bb2]; +// drop(_6) -> [return: bb9, unwind: bb2]; // } -// bb7: { +// bb9: { // StorageDead(_6); -// drop(_4) -> bb8; +// drop(_4) -> bb10; // } -// bb8: { +// bb10: { // StorageDead(_4); // StorageDead(_2); // StorageDead(_1); From 163d40d1d86d06b6c937ec5a062a134025984b4f Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 31 May 2017 16:07:24 +0200 Subject: [PATCH 10/11] Update test/codegen/drop.rs to reflect inconsequential change in basic block ordering. --- src/test/codegen/drop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/codegen/drop.rs b/src/test/codegen/drop.rs index d7e2cb6d9a50b..1961060c2c266 100644 --- a/src/test/codegen/drop.rs +++ b/src/test/codegen/drop.rs @@ -32,9 +32,9 @@ pub fn droppy() { // CHECK-NOT: invoke{{.*}}drop{{.*}}SomeUniqueName // CHECK: call{{.*}}drop{{.*}}SomeUniqueName // CHECK: call{{.*}}drop{{.*}}SomeUniqueName -// CHECK: call{{.*}}drop{{.*}}SomeUniqueName // CHECK-NOT: call{{.*}}drop{{.*}}SomeUniqueName // CHECK: invoke{{.*}}drop{{.*}}SomeUniqueName +// CHECK: call{{.*}}drop{{.*}}SomeUniqueName // CHECK: invoke{{.*}}drop{{.*}}SomeUniqueName // CHECK: call{{.*}}drop{{.*}}SomeUniqueName // CHECK-NOT: {{(call|invoke).*}}drop{{.*}}SomeUniqueName From 11f4968bd7372ed3986ff7d83fb14218ef0f2f20 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 2 Jun 2017 12:52:09 +0200 Subject: [PATCH 11/11] Revised comment explaining my addition of case for `TerminatorKind::Resume`. Also, I removed the `continue;` statement. I don't think it makes a difference whether its there or not, but having it there confuses things when the actual goal was to side-step the assertion in the default case. --- src/librustc_borrowck/borrowck/mir/elaborate_drops.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs b/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs index 7acfc2c3bbaff..833697726089e 100644 --- a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs +++ b/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs @@ -585,11 +585,6 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { // drop elaboration should handle that by itself continue } - TerminatorKind::Resume => { - // We can replace resumes with gotos - // jumping to a canonical resume. - continue - } TerminatorKind::DropAndReplace { .. } => { // this contains the move of the source and // the initialization of the destination. We @@ -599,6 +594,11 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { assert!(self.patch.is_patched(bb)); allow_initializations = false; } + TerminatorKind::Resume => { + // It is possible for `Resume` to be patched + // (in particular it can be patched to be replaced with + // a Goto; see `MirPatch::new`). + } _ => { assert!(!self.patch.is_patched(bb)); }