From 101a2f59b490650c12c5f9e4561a7390bfce78d3 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 15 Jun 2019 10:08:41 +0100 Subject: [PATCH] Use `as_temp` to evaluate statement expressions --- src/librustc_mir/build/block.rs | 4 +- src/librustc_mir/build/expr/stmt.rs | 80 ++++++++----------- src/librustc_mir/build/scope.rs | 21 ----- src/librustc_mir/hair/cx/block.rs | 3 - src/librustc_mir/hair/mod.rs | 4 - src/test/mir-opt/box_expr.rs | 4 +- src/test/mir-opt/copy_propagation_arg.rs | 2 + src/test/mir-opt/generator-drop-cleanup.rs | 2 + .../mir-opt/generator-storage-dead-unwind.rs | 6 ++ src/test/mir-opt/issue-38669.rs | 3 + src/test/mir-opt/issue-41110.rs | 2 +- src/test/mir-opt/issue-49232.rs | 4 +- src/test/mir-opt/loop_test.rs | 1 + src/test/mir-opt/match_test.rs | 1 + .../mir-opt/nll/region-subtyping-basic.rs | 6 +- src/test/mir-opt/storage_ranges.rs | 2 + .../issue-61442-stmt-expr-with-drop.rs | 32 ++++++++ 17 files changed, 96 insertions(+), 81 deletions(-) create mode 100644 src/test/ui/generator/issue-61442-stmt-expr-with-drop.rs diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 749cd6fc8bf1e..7ea08b15b443d 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -78,7 +78,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let source_info = this.source_info(span); for stmt in stmts { - let Stmt { kind, opt_destruction_scope, span: stmt_span } = this.hir.mirror(stmt); + 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 }); @@ -87,7 +87,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let si = (scope, source_info); this.in_scope(si, LintLevel::Inherited, |this| { let expr = this.hir.mirror(expr); - this.stmt_expr(block, expr, Some(stmt_span)) + this.stmt_expr(block, expr, Some(scope)) }) })); } diff --git a/src/librustc_mir/build/expr/stmt.rs b/src/librustc_mir/build/expr/stmt.rs index 4463e7fd4d4a6..3c5eafb41a233 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -1,6 +1,7 @@ use crate::build::scope::BreakableScope; use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; use crate::hair::*; +use rustc::middle::region; use rustc::mir::*; impl<'a, 'tcx> Builder<'a, 'tcx> { @@ -8,14 +9,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// If the original expression was an AST statement, /// (e.g., `some().code(&here());`) then `opt_stmt_span` is the /// span of that statement (including its semicolon, if any). - /// Diagnostics use this span (which may be larger than that of - /// `expr`) to identify when statement temporaries are dropped. - pub fn stmt_expr(&mut self, - mut block: BasicBlock, - expr: Expr<'tcx>, - opt_stmt_span: Option) - -> BlockAnd<()> - { + /// The scope is used if a statement temporary must be dropped. + pub fn stmt_expr( + &mut self, + mut block: BasicBlock, + expr: Expr<'tcx>, + statement_scope: Option, + ) -> BlockAnd<()> { let this = self; let expr_span = expr.span; let source_info = this.source_info(expr.span); @@ -30,7 +30,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } => { let value = this.hir.mirror(value); this.in_scope((region_scope, source_info), lint_level, |this| { - this.stmt_expr(block, value, opt_stmt_span) + this.stmt_expr(block, value, statement_scope) }) } ExprKind::Assign { lhs, rhs } => { @@ -199,7 +199,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.unit() } _ => { - let expr_ty = expr.ty; + assert!( + statement_scope.is_some(), + "Should not be calling `stmt_expr` on a general expression \ + without a statement scope", + ); // Issue #54382: When creating temp for the value of // expression like: @@ -208,48 +212,34 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // // it is usually better to focus on `the_value` rather // than the entirety of block(s) surrounding it. - let mut temp_span = expr_span; - let mut temp_in_tail_of_block = false; - if let ExprKind::Block { body } = expr.kind { - if let Some(tail_expr) = &body.expr { - let mut expr = tail_expr; - while let rustc::hir::ExprKind::Block(subblock, _label) = &expr.node { - if let Some(subtail_expr) = &subblock.expr { - expr = subtail_expr - } else { - break; + let adjusted_span = (|| { + if let ExprKind::Block { body } = expr.kind { + if let Some(tail_expr) = &body.expr { + let mut expr = tail_expr; + while let rustc::hir::ExprKind::Block(subblock, _label) = &expr.node { + if let Some(subtail_expr) = &subblock.expr { + expr = subtail_expr + } else { + break; + } } - } - temp_span = expr.span; - temp_in_tail_of_block = true; - } - } - - let temp = { - let mut local_decl = LocalDecl::new_temp(expr.ty.clone(), temp_span); - if temp_in_tail_of_block { - if this.block_context.currently_ignores_tail_results() { - local_decl = local_decl.block_tail(BlockTailInfo { + this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored: true }); + return Some(expr.span); } } - let temp = this.local_decls.push(local_decl); - let place = Place::from(temp); - debug!("created temp {:?} for expr {:?} in block_context: {:?}", - temp, expr, this.block_context); - place - }; - unpack!(block = this.into(&temp, block, expr)); + None + })(); + + let temp = unpack!(block = + this.as_temp(block, statement_scope, expr, Mutability::Not)); - // Attribute drops of the statement's temps to the - // semicolon at the statement's end. - let drop_point = this.hir.tcx().sess.source_map().end_point(match opt_stmt_span { - None => expr_span, - Some(StatementSpan(span)) => span, - }); + if let Some(span) = adjusted_span { + this.local_decls[temp].source_info.span = span; + this.block_context.pop(); + } - unpack!(block = this.build_drop(block, drop_point, temp, expr_ty)); block.unit() } } diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index db58a709e9f7c..c5b5f2512436f 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -805,27 +805,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { target } - /// Utility function for *non*-scope code to build their own drops - pub fn build_drop(&mut self, - block: BasicBlock, - span: Span, - location: Place<'tcx>, - ty: Ty<'tcx>) -> BlockAnd<()> { - if !self.hir.needs_drop(ty) { - return block.unit(); - } - let source_info = self.source_info(span); - let next_target = self.cfg.start_new_block(); - let diverge_target = self.diverge_cleanup(); - self.cfg.terminate(block, source_info, - TerminatorKind::Drop { - location, - target: next_target, - unwind: Some(diverge_target), - }); - next_target.unit() - } - /// Utility function for *non*-scope code to build their own drops pub fn build_drop_and_replace(&mut self, block: BasicBlock, diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index d5932052d1aa3..9a73842d2f02a 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -49,7 +49,6 @@ fn mirror_stmts<'a, 'tcx>( for (index, stmt) in stmts.iter().enumerate() { let hir_id = stmt.hir_id; let opt_dxn_ext = cx.region_scope_tree.opt_destruction_scope(hir_id.local_id); - let stmt_span = StatementSpan(cx.tcx.hir().span(hir_id)); match stmt.node { hir::StmtKind::Expr(ref expr) | hir::StmtKind::Semi(ref expr) => { @@ -62,7 +61,6 @@ fn mirror_stmts<'a, 'tcx>( expr: expr.to_ref(), }, opt_destruction_scope: opt_dxn_ext, - span: stmt_span, }))) } hir::StmtKind::Item(..) => { @@ -107,7 +105,6 @@ fn mirror_stmts<'a, 'tcx>( lint_level: LintLevel::Explicit(local.hir_id), }, opt_destruction_scope: opt_dxn_ext, - span: stmt_span, }))); } } diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 4694241528b71..5431a31c4bb2a 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -55,14 +55,10 @@ pub enum StmtRef<'tcx> { Mirror(Box>), } -#[derive(Clone, Debug)] -pub struct StatementSpan(pub Span); - #[derive(Clone, Debug)] pub struct Stmt<'tcx> { pub kind: StmtKind<'tcx>, pub opt_destruction_scope: Option, - pub span: StatementSpan, } #[derive(Clone, Debug)] diff --git a/src/test/mir-opt/box_expr.rs b/src/test/mir-opt/box_expr.rs index 9f55d84964405..d9fa3d3d4736d 100644 --- a/src/test/mir-opt/box_expr.rs +++ b/src/test/mir-opt/box_expr.rs @@ -24,7 +24,7 @@ impl Drop for S { // let mut _0: (); // let _1: std::boxed::Box; // let mut _2: std::boxed::Box; -// let mut _3: (); +// let _3: (); // let mut _4: std::boxed::Box; // scope 1 { // } @@ -50,6 +50,7 @@ impl Drop for S { // // bb4: { // StorageDead(_2); +// StorageLive(_3); // StorageLive(_4); // _4 = move _1; // _3 = const std::mem::drop::>(move _4) -> [return: bb5, unwind: bb7]; @@ -69,6 +70,7 @@ impl Drop for S { // // bb8: { // StorageDead(_4); +// StorageDead(_3); // _0 = (); // drop(_1) -> bb9; // } diff --git a/src/test/mir-opt/copy_propagation_arg.rs b/src/test/mir-opt/copy_propagation_arg.rs index 4e05484a80c1e..5e5fed12fb5f3 100644 --- a/src/test/mir-opt/copy_propagation_arg.rs +++ b/src/test/mir-opt/copy_propagation_arg.rs @@ -61,12 +61,14 @@ fn main() { // END rustc.foo.CopyPropagation.after.mir // START rustc.bar.CopyPropagation.before.mir // bb0: { +// StorageLive(_2); // StorageLive(_3); // _3 = _1; // _2 = const dummy(move _3) -> bb1; // } // bb1: { // StorageDead(_3); +// StorageDead(_2); // _1 = const 5u8; // ... // return; diff --git a/src/test/mir-opt/generator-drop-cleanup.rs b/src/test/mir-opt/generator-drop-cleanup.rs index 30f6d0deb9187..f97e1ba6c89d4 100644 --- a/src/test/mir-opt/generator-drop-cleanup.rs +++ b/src/test/mir-opt/generator-drop-cleanup.rs @@ -18,6 +18,7 @@ fn main() { // } // bb1: { // StorageDead(_3); +// StorageDead(_2); // goto -> bb5; // } // bb2: { @@ -36,6 +37,7 @@ fn main() { // goto -> bb3; // } // bb7: { +// StorageLive(_2); // StorageLive(_3); // goto -> bb1; // } diff --git a/src/test/mir-opt/generator-storage-dead-unwind.rs b/src/test/mir-opt/generator-storage-dead-unwind.rs index 7be17c4292ae6..bcdb937542716 100644 --- a/src/test/mir-opt/generator-storage-dead-unwind.rs +++ b/src/test/mir-opt/generator-storage-dead-unwind.rs @@ -54,6 +54,7 @@ fn main() { // } // bb2: { // ... +// StorageLive(_6); // StorageLive(_7); // _7 = move _2; // _6 = const take::(move _7) -> [return: bb9, unwind: bb8]; @@ -81,16 +82,20 @@ fn main() { // } // bb8 (cleanup): { // StorageDead(_7); +// StorageDead(_6); // goto -> bb7; // } // bb9: { // StorageDead(_7); +// StorageDead(_6); +// StorageLive(_8); // StorageLive(_9); // _9 = move _3; // _8 = const take::(move _9) -> [return: bb10, unwind: bb11]; // } // bb10: { // StorageDead(_9); +// StorageDead(_8); // ... // StorageDead(_3); // StorageDead(_2); @@ -98,6 +103,7 @@ fn main() { // } // bb11 (cleanup): { // StorageDead(_9); +// StorageDead(_8); // goto -> bb7; // } // bb12: { diff --git a/src/test/mir-opt/issue-38669.rs b/src/test/mir-opt/issue-38669.rs index 909f9b7b6b79a..d980cc891dc40 100644 --- a/src/test/mir-opt/issue-38669.rs +++ b/src/test/mir-opt/issue-38669.rs @@ -25,6 +25,7 @@ fn main() { // falseUnwind -> [real: bb3, cleanup: bb1]; // } // bb3: { +// StorageLive(_3); // StorageLive(_4); // _4 = _1; // FakeRead(ForMatchedPlace, _4); @@ -34,6 +35,7 @@ fn main() { // bb5: { // _3 = (); // StorageDead(_4); +// StorageDead(_3); // _1 = const true; // _2 = (); // goto -> bb2; @@ -41,6 +43,7 @@ fn main() { // bb6: { // _0 = (); // StorageDead(_4); +// StorageDead(_3); // StorageDead(_1); // return; // } diff --git a/src/test/mir-opt/issue-41110.rs b/src/test/mir-opt/issue-41110.rs index 0b678be2ab319..e73390f52b5d5 100644 --- a/src/test/mir-opt/issue-41110.rs +++ b/src/test/mir-opt/issue-41110.rs @@ -42,7 +42,7 @@ impl S { // START rustc.test.ElaborateDrops.after.mir // let mut _0: (); // let _1: S; -// let mut _3: (); +// let _3: (); // let mut _4: S; // let mut _5: S; // let mut _6: bool; diff --git a/src/test/mir-opt/issue-49232.rs b/src/test/mir-opt/issue-49232.rs index 9dde6d821f2bc..d0dbcbd7515f8 100644 --- a/src/test/mir-opt/issue-49232.rs +++ b/src/test/mir-opt/issue-49232.rs @@ -21,7 +21,7 @@ fn main() { // let _2: i32; // let mut _3: bool; // let mut _4: !; -// let mut _5: (); +// let _5: (); // let mut _6: &i32; // scope 1 { // } @@ -73,12 +73,14 @@ fn main() { // bb12: { // FakeRead(ForLet, _2); // StorageDead(_3); +// StorageLive(_5); // StorageLive(_6); // _6 = &_2; // _5 = const std::mem::drop::<&i32>(move _6) -> [return: bb13, unwind: bb4]; // } // bb13: { // StorageDead(_6); +// StorageDead(_5); // _1 = (); // StorageDead(_2); // goto -> bb1; diff --git a/src/test/mir-opt/loop_test.rs b/src/test/mir-opt/loop_test.rs index 68ea60d92787c..177080c04f972 100644 --- a/src/test/mir-opt/loop_test.rs +++ b/src/test/mir-opt/loop_test.rs @@ -25,6 +25,7 @@ fn main() { // bb3: { // Entry into the loop // _1 = (); // StorageDead(_2); +// StorageDead(_1); // goto -> bb5; // } // ... diff --git a/src/test/mir-opt/match_test.rs b/src/test/mir-opt/match_test.rs index ef60a04d1bdfc..aeb162772fac0 100644 --- a/src/test/mir-opt/match_test.rs +++ b/src/test/mir-opt/match_test.rs @@ -75,6 +75,7 @@ fn main() { // goto -> bb14; // } // bb14: { +// StorageDead(_3); // _0 = (); // StorageDead(_2); // StorageDead(_1); diff --git a/src/test/mir-opt/nll/region-subtyping-basic.rs b/src/test/mir-opt/nll/region-subtyping-basic.rs index fa0dbe51c5dc3..8228d9740f0d3 100644 --- a/src/test/mir-opt/nll/region-subtyping-basic.rs +++ b/src/test/mir-opt/nll/region-subtyping-basic.rs @@ -22,9 +22,9 @@ fn main() { // END RUST SOURCE // START rustc.main.nll.0.mir -// | '_#2r | U0 | {bb2[0..=8], bb3[0], bb5[0..=1]} -// | '_#3r | U0 | {bb2[1..=8], bb3[0], bb5[0..=1]} -// | '_#4r | U0 | {bb2[4..=8], bb3[0], bb5[0..=1]} +// | '_#2r | U0 | {bb2[0..=8], bb3[0], bb5[0..=2]} +// | '_#3r | U0 | {bb2[1..=8], bb3[0], bb5[0..=2]} +// | '_#4r | U0 | {bb2[4..=8], bb3[0], bb5[0..=2]} // END rustc.main.nll.0.mir // START rustc.main.nll.0.mir // let _2: &'_#3r usize; diff --git a/src/test/mir-opt/storage_ranges.rs b/src/test/mir-opt/storage_ranges.rs index 6d22e9cd9fab9..95570ff76a6d0 100644 --- a/src/test/mir-opt/storage_ranges.rs +++ b/src/test/mir-opt/storage_ranges.rs @@ -12,6 +12,7 @@ fn main() { // StorageLive(_1); // _1 = const 0i32; // FakeRead(ForLet, _1); +// StorageLive(_2); // StorageLive(_3); // StorageLive(_4); // StorageLive(_5); @@ -23,6 +24,7 @@ fn main() { // _2 = (); // StorageDead(_4); // StorageDead(_3); +// StorageDead(_2); // StorageLive(_6); // _6 = const 1i32; // FakeRead(ForLet, _6); diff --git a/src/test/ui/generator/issue-61442-stmt-expr-with-drop.rs b/src/test/ui/generator/issue-61442-stmt-expr-with-drop.rs new file mode 100644 index 0000000000000..ce4642020f0f1 --- /dev/null +++ b/src/test/ui/generator/issue-61442-stmt-expr-with-drop.rs @@ -0,0 +1,32 @@ +// Test that we don't consider temporaries for statement expressions as live +// across yields + +// check-pass +// edition:2018 + +#![feature(async_await, generators, generator_trait)] + +use std::ops::Generator; + +async fn drop_and_await() { + async {}; + async {}.await; +} + +fn drop_and_yield() { + let x = || { + String::new(); + yield; + }; + Box::pin(x).as_mut().resume(); + let y = static || { + String::new(); + yield; + }; + Box::pin(y).as_mut().resume(); +} + +fn main() { + drop_and_await(); + drop_and_yield(); +}