Skip to content

Commit

Permalink
Convert Drop statement into terminator
Browse files Browse the repository at this point in the history
The structure of the old translator as well as MIR assumed that drop glue cannot possibly panic and
translated the drops accordingly. However, in presence of `Drop::drop` this assumption can be
trivially shown to be untrue. As such, the Rust code like the following would never print number 2:

```rust
struct Droppable(u32);
impl Drop for Droppable {
    fn drop(&mut self) {
        if self.0 == 1 { panic!("Droppable(1)") } else { println!("{}", self.0) }
    }
}
fn main() {
    let x = Droppable(2);
    let y = Droppable(1);
}
```

While the behaviour is allowed according to the language rules (we allow drops to not run), that’s
a very counter-intuitive behaviour. We fix this in MIR by allowing `Drop` to have a target to take
on divergence and connect the drops in such a way so the leftover drops are executed when some drop
unwinds.

Note, that this commit still does not implement the translator part of changes necessary for the
grand scheme of things to fully work, so the actual observed behaviour does not change yet. Coming
soon™.

See #14875.
  • Loading branch information
nagisa committed Feb 4, 2016
1 parent 65dd5e6 commit 98265d3
Show file tree
Hide file tree
Showing 9 changed files with 309 additions and 161 deletions.
18 changes: 15 additions & 3 deletions src/librustc/mir/repr.rs
Expand Up @@ -262,6 +262,13 @@ pub enum Terminator<'tcx> {
/// `END_BLOCK`.
Return,

/// Drop the Lvalue
Drop {
value: Lvalue<'tcx>,
target: BasicBlock,
unwind: Option<BasicBlock>
},

/// Block ends with a call of a converging function
Call {
/// The function that’s being called
Expand Down Expand Up @@ -290,6 +297,8 @@ impl<'tcx> Terminator<'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(),
}
}

Expand All @@ -308,6 +317,8 @@ impl<'tcx> Terminator<'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![],
Drop { ref mut target, unwind: Some(ref mut unwind), .. } => vec![target, unwind],
Drop { ref mut target, .. } => vec![target]
}
}
}
Expand Down Expand Up @@ -374,6 +385,7 @@ impl<'tcx> Terminator<'tcx> {
SwitchInt { discr: ref lv, .. } => write!(fmt, "switchInt({:?})", lv),
Return => write!(fmt, "return"),
Resume => write!(fmt, "resume"),
Drop { ref value, .. } => write!(fmt, "drop({:?})", value),
Call { ref func, ref args, ref destination, .. } => {
if let Some((ref destination, _)) = *destination {
try!(write!(fmt, "{:?} = ", destination));
Expand Down Expand Up @@ -418,6 +430,8 @@ impl<'tcx> Terminator<'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![],
Drop { unwind: None, .. } => vec!["return".into_cow()],
Drop { .. } => vec!["return".into_cow(), "unwind".into_cow()],
}
}
}
Expand All @@ -435,15 +449,13 @@ pub struct Statement<'tcx> {
#[derive(Clone, Debug, RustcEncodable, RustcDecodable)]
pub enum StatementKind<'tcx> {
Assign(Lvalue<'tcx>, Rvalue<'tcx>),
Drop(Lvalue<'tcx>),
}

impl<'tcx> Debug for Statement<'tcx> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
use self::StatementKind::*;
match self.kind {
Assign(ref lv, ref rv) => write!(fmt, "{:?} = {:?}", lv, rv),
Drop(ref lv) => write!(fmt, "drop {:?}", lv),
Assign(ref lv, ref rv) => write!(fmt, "{:?} = {:?}", lv, rv)
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions src/librustc/mir/visit.rs
Expand Up @@ -124,9 +124,6 @@ macro_rules! make_mir_visitor {
ref $($mutability)* rvalue) => {
self.visit_assign(block, lvalue, rvalue);
}
StatementKind::Drop(ref $($mutability)* lvalue) => {
self.visit_lvalue(lvalue, LvalueContext::Drop);
}
}
}

Expand Down Expand Up @@ -177,10 +174,16 @@ macro_rules! make_mir_visitor {
Terminator::Return => {
}

Terminator::Drop { ref $($mutability)* value, target, unwind } => {
self.visit_lvalue(value, LvalueContext::Drop);
self.visit_branch(block, target);
unwind.map(|t| self.visit_branch(block, t));
}

Terminator::Call { ref $($mutability)* func,
ref $($mutability)* args,
ref $($mutability)* destination,
ref $($mutability)* cleanup } => {
cleanup } => {
self.visit_operand(func);
for arg in args {
self.visit_operand(arg);
Expand Down
7 changes: 0 additions & 7 deletions src/librustc_mir/build/cfg.rs
Expand Up @@ -43,13 +43,6 @@ impl<'tcx> CFG<'tcx> {
self.block_data_mut(block).statements.push(statement);
}

pub fn push_drop(&mut self, block: BasicBlock, span: Span, lvalue: &Lvalue<'tcx>) {
self.push(block, Statement {
span: span,
kind: StatementKind::Drop(lvalue.clone())
});
}

pub fn push_assign(&mut self,
block: BasicBlock,
span: Span,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/build/expr/into.rs
Expand Up @@ -188,7 +188,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
// operators like x[j] = x[i].
let rhs = unpack!(block = this.as_operand(block, rhs));
let lhs = unpack!(block = this.as_lvalue(block, lhs));
this.cfg.push_drop(block, expr_span, &lhs);
unpack!(block = this.build_drop(block, lhs.clone()));
this.cfg.push_assign(block, expr_span, &lhs, Rvalue::Use(rhs));
block.unit()
}
Expand Down

0 comments on commit 98265d3

Please sign in to comment.