Skip to content

Commit

Permalink
Revert "Make into schedule drop for the destination"
Browse files Browse the repository at this point in the history
This reverts commit 3702683.
  • Loading branch information
matthewjasper committed Oct 9, 2019
1 parent 321ccbe commit a0342c8
Show file tree
Hide file tree
Showing 13 changed files with 247 additions and 466 deletions.
70 changes: 21 additions & 49 deletions src/librustc_mir/build/block.rs
@@ -1,22 +1,18 @@
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use crate::build::ForGuard::OutsideGuard;
use crate::build::matches::ArmHasGuard;
use crate::build::scope::DropKind;
use crate::hair::*;
use rustc::middle::region;
use rustc::mir::*;
use rustc::hir;
use syntax_pos::Span;

impl<'a, 'tcx> Builder<'a, 'tcx> {
pub fn ast_block(
&mut self,
destination: &Place<'tcx>,
scope: Option<region::Scope>,
block: BasicBlock,
ast_block: &'tcx hir::Block,
source_info: SourceInfo,
) -> BlockAnd<()> {
pub fn ast_block(&mut self,
destination: &Place<'tcx>,
block: BasicBlock,
ast_block: &'tcx hir::Block,
source_info: SourceInfo)
-> BlockAnd<()> {
let Block {
region_scope,
opt_destruction_scope,
Expand All @@ -25,61 +21,37 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
expr,
targeted_by_break,
safety_mode
} = self.hir.mirror(ast_block);
} =
self.hir.mirror(ast_block);
self.in_opt_scope(opt_destruction_scope.map(|de|(de, source_info)), move |this| {
this.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
if targeted_by_break {
// This is a `break`-able block
let exit_block = this.cfg.start_new_block();
if let Some(scope) = scope {
// Breakable blocks assign to their destination on each
// `break`, as well as when they exit normally. So we
// can't schedule the drop in the last expression like
// normal blocks do.
let local = destination.as_local()
.expect("cannot schedule drop of non-Local place");
this.schedule_drop(span, scope, local, DropKind::Value);
}
let block_exit = this.in_breakable_scope(
None, exit_block, destination.clone(), |this| {
this.ast_block_stmts(
destination,
None,
block,
span,
stmts,
expr,
safety_mode,
)
this.ast_block_stmts(destination, block, span, stmts, expr,
safety_mode)
});
this.cfg.terminate(unpack!(block_exit), source_info,
TerminatorKind::Goto { target: exit_block });
exit_block.unit()
} else {
this.ast_block_stmts(
destination,
scope,
block,
span,
stmts,
expr,
safety_mode,
)
this.ast_block_stmts(destination, block, span, stmts, expr,
safety_mode)
}
})
})
}

fn ast_block_stmts(
&mut self,
destination: &Place<'tcx>,
scope: Option<region::Scope>,
mut block: BasicBlock,
span: Span,
stmts: Vec<StmtRef<'tcx>>,
expr: Option<ExprRef<'tcx>>,
safety_mode: BlockSafety,
) -> BlockAnd<()> {
fn ast_block_stmts(&mut self,
destination: &Place<'tcx>,
mut block: BasicBlock,
span: Span,
stmts: Vec<StmtRef<'tcx>>,
expr: Option<ExprRef<'tcx>>,
safety_mode: BlockSafety)
-> BlockAnd<()> {
let this = self;

// This convoluted structure is to avoid using recursion as we walk down a list
Expand Down Expand Up @@ -205,7 +177,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.block_context.currently_ignores_tail_results();
this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored });

unpack!(block = this.into(destination, scope, block, expr));
unpack!(block = this.into(destination, block, expr));
let popped = this.block_context.pop();

assert!(popped.map_or(false, |bf|bf.is_tail_expr()));
Expand Down
7 changes: 2 additions & 5 deletions src/librustc_mir/build/expr/as_rvalue.rs
Expand Up @@ -136,14 +136,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.cfg
.push_assign(block, source_info, &Place::from(result), box_);

