From fb3e871734957d83aab0cc55d45be7d75d6a7b15 Mon Sep 17 00:00:00 2001 From: James Miller Date: Tue, 16 Dec 2014 12:35:59 +1300 Subject: [PATCH] Add some documentation --- src/librustc_trans/trans/base.rs | 10 ++++++++++ src/librustc_trans/trans/controlflow.rs | 9 +++++++++ src/librustc_trans/trans/expr.rs | 20 +++++++++++++++++++- 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 1a82a7131a774..87f9d4c447f9f 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -1393,6 +1393,16 @@ fn build_cfg(tcx: &ty::ctxt, id: ast::NodeId) -> (ast::NodeId, Option) (blk.id, Some(cfg::CFG::new(tcx, &**blk))) } +// Checks for the presence of "nested returns" in a function. +// Nested returns are when the inner expression of a return expression +// (the 'expr' in 'return expr') contains a return expression. Only cases +// where the outer return is actually reachable are considered. Implicit +// returns from the end of blocks are considered as well. +// +// This check is needed to handle the case where the inner expression is +// part of a larger expression that may have already partially-filled the +// return slot alloca. This can cause errors related to clean-up due to +// the clobbering of the existing value in the return slot. fn has_nested_returns(tcx: &ty::ctxt, cfg: &cfg::CFG, blk_id: ast::NodeId) -> bool { for n in cfg.graph.depth_traverse(cfg.entry) { match tcx.map.find(n.id) { diff --git a/src/librustc_trans/trans/controlflow.rs b/src/librustc_trans/trans/controlflow.rs index a1574aa2f0e43..e55da561c9409 100644 --- a/src/librustc_trans/trans/controlflow.rs +++ b/src/librustc_trans/trans/controlflow.rs @@ -112,8 +112,17 @@ pub fn trans_block<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, if dest != expr::Ignore { let block_ty = node_id_type(bcx, b.id); + if b.expr.is_none() || type_is_zero_size(bcx.ccx(), block_ty) { dest = expr::Ignore; + } else if b.expr.is_some() { + // If the block has an expression, but that expression isn't reachable, + // don't save into the destination given, ignore it. + if let Some(ref cfg) = bcx.fcx.cfg { + if !cfg.node_is_reachable(b.expr.as_ref().unwrap().id) { + dest = expr::Ignore; + } + } } } diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs index 690e7cf81f5f5..0307412ce74ce 100644 --- a/src/librustc_trans/trans/expr.rs +++ b/src/librustc_trans/trans/expr.rs @@ -928,7 +928,25 @@ fn trans_rvalue_stmt_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, controlflow::trans_cont(bcx, expr.id, label_opt) } ast::ExprRet(ref ex) => { - controlflow::trans_ret(bcx, ex.as_ref().map(|e| &**e)) + // Check to see if the return expression itself is reachable. + // This can occur when the inner expression contains a return + let reachable = if let Some(ref cfg) = bcx.fcx.cfg { + cfg.node_is_reachable(expr.id) + } else { + true + }; + + if reachable { + controlflow::trans_ret(bcx, ex.as_ref().map(|e| &**e)) + } else { + // If it's not reachable, just translate the inner expression + // directly. This avoids having to manage a return slot when + // it won't actually be used anyway. + if let &Some(ref x) = ex { + bcx = trans_into(bcx, &**x, Ignore); + } + bcx + } } ast::ExprWhile(ref cond, ref body, _) => { controlflow::trans_while(bcx, expr.id, &**cond, &**body)