Skip to content

Commit

Permalink
Only count nested returns when the outer return is reachable
Browse files Browse the repository at this point in the history 
This narrows the definition of nested returns such that only when the
outer return has a chance of being executed (due to the inner return
being conditional) do we mark the function as having nested returns.

Fixes #19684
  • Loading branch information
@Aatch
Aatch committed Dec 18, 2014
1 parent 22a9f25 commit eee209d
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 44 deletions.
7 changes: 7 additions & 0 deletions src/librustc/middle/cfg/mod.rs
View file
Expand Up @@ -49,4 +49,11 @@ impl CFG {
blk: &ast::Block) -> CFG {
construct::construct(tcx, blk)
}

pub fn node_is_reachable(&self, id: ast::NodeId) -> bool {
for node in self.graph.depth_traverse(self.entry) {
if node.id == id { return true }
}
return false;
}
}
37 changes: 37 additions & 0 deletions src/librustc/middle/graph.rs
View file
Expand Up @@ -34,6 +34,7 @@

use std::fmt::{Formatter, Error, Show};
use std::uint;
use std::collections::BitvSet;

pub struct Graph<N,E> {
nodes: Vec<Node<N>> ,
Expand Down Expand Up @@ -294,6 +295,42 @@ impl<N,E> Graph<N,E> {
}
}
}

pub fn depth_traverse<'a>(&'a self, start: NodeIndex) -> DepthFirstTraversal<'a, N, E> {
DepthFirstTraversal {
graph: self,
stack: vec![start],
visited: BitvSet::new()
}
}
}

pub struct DepthFirstTraversal<'g, N:'g, E:'g> {
graph: &'g Graph<N, E>,
stack: Vec<NodeIndex>,
visited: BitvSet
}

impl<'g, N, E> Iterator<&'g N> for DepthFirstTraversal<'g, N, E> {
fn next(&mut self) -> Option<&'g N> {
while self.stack.len() > 0 {
let idx = self.stack.pop().unwrap();
if self.visited.contains(&idx.node_id()) {
continue;
}
self.visited.insert(idx.node_id());
self.graph.each_outgoing_edge(idx, |_, e| -> bool {
if !self.visited.contains(&e.target().node_id()) {
self.stack.push(e.target());
}
true
});

return Some(self.graph.node_data(idx));
}

return None;
}
}