// Initialize the box contents. No scope is needed since the
// `Box` is already scheduled to be dropped.
// initialize the box contents:
unpack!(
block = this.into(
&Place::from(result).deref(),
None,
block,
value
block, value
)
);
block.and(Rvalue::Use(Operand::Move(Place::from(result))))
Expand Down
11 changes: 10 additions & 1 deletion src/librustc_mir/build/expr/as_temp.rs
Expand Up @@ -109,7 +109,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

unpack!(block = this.into(temp_place, temp_lifetime, block, expr));
unpack!(block = this.into(temp_place, block, expr));

if let Some(temp_lifetime) = temp_lifetime {
this.schedule_drop(
expr_span,
temp_lifetime,
temp,
DropKind::Value,
);
}

block.and(temp)
}
Expand Down
46 changes: 9 additions & 37 deletions src/librustc_mir/build/expr/into.rs
Expand Up @@ -2,9 +2,7 @@

use crate::build::expr::category::{Category, RvalueFunc};
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use crate::build::scope::DropKind;
use crate::hair::*;
use rustc::middle::region;
use rustc::mir::*;
use rustc::ty;

Expand All @@ -13,18 +11,15 @@ use rustc_target::spec::abi::Abi;
impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Compile `expr`, storing the result into `destination`, which
/// is assumed to be uninitialized.
/// If a `drop_scope` is provided, `destination` is scheduled to be dropped
/// in `scope` once it has been initialized.
pub fn into_expr(
&mut self,
destination: &Place<'tcx>,
scope: Option<region::Scope>,
mut block: BasicBlock,
expr: Expr<'tcx>,
) -> BlockAnd<()> {
debug!(
"into_expr(destination={:?}, scope={:?}, block={:?}, expr={:?})",
destination, scope, block, expr
"into_expr(destination={:?}, block={:?}, expr={:?})",
destination, block, expr
);

// since we frequently have to reference `self` from within a
Expand All @@ -40,14 +35,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
_ => false,
};

let schedule_drop = move |this: &mut Self| {
if let Some(drop_scope) = scope {
let local = destination.as_local()
.expect("cannot schedule drop of non-Local place");
this.schedule_drop(expr_span, drop_scope, local, DropKind::Value);
}
};

if !expr_is_block_or_scope {
this.block_context.push(BlockFrame::SubExpr);
}
Expand All @@ -60,14 +47,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} => {
let region_scope = (region_scope, source_info);
this.in_scope(region_scope, lint_level, |this| {
this.into(destination, scope, block, value)
this.into(destination, block, value)
})
}
ExprKind::Block { body: ast_block } => {
this.ast_block(destination, scope, block, ast_block, source_info)
this.ast_block(destination, block, ast_block, source_info)
}
ExprKind::Match { scrutinee, arms } => {
this.match_expr(destination, scope, expr_span, block, scrutinee, arms)
this.match_expr(destination, expr_span, block, scrutinee, arms)
}
ExprKind::NeverToAny { source } => {
let source = this.hir.mirror(source);
Expand All @@ -80,7 +67,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

// This is an optimization. If the expression was a call then we already have an
// unreachable block. Don't bother to terminate it and create a new one.
schedule_drop(this);
if is_call {
block.unit()
} else {
Expand Down Expand Up @@ -178,9 +164,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
TerminatorKind::Goto { target: loop_block },
);

// Loops assign to their destination on each `break`. Since we
// can't easily unschedule drops, we schedule the drop now.
schedule_drop(this);
this.in_breakable_scope(
Some(loop_block),
exit_block,
Expand All @@ -202,8 +185,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// introduce a unit temporary as the destination for the loop body.
let tmp = this.get_unit_temp();
// Execute the body, branching back to the test.
// No scope is provided, since we've scheduled the drop above.
let body_block_end = unpack!(this.into(&tmp, None, body_block, body));
let body_block_end = unpack!(this.into(&tmp, body_block, body));
this.cfg.terminate(
body_block_end,
source_info,
Expand Down Expand Up @@ -252,14 +234,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
is_block_tail: None,
});
let ptr_temp = Place::from(ptr_temp);
// No need for a scope, ptr_temp doesn't need drop
let block = unpack!(this.into(&ptr_temp, None, block, ptr));
// Maybe we should provide a scope here so that
// `move_val_init` wouldn't leak on panic even with an
// arbitrary `val` expression, but `schedule_drop`,
// borrowck and drop elaboration all prevent us from
// dropping `ptr_temp.deref()`.
this.into(&ptr_temp.deref(), None, block, val)
let block = unpack!(this.into(&ptr_temp, block, ptr));
this.into(&ptr_temp.deref(), block, val)
} else {
let args: Vec<_> = args
.into_iter()
Expand Down Expand Up @@ -289,12 +265,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
from_hir_call,
},
);
schedule_drop(this);
success.unit()
}
}
ExprKind::Use { source } => {
this.into(destination, scope, block, source)
this.into(destination, block, source)
}

