Skip to content

Commit

Permalink
Auto merge of #33239 - eddyb:mir-temp-drops, r=arielb1
Browse files Browse the repository at this point in the history
mir: drop temps outside-in by scheduling the drops inside-out.

It was backwards all along, but only noticeable with multiple drops in one rvalue scope. Fixes #32433.
  • Loading branch information
bors committed May 11, 2016
2 parents c7ab884 + e940de6 commit c049541
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 16 deletions.
16 changes: 9 additions & 7 deletions src/librustc_mir/build/expr/as_temp.rs
Expand Up @@ -35,12 +35,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {

let expr_ty = expr.ty.clone();
let temp = this.temp(expr_ty.clone());
// In constants, temp_lifetime is None. We should not need to drop
// anything because no values with a destructor can be created in
// a constant at this time, even if the type may need dropping.
if let Some(temp_lifetime) = expr.temp_lifetime {
this.schedule_drop(expr.span, temp_lifetime, &temp, expr_ty);
}
let temp_lifetime = expr.temp_lifetime;
let expr_span = expr.span;

// Careful here not to cause an infinite cycle. If we always
// called `into`, then for lvalues like `x.f`, it would
Expand All @@ -51,7 +47,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// course) `as_temp`.
match Category::of(&expr.kind).unwrap() {
Category::Lvalue => {
let expr_span = expr.span;
let lvalue = unpack!(block = this.as_lvalue(block, expr));
let rvalue = Rvalue::Use(Operand::Consume(lvalue));
let scope_id = this.innermost_scope_id();
Expand All @@ -62,6 +57,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
}

// In constants, temp_lifetime is None. We should not need to drop
// anything because no values with a destructor can be created in
// a constant at this time, even if the type may need dropping.
if let Some(temp_lifetime) = temp_lifetime {
this.schedule_drop(expr_span, temp_lifetime, &temp, expr_ty);
}

block.and(temp)
}
}
16 changes: 7 additions & 9 deletions src/test/run-pass/issue-23338-ensure-param-drop-order.rs
Expand Up @@ -8,8 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(rustc_attrs)]

// ignore-pretty : (#23623) problems when ending with // comments

// This test is ensuring that parameters are indeed dropped after
Expand Down Expand Up @@ -42,11 +40,11 @@ pub fn main() {
// | | | | eval tail of foo
// | | | +-- Make D(de_5, 6)
// | | | | +-- Make D(de_6, 7)
6, // | | | +-- Drop D(de_5, 6)
// | | | | |
5, // | | | | +-- Drop D(de_4, 5)
// | | | |
5, // | | | | | +-- Drop D(de_4, 5)
// | | | | |
2, // | | +-- Drop D(de_2, 2)
// | | | |
6, // | | +-- Drop D(de_5, 6)
// | | |
1, // | +-- Drop D(de_1, 1)
// | |
Expand All @@ -66,8 +64,8 @@ fn test<'a>(log: d::Log<'a>) {
d::println(&format!("result {}", result));
}

#[rustc_no_mir] // FIXME #29855 MIR doesn't handle all drops correctly.
fn foo<'a>(da0: D<'a>, de1: D<'a>) -> D<'a> {
// FIXME(#33490) Remove the double braces when old trans is gone.
fn foo<'a>(da0: D<'a>, de1: D<'a>) -> D<'a> {{
d::println("entered foo");
let de2 = de1.incr(); // creates D(de_2, 2)
let de4 = {
Expand All @@ -76,7 +74,7 @@ fn foo<'a>(da0: D<'a>, de1: D<'a>) -> D<'a> {
};
d::println("eval tail of foo");
de4.incr().incr() // creates D(de_5, 6) and D(de_6, 7)
}
}}

// This module provides simultaneous printouts of the dynamic extents
// of all of the D values, in addition to logging the order that each
Expand Down

0 comments on commit c049541

Please sign in to comment.