pub fn each_edge_index<F>(max_edge_index: EdgeIndex, mut f: F) where
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_driver/lib.rs
View file
Expand Up @@ -472,7 +472,7 @@ pub fn list_metadata(sess: &Session, path: &Path,
/// The diagnostic emitter yielded to the procedure should be used for reporting
/// errors of the compiler.
pub fn monitor<F:FnOnce()+Send>(f: F) {
static STACK_SIZE: uint = 32000000; // 32MB
static STACK_SIZE: uint = 8 * 1024 * 1024; // 8MB

let (tx, rx) = channel();
let w = io::ChanWriter::new(tx);
Expand Down
94 changes: 51 additions & 43 deletions src/librustc_trans/trans/base.rs
View file
Expand Up @@ -38,6 +38,7 @@ use llvm::{BasicBlockRef, Linkage, ValueRef, Vector, get_param};
use llvm;
use metadata::{csearch, encoder, loader};
use middle::astencode;
use middle::cfg;
use middle::lang_items::{LangItem, ExchangeMallocFnLangItem, StartFnLangItem};
use middle::subst;
use middle::weak_lang_items;
Expand Down Expand Up @@ -1305,47 +1306,33 @@ pub fn make_return_slot_pointer<'a, 'tcx>(fcx: &FunctionContext<'a, 'tcx>,
}
}

struct CheckForNestedReturnsVisitor {
struct FindNestedReturn {
found: bool,
in_return: bool
}

impl CheckForNestedReturnsVisitor {
fn explicit() -> CheckForNestedReturnsVisitor {
CheckForNestedReturnsVisitor { found: false, in_return: false }
}
fn implicit() -> CheckForNestedReturnsVisitor {
CheckForNestedReturnsVisitor { found: false, in_return: true }
impl FindNestedReturn {
fn new() -> FindNestedReturn {
FindNestedReturn { found: false }
}
}

impl<'v> Visitor<'v> for CheckForNestedReturnsVisitor {
impl<'v> Visitor<'v> for FindNestedReturn {
fn visit_expr(&mut self, e: &ast::Expr) {
match e.node {
ast::ExprRet(..) => {
if self.in_return {
self.found = true;
} else {
self.in_return = true;
visit::walk_expr(self, e);
self.in_return = false;
}
self.found = true;
}
_ => visit::walk_expr(self, e)
}
}
}

fn has_nested_returns(tcx: &ty::ctxt, id: ast::NodeId) -> bool {
match tcx.map.find(id) {
fn build_cfg(tcx: &ty::ctxt, id: ast::NodeId) -> (ast::NodeId, Option<cfg::CFG>) {
let blk = match tcx.map.find(id) {
Some(ast_map::NodeItem(i)) => {
match i.node {
ast::ItemFn(_, _, _, _, ref blk) => {
let mut explicit = CheckForNestedReturnsVisitor::explicit();
let mut implicit = CheckForNestedReturnsVisitor::implicit();
visit::walk_item(&mut explicit, &*i);
visit::walk_expr_opt(&mut implicit, &blk.expr);
explicit.found || implicit.found
blk
}
_ => tcx.sess.bug("unexpected item variant in has_nested_returns")
}
Expand All @@ -1355,11 +1342,7 @@ fn has_nested_returns(tcx: &ty::ctxt, id: ast::NodeId) -> bool {
ast::ProvidedMethod(ref m) => {
match m.node {
ast::MethDecl(_, _, _, _, _, _, ref blk, _) => {
let mut explicit = CheckForNestedReturnsVisitor::explicit();
let mut implicit = CheckForNestedReturnsVisitor::implicit();
visit::walk_method_helper(&mut explicit, &**m);
visit::walk_expr_opt(&mut implicit, &blk.expr);
explicit.found || implicit.found
blk
}
ast::MethMac(_) => tcx.sess.bug("unexpanded macro")
}
Expand All @@ -1379,11 +1362,7 @@ fn has_nested_returns(tcx: &ty::ctxt, id: ast::NodeId) -> bool {
ast::MethodImplItem(ref m) => {
match m.node {
ast::MethDecl(_, _, _, _, _, _, ref blk, _) => {
let mut explicit = CheckForNestedReturnsVisitor::explicit();
let mut implicit = CheckForNestedReturnsVisitor::implicit();
visit::walk_method_helper(&mut explicit, &**m);
visit::walk_expr_opt(&mut implicit, &blk.expr);
explicit.found || implicit.found
blk
}
ast::MethMac(_) => tcx.sess.bug("unexpanded macro")
}
Expand All @@ -1397,24 +1376,47 @@ fn has_nested_returns(tcx: &ty::ctxt, id: ast::NodeId) -> bool {
Some(ast_map::NodeExpr(e)) => {
match e.node {
ast::ExprClosure(_, _, _, ref blk) => {
let mut explicit = CheckForNestedReturnsVisitor::explicit();
let mut implicit = CheckForNestedReturnsVisitor::implicit();
visit::walk_expr(&mut explicit, e);
visit::walk_expr_opt(&mut implicit, &blk.expr);
explicit.found || implicit.found
blk
}
_ => tcx.sess.bug("unexpected expr variant in has_nested_returns")
}
}

Some(ast_map::NodeVariant(..)) | Some(ast_map::NodeStructCtor(..)) => false,
Some(ast_map::NodeVariant(..)) | Some(ast_map::NodeStructCtor(..)) => return (ast::DUMMY_NODE_ID, None),

// glue, shims, etc
None if id == ast::DUMMY_NODE_ID => false,
None if id == ast::DUMMY_NODE_ID => return (ast::DUMMY_NODE_ID, None),

_ => tcx.sess.bug(format!("unexpected variant in has_nested_returns: {}",
tcx.map.path_to_string(id)).as_slice())
};

(blk.id, Some(cfg::CFG::new(tcx, &**blk)))
}

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) {
Some(ast_map::NodeExpr(ex)) => {
if let ast::ExprRet(Some(ref ret_expr)) = ex.node {
let mut visitor = FindNestedReturn::new();
visit::walk_expr(&mut visitor, &**ret_expr);
if visitor.found {
return true;
}
}
}
Some(ast_map::NodeBlock(blk)) if blk.id == blk_id => {
let mut visitor = FindNestedReturn::new();
visit::walk_expr_opt(&mut visitor, &blk.expr);
if visitor.found {
return true;
}
}
_ => {}
}
}

return false;
}

// NB: must keep 4 fns in sync:
Expand Down Expand Up @@ -1453,7 +1455,12 @@ pub fn new_fn_ctxt<'a, 'tcx>(ccx: &'a CrateContext<'a, 'tcx>,
ty::FnDiverging => false
};
let debug_context = debuginfo::create_function_debug_context(ccx, id, param_substs, llfndecl);
let nested_returns = has_nested_returns(ccx.tcx(), id);
let (blk_id, cfg) = build_cfg(ccx.tcx(), id);
let nested_returns = if let Some(ref cfg) = cfg {
has_nested_returns(ccx.tcx(), cfg, blk_id)
} else {
false
};

let mut fcx = FunctionContext {
llfn: llfndecl,
Expand All @@ -1472,7 +1479,8 @@ pub fn new_fn_ctxt<'a, 'tcx>(ccx: &'a CrateContext<'a, 'tcx>,
block_arena: block_arena,
ccx: ccx,
debug_context: debug_context,
scopes: RefCell::new(Vec::new())
scopes: RefCell::new(Vec::new()),
cfg: cfg
};

if has_env {
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_trans/trans/common.rs
View file
Expand Up @@ -18,6 +18,7 @@ use session::Session;
use llvm;
use llvm::{ValueRef, BasicBlockRef, BuilderRef, ContextRef};
use llvm::{True, False, Bool};
use middle::cfg;
use middle::def;
use middle::infer;
use middle::lang_items::LangItem;
Expand Down Expand Up @@ -264,6 +265,8 @@ pub struct FunctionContext<'a, 'tcx: 'a> {

// Cleanup scopes.
pub scopes: RefCell<Vec<cleanup::CleanupScope<'a, 'tcx>>>,

pub cfg: Option<cfg::CFG>,
}

impl<'a, 'tcx> FunctionContext<'a, 'tcx> {
Expand Down

0 comments on commit eee209d

Please sign in to comment.