From de7cb0fdd69c95158d217b9a913f1e25f3bfeef0 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 17 May 2016 01:06:52 +0300 Subject: [PATCH] introduce DropAndReplace for translating assignments this introduces a DropAndReplace terminator as a fix to #30380. That terminator is suppsoed to be translated by desugaring during drop elaboration, which is not implemented in this commit, so this breaks `-Z orbit` temporarily. --- src/librustc/mir/repr.rs | 36 +++++++++++++++---- src/librustc/mir/visit.rs | 14 ++++++-- .../borrowck/mir/dataflow/mod.rs | 11 ++++-- .../borrowck/mir/gather_moves.rs | 12 +++++-- src/librustc_borrowck/borrowck/mir/mod.rs | 16 ++++++--- src/librustc_mir/build/expr/stmt.rs | 28 +++++++-------- src/librustc_mir/build/scope.rs | 33 +++++++++++++---- .../transform/break_cleanup_edges.rs | 4 ++- src/librustc_mir/transform/no_landing_pads.rs | 5 ++- src/librustc_mir/transform/promote_consts.rs | 2 +- src/librustc_mir/transform/qualify_consts.rs | 1 + src/librustc_mir/transform/type_check.rs | 14 ++++++++ src/librustc_trans/mir/block.rs | 8 +++-- 13 files changed, 139 insertions(+), 45 deletions(-) diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs index f9a671435ff..db4c0c1e9eb 100644 --- a/src/librustc/mir/repr.rs +++ b/src/librustc/mir/repr.rs @@ -330,11 +330,19 @@ pub enum TerminatorKind<'tcx> { /// Drop the Lvalue Drop { - value: Lvalue<'tcx>, + location: Lvalue<'tcx>, target: BasicBlock, unwind: Option }, + /// Drop the Lvalue and assign the new value over it + DropAndReplace { + location: Lvalue<'tcx>, + value: Operand<'tcx>, + target: BasicBlock, + unwind: Option, + }, + /// Block ends with a call of a converging function Call { /// The function that’s being called @@ -373,8 +381,14 @@ impl<'tcx> TerminatorKind<'tcx> { slice::ref_slice(t).into_cow(), Call { destination: None, cleanup: Some(ref c), .. } => slice::ref_slice(c).into_cow(), Call { destination: None, cleanup: None, .. } => (&[]).into_cow(), - Drop { target, unwind: Some(unwind), .. } => vec![target, unwind].into_cow(), - Drop { ref target, .. } => slice::ref_slice(target).into_cow(), + DropAndReplace { target, unwind: Some(unwind), .. } | + Drop { target, unwind: Some(unwind), .. } => { + vec![target, unwind].into_cow() + } + DropAndReplace { ref target, unwind: None, .. } | + Drop { ref target, unwind: None, .. } => { + slice::ref_slice(target).into_cow() + } } } @@ -393,8 +407,12 @@ impl<'tcx> TerminatorKind<'tcx> { Call { destination: Some((_, ref mut t)), cleanup: None, .. } => vec![t], Call { destination: None, cleanup: Some(ref mut c), .. } => vec![c], Call { destination: None, cleanup: None, .. } => vec![], + DropAndReplace { ref mut target, unwind: Some(ref mut unwind), .. } | Drop { ref mut target, unwind: Some(ref mut unwind), .. } => vec![target, unwind], - Drop { ref mut target, .. } => vec![target] + DropAndReplace { ref mut target, unwind: None, .. } | + Drop { ref mut target, unwind: None, .. } => { + vec![target] + } } } } @@ -461,7 +479,9 @@ impl<'tcx> TerminatorKind<'tcx> { SwitchInt { discr: ref lv, .. } => write!(fmt, "switchInt({:?})", lv), Return => write!(fmt, "return"), Resume => write!(fmt, "resume"), - Drop { ref value, .. } => write!(fmt, "drop({:?})", value), + Drop { ref location, .. } => write!(fmt, "drop({:?})", location), + DropAndReplace { ref location, ref value, .. } => + write!(fmt, "replace({:?} <- {:?})", location, value), Call { ref func, ref args, ref destination, .. } => { if let Some((ref destination, _)) = *destination { write!(fmt, "{:?} = ", destination)?; @@ -506,8 +526,12 @@ impl<'tcx> TerminatorKind<'tcx> { Call { destination: Some(_), cleanup: None, .. } => vec!["return".into_cow()], Call { destination: None, cleanup: Some(_), .. } => vec!["unwind".into_cow()], Call { destination: None, cleanup: None, .. } => vec![], + DropAndReplace { unwind: None, .. } | Drop { unwind: None, .. } => vec!["return".into_cow()], - Drop { .. } => vec!["return".into_cow(), "unwind".into_cow()], + DropAndReplace { unwind: Some(_), .. } | + Drop { unwind: Some(_), .. } => { + vec!["return".into_cow(), "unwind".into_cow()] + } } } } diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 88460651352..17a8d040ab4 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -394,10 +394,20 @@ macro_rules! make_mir_visitor { TerminatorKind::Return => { } - TerminatorKind::Drop { ref $($mutability)* value, + TerminatorKind::Drop { ref $($mutability)* location, target, unwind } => { - self.visit_lvalue(value, LvalueContext::Drop); + self.visit_lvalue(location, LvalueContext::Drop); + self.visit_branch(block, target); + unwind.map(|t| self.visit_branch(block, t)); + } + + TerminatorKind::DropAndReplace { ref $($mutability)* location, + ref $($mutability)* value, + target, + unwind } => { + self.visit_lvalue(location, LvalueContext::Drop); + self.visit_operand(value); self.visit_branch(block, target); unwind.map(|t| self.visit_branch(block, t)); } diff --git a/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs b/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs index b46b6c368a0..99592e5d60f 100644 --- a/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs +++ b/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs @@ -444,10 +444,17 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> repr::TerminatorKind::Return | repr::TerminatorKind::Resume => {} repr::TerminatorKind::Goto { ref target } | - repr::TerminatorKind::Drop { ref target, value: _, unwind: None } => { + repr::TerminatorKind::Drop { ref target, location: _, unwind: None } | + + repr::TerminatorKind::DropAndReplace { + ref target, value: _, location: _, unwind: None + } => { self.propagate_bits_into_entry_set_for(in_out, changed, target); } - repr::TerminatorKind::Drop { ref target, value: _, unwind: Some(ref unwind) } => { + repr::TerminatorKind::Drop { ref target, location: _, unwind: Some(ref unwind) } | + repr::TerminatorKind::DropAndReplace { + ref target, value: _, location: _, unwind: Some(ref unwind) + } => { self.propagate_bits_into_entry_set_for(in_out, changed, target); self.propagate_bits_into_entry_set_for(in_out, changed, unwind); } diff --git a/src/librustc_borrowck/borrowck/mir/gather_moves.rs b/src/librustc_borrowck/borrowck/mir/gather_moves.rs index 48511cd5ebc..fcaa655f749 100644 --- a/src/librustc_borrowck/borrowck/mir/gather_moves.rs +++ b/src/librustc_borrowck/borrowck/mir/gather_moves.rs @@ -671,10 +671,18 @@ fn gather_moves<'a, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> MoveD let _ = discr; } - TerminatorKind::Drop { value: ref lval, target: _, unwind: _ } => { + TerminatorKind::Drop { ref location, target: _, unwind: _ } => { let source = Location { block: bb, index: bb_data.statements.len() }; - bb_ctxt.on_move_out_lval(SK::Drop, lval, source); + bb_ctxt.on_move_out_lval(SK::Drop, location, source); + } + TerminatorKind::DropAndReplace { ref location, ref value, .. } => { + let assigned_path = bb_ctxt.builder.move_path_for(location); + bb_ctxt.path_map.fill_to(assigned_path.idx()); + + let source = Location { block: bb, + index: bb_data.statements.len() }; + bb_ctxt.on_operand(SK::Use, value, source); } TerminatorKind::Call { ref func, ref args, ref destination, cleanup: _ } => { let source = Location { block: bb, diff --git a/src/librustc_borrowck/borrowck/mir/mod.rs b/src/librustc_borrowck/borrowck/mir/mod.rs index 1b9d08bade7..38ebecf248f 100644 --- a/src/librustc_borrowck/borrowck/mir/mod.rs +++ b/src/librustc_borrowck/borrowck/mir/mod.rs @@ -309,15 +309,23 @@ fn drop_flag_effects_for_location<'a, 'tcx, F>( Some(stmt) => match stmt.kind { repr::StatementKind::Assign(ref lvalue, _) => { debug!("drop_flag_effects: assignment {:?}", stmt); - on_all_children_bits(tcx, mir, move_data, + on_all_children_bits(tcx, mir, move_data, move_data.rev_lookup.find(lvalue), |moi| callback(moi, DropFlagState::Present)) } }, None => { - // terminator - no move-ins except for function return edge - let term = bb.terminator(); - debug!("drop_flag_effects: terminator {:?}", term); + debug!("drop_flag_effects: replace {:?}", bb.terminator()); + match bb.terminator().kind { + repr::TerminatorKind::DropAndReplace { ref location, .. } => { + on_all_children_bits(tcx, mir, move_data, + move_data.rev_lookup.find(location), + |moi| callback(moi, DropFlagState::Present)) + } + _ => { + // other terminators do not contain move-ins + } + } } } } diff --git a/src/librustc_mir/build/expr/stmt.rs b/src/librustc_mir/build/expr/stmt.rs index 9629396f48b..3324467e70d 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -34,29 +34,25 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let scope_id = this.innermost_scope_id(); let lhs_span = lhs.span; - let lhs_ty = lhs.ty; - let rhs_ty = rhs.ty; - - let lhs_needs_drop = this.hir.needs_drop(lhs_ty); - let rhs_needs_drop = this.hir.needs_drop(rhs_ty); - // Note: we evaluate assignments right-to-left. This // is better for borrowck interaction with overloaded // operators like x[j] = x[i]. // Generate better code for things that don't need to be // dropped. - let rhs = if lhs_needs_drop || rhs_needs_drop { - let op = unpack!(block = this.as_operand(block, rhs)); - Rvalue::Use(op) + if this.hir.needs_drop(lhs.ty) { + let rhs = unpack!(block = this.as_operand(block, rhs)); + let lhs = unpack!(block = this.as_lvalue(block, lhs)); + unpack!(block = this.build_drop_and_replace( + block, lhs_span, lhs, rhs + )); + block.unit() } else { - unpack!(block = this.as_rvalue(block, rhs)) - }; - - let lhs = unpack!(block = this.as_lvalue(block, lhs)); - unpack!(block = this.build_drop(block, lhs_span, lhs.clone(), lhs_ty)); - this.cfg.push_assign(block, scope_id, expr_span, &lhs, rhs); - block.unit() + let rhs = unpack!(block = this.as_rvalue(block, rhs)); + let lhs = unpack!(block = this.as_lvalue(block, lhs)); + this.cfg.push_assign(block, scope_id, expr_span, &lhs, rhs); + block.unit() + } } ExprKind::AssignOp { op, lhs, rhs } => { // FIXME(#28160) there is an interesting semantics diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 071c8d618c8..cd81fc764f4 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -139,7 +139,7 @@ struct DropData<'tcx> { span: Span, /// lvalue to drop - value: Lvalue<'tcx>, + location: Lvalue<'tcx>, /// The cached block for the cleanups-on-diverge path. This block /// contains code to run the current drop and all the preceding @@ -402,7 +402,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // the drop that comes before it in the vector. scope.drops.push(DropData { span: span, - value: lvalue.clone(), + location: lvalue.clone(), cached_block: None }); return; @@ -497,7 +497,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { pub fn build_drop(&mut self, block: BasicBlock, span: Span, - value: Lvalue<'tcx>, + location: Lvalue<'tcx>, ty: Ty<'tcx>) -> BlockAnd<()> { if !self.hir.needs_drop(ty) { return block.unit(); @@ -509,7 +509,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { scope_id, span, TerminatorKind::Drop { - value: value, + location: location, target: next_target, unwind: diverge_target, }); @@ -517,6 +517,27 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } + + pub fn build_drop_and_replace(&mut self, + block: BasicBlock, + span: Span, + location: Lvalue<'tcx>, + value: Operand<'tcx>) -> BlockAnd<()> { + let scope_id = self.innermost_scope_id(); + let next_target = self.cfg.start_new_block(); + let diverge_target = self.diverge_cleanup(); + self.cfg.terminate(block, + scope_id, + span, + TerminatorKind::DropAndReplace { + location: location, + value: value, + target: next_target, + unwind: diverge_target, + }); + next_target.unit() + } + // Panicking // ========= // FIXME: should be moved into their own module @@ -653,7 +674,7 @@ fn build_scope_drops<'tcx>(cfg: &mut CFG<'tcx>, }); let next = cfg.start_new_block(); cfg.terminate(block, scope.id, drop_data.span, TerminatorKind::Drop { - value: drop_data.value.clone(), + location: drop_data.location.clone(), target: next, unwind: on_diverge }); @@ -709,7 +730,7 @@ fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, scope.id, drop_data.span, TerminatorKind::Drop { - value: drop_data.value.clone(), + location: drop_data.location.clone(), target: target, unwind: None }); diff --git a/src/librustc_mir/transform/break_cleanup_edges.rs b/src/librustc_mir/transform/break_cleanup_edges.rs index 0eb6223a71e..4902d31cf4d 100644 --- a/src/librustc_mir/transform/break_cleanup_edges.rs +++ b/src/librustc_mir/transform/break_cleanup_edges.rs @@ -105,7 +105,9 @@ impl Pass for BreakCleanupEdges {} fn term_is_invoke(term: &Terminator) -> bool { match term.kind { TerminatorKind::Call { cleanup: Some(_), .. } | - TerminatorKind::Drop { unwind: Some(_), .. } => true, + // FIXME: not sure whether we need this one + TerminatorKind::Drop { unwind: Some(_), .. } | + TerminatorKind::DropAndReplace { .. } => true, _ => false } } diff --git a/src/librustc_mir/transform/no_landing_pads.rs b/src/librustc_mir/transform/no_landing_pads.rs index de05032fa55..67710c43285 100644 --- a/src/librustc_mir/transform/no_landing_pads.rs +++ b/src/librustc_mir/transform/no_landing_pads.rs @@ -29,12 +29,11 @@ impl<'tcx> MutVisitor<'tcx> for NoLandingPads { TerminatorKind::SwitchInt { .. } => { /* nothing to do */ }, + TerminatorKind::Call { cleanup: ref mut unwind, .. } | + TerminatorKind::DropAndReplace { ref mut unwind, .. } | TerminatorKind::Drop { ref mut unwind, .. } => { unwind.take(); }, - TerminatorKind::Call { ref mut cleanup, .. } => { - cleanup.take(); - }, } self.super_terminator(bb, terminator); } diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 431568b004d..d81c4e2dfb6 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -399,7 +399,7 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, }); let terminator = block.terminator_mut(); match terminator.kind { - TerminatorKind::Drop { value: Lvalue::Temp(index), target, .. } => { + TerminatorKind::Drop { location: Lvalue::Temp(index), target, .. } => { if promoted(index) { terminator.kind = TerminatorKind::Goto { target: target diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 2e4400c834f..18a1f1595f3 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -422,6 +422,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { TerminatorKind::Switch {..} | TerminatorKind::SwitchInt {..} | + TerminatorKind::DropAndReplace { .. } | TerminatorKind::Resume => None, TerminatorKind::Return => { diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 80c56a5dc08..7a41211381c 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -363,6 +363,20 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { // no checks needed for these } + + TerminatorKind::DropAndReplace { + ref location, + ref value, + .. + } => { + let lv_ty = mir.lvalue_ty(tcx, location).to_ty(tcx); + let rv_ty = mir.operand_ty(tcx, value); + if let Err(terr) = self.sub_types(self.last_span, rv_ty, lv_ty) { + span_mirbug!(self, term, "bad DropAndReplace ({:?} = {:?}): {:?}", + lv_ty, rv_ty, terr); + } + } + TerminatorKind::If { ref cond, .. } => { let cond_ty = mir.operand_ty(tcx, cond); match cond_ty.sty { diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index 4e3386bc736..fb93e487f3b 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -143,8 +143,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { }) } - mir::TerminatorKind::Drop { ref value, target, unwind } => { - let lvalue = self.trans_lvalue(&bcx, value); + mir::TerminatorKind::Drop { ref location, target, unwind } => { + let lvalue = self.trans_lvalue(&bcx, location); let ty = lvalue.ty.to_ty(bcx.tcx()); // Double check for necessity to drop if !glue::type_needs_drop(bcx.tcx(), ty) { @@ -177,6 +177,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { } } + mir::TerminatorKind::DropAndReplace { .. } => { + bug!("undesugared DropAndReplace in trans: {:?}", data); + } + mir::TerminatorKind::Call { ref func, ref args, ref destination, ref cleanup } => { // Create the callee. This is a fn ptr or zero-sized and hence a kind of scalar. let callee = self.trans_operand(&bcx, func);