// These cases don't actually need a destination
Expand All @@ -321,7 +296,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place));
this.cfg
.push_assign(block, source_info, destination, rvalue);
schedule_drop(this);
block.unit()
}
ExprKind::Index { .. } | ExprKind::Deref { .. } | ExprKind::Field { .. } => {
Expand All @@ -341,7 +315,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place));
this.cfg
.push_assign(block, source_info, destination, rvalue);
schedule_drop(this);
block.unit()
}

Expand Down Expand Up @@ -373,7 +346,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

let rvalue = unpack!(block = this.as_local_rvalue(block, expr));
this.cfg.push_assign(block, source_info, destination, rvalue);
schedule_drop(this);
block.unit()
}
};
Expand Down
25 changes: 9 additions & 16 deletions src/librustc_mir/build/into.rs
Expand Up @@ -6,31 +6,26 @@

use crate::build::{BlockAnd, Builder};
use crate::hair::*;
use rustc::middle::region;
use rustc::mir::*;

pub(in crate::build) trait EvalInto<'tcx> {
fn eval_into(
self,
builder: &mut Builder<'_, 'tcx>,
destination: &Place<'tcx>,
scope: Option<region::Scope>,
block: BasicBlock,
) -> BlockAnd<()>;
}

impl<'a, 'tcx> Builder<'a, 'tcx> {
pub fn into<E>(
&mut self,
destination: &Place<'tcx>,
scope: Option<region::Scope>,
block: BasicBlock,
expr: E,
) -> BlockAnd<()>
where
E: EvalInto<'tcx>,
pub fn into<E>(&mut self,
destination: &Place<'tcx>,
block: BasicBlock,
expr: E)
-> BlockAnd<()>
where E: EvalInto<'tcx>
{
expr.eval_into(self, destination, scope, block)
expr.eval_into(self, destination, block)
}
}

Expand All @@ -39,11 +34,10 @@ impl<'tcx> EvalInto<'tcx> for ExprRef<'tcx> {
self,
builder: &mut Builder<'_, 'tcx>,
destination: &Place<'tcx>,
scope: Option<region::Scope>,
block: BasicBlock,
) -> BlockAnd<()> {
let expr = builder.hir.mirror(self);
builder.into_expr(destination, scope, block, expr)
builder.into_expr(destination, block, expr)
}
}

Expand All @@ -52,9 +46,8 @@ impl<'tcx> EvalInto<'tcx> for Expr<'tcx> {
self,
builder: &mut Builder<'_, 'tcx>,
destination: &Place<'tcx>,
scope: Option<region::Scope>,
block: BasicBlock,
) -> BlockAnd<()> {
builder.into_expr(destination, scope, block, self)
builder.into_expr(destination, block, self)
}
}

0 comments on commit a0342c8

Please sign in to comment.