auto merge of #8462 : thestinger/rust/loop-cleanup, r=cmr

I missed some of this in e7bb33a. Hopefully it's all gone now :).

Author: bors

src/librustc/middle/trans/base.rs

@@ -1624,7 +1624,6 @@ pub fn new_fn_ctxt_w_id(ccx: @mut CrateContext,
           llreturn: None,
           llself: None,
           personality: None,
-          loop_ret: None,
           has_immediate_return_value: is_immediate,
           llargs: @mut HashMap::new(),
           lllocals: @mut HashMap::new(),
@@ -1834,8 +1833,7 @@ pub fn trans_closure(ccx: @mut CrateContext,
                      id: ast::NodeId,
                      attributes: &[ast::Attribute],
                      output_type: ty::t,
-                     maybe_load_env: &fn(@mut FunctionContext),
-                     finish: &fn(@mut Block)) {
+                     maybe_load_env: &fn(@mut FunctionContext)) {
     ccx.stats.n_closures += 1;
     let _icx = push_ctxt("trans_closure");
     set_uwtable(llfndecl);
@@ -1885,7 +1883,6 @@ pub fn trans_closure(ccx: @mut CrateContext,
         bcx = controlflow::trans_block(bcx, body, dest);
     }
 
-    finish(bcx);
     match fcx.llreturn {
         Some(llreturn) => cleanup_and_Br(bcx, bcx_top, llreturn),
         None => bcx = cleanup_block(bcx, Some(bcx_top.llbb))
@@ -1937,8 +1934,7 @@ pub fn trans_fn(ccx: @mut CrateContext,
                           && fcx_has_nonzero_span(fcx) {
                           debuginfo::create_function_metadata(fcx);
                       }
-                  },
-                  |_bcx| { });
+                  });
 }
 
 fn insert_synthetic_type_entries(bcx: @mut Block,

src/librustc/middle/trans/closure.rs

@@ -16,15 +16,13 @@ use middle::moves;
 use middle::trans::base::*;
 use middle::trans::build::*;
 use middle::trans::common::*;
-use middle::trans::datum::{Datum, INIT, ByRef, ZeroMem};
+use middle::trans::datum::{Datum, INIT};
 use middle::trans::expr;
 use middle::trans::glue;
 use middle::trans::type_of::*;
 use middle::ty;
 use util::ppaux::ty_to_str;
 
-use middle::trans::type_::Type;
-
 use std::vec;
 use syntax::ast;
 use syntax::ast_map::path_name;
@@ -259,8 +257,7 @@ pub fn store_environment(bcx: @mut Block,
 // collects the upvars and packages them up for store_environment.
 pub fn build_closure(bcx0: @mut Block,
                      cap_vars: &[moves::CaptureVar],
-                     sigil: ast::Sigil,
-                     include_ret_handle: Option<ValueRef>) -> ClosureResult {
+                     sigil: ast::Sigil) -> ClosureResult {
     let _icx = push_ctxt("closure::build_closure");
 
     // If we need to, package up the iterator body to call
@@ -288,30 +285,6 @@ pub fn build_closure(bcx0: @mut Block,
         }
     }
 
-    // If this is a `for` loop body, add two special environment
-    // variables:
-    for flagptr in include_ret_handle.iter() {
-        // Flag indicating we have returned (a by-ref bool):
-        let flag_datum = Datum {val: *flagptr, ty: ty::mk_bool(),
-                                mode: ByRef(ZeroMem)};
-        env_vals.push(EnvValue {action: EnvRef,
-                                datum: flag_datum});
-
-        // Return value (we just pass a by-ref () and cast it later to
-        // the right thing):
-        let ret_true = match bcx.fcx.loop_ret {
-            Some((_, retptr)) => retptr,
-            None => match bcx.fcx.llretptr {
-                None => C_null(Type::nil().ptr_to()),
-                Some(retptr) => PointerCast(bcx, retptr, Type::nil().ptr_to()),
-            }
-        };
-        let ret_datum = Datum {val: ret_true, ty: ty::mk_nil(),
-                               mode: ByRef(ZeroMem)};
-        env_vals.push(EnvValue {action: EnvRef,
-                                datum: ret_datum});
-    }
-
     return store_environment(bcx, env_vals, sigil);
 }
 
@@ -321,12 +294,11 @@ pub fn build_closure(bcx0: @mut Block,
 pub fn load_environment(fcx: @mut FunctionContext,
                         cdata_ty: ty::t,
                         cap_vars: &[moves::CaptureVar],
-                        load_ret_handle: bool,
                         sigil: ast::Sigil) {
     let _icx = push_ctxt("closure::load_environment");
 
     // Don't bother to create the block if there's nothing to load
-    if cap_vars.len() == 0 && !load_ret_handle {
+    if cap_vars.len() == 0 {
         return;
     }
 
@@ -347,12 +319,6 @@ pub fn load_environment(fcx: @mut FunctionContext,
         fcx.llupvars.insert(def_id.node, upvarptr);
         i += 1u;
     }
-    if load_ret_handle {
-        let flagptr = Load(bcx, GEPi(bcx, llcdata, [0u, i]));
-        let retptr = Load(bcx,
-                          GEPi(bcx, llcdata, [0u, i+1u]));
-        fcx.loop_ret = Some((flagptr, retptr));
-    }
 }
 
 pub fn trans_expr_fn(bcx: @mut Block,
@@ -361,7 +327,6 @@ pub fn trans_expr_fn(bcx: @mut Block,
                      body: &ast::Block,
                      outer_id: ast::NodeId,
                      user_id: ast::NodeId,
-                     is_loop_body: Option<Option<ValueRef>>,
                      dest: expr::Dest) -> @mut Block {
     /*!
      *
@@ -378,7 +343,6 @@ pub fn trans_expr_fn(bcx: @mut Block,
      * - `user_id`: The id of the closure as the user expressed it.
          Generally the same as `outer_id`
      * - `cap_clause`: information about captured variables, if any.
-     * - `is_loop_body`: `Some()` if this is part of a `for` loop.
      * - `dest`: where to write the closure value, which must be a
          (fn ptr, env) pair
      */
@@ -405,28 +369,14 @@ pub fn trans_expr_fn(bcx: @mut Block,
                                                  "expr_fn");
     let llfn = decl_internal_cdecl_fn(ccx.llmod, s, llfnty);
 
-    // Always mark inline if this is a loop body. This is important for
-    // performance on many programs with tight loops.
-    if is_loop_body.is_some() {
-        set_always_inline(llfn);
-    } else {
-        // Can't hurt.
-        set_inline_hint(llfn);
-    }
-
-    let real_return_type = if is_loop_body.is_some() {
-        ty::mk_bool()
-    } else {
-        ty::ty_fn_ret(fty)
-    };
+    // set an inline hint for all closures
+    set_inline_hint(llfn);
 
     let Result {bcx: bcx, val: closure} = match sigil {
         ast::BorrowedSigil | ast::ManagedSigil | ast::OwnedSigil => {
             let cap_vars = ccx.maps.capture_map.get_copy(&user_id);
-            let ret_handle = match is_loop_body {Some(x) => x,
-                                                 None => None};
             let ClosureResult {llbox, cdata_ty, bcx}
-                = build_closure(bcx, cap_vars, sigil, ret_handle);
+                = build_closure(bcx, cap_vars, sigil);
             trans_closure(ccx,
                           sub_path,
                           decl,
@@ -436,16 +386,8 @@ pub fn trans_expr_fn(bcx: @mut Block,
                           bcx.fcx.param_substs,
                           user_id,
                           [],
-                          real_return_type,
-                          |fcx| load_environment(fcx, cdata_ty, cap_vars,
-                                                 ret_handle.is_some(), sigil),
-                          |bcx| {
-                              if is_loop_body.is_some() {
-                                  Store(bcx,
-                                        C_bool(true),
-                                        bcx.fcx.llretptr.unwrap());
-                              }
-                          });
+                          ty::ty_fn_ret(fty),
+                          |fcx| load_environment(fcx, cdata_ty, cap_vars, sigil));
             rslt(bcx, llbox)
         }
     };

src/librustc/middle/trans/common.rs

@@ -196,9 +196,6 @@ pub struct FunctionContext {
     // The a value alloca'd for calls to upcalls.rust_personality. Used when
     // outputting the resume instruction.
     personality: Option<ValueRef>,
-    // If this is a for-loop body that returns, this holds the pointers needed
-    // for that (flagptr, retptr)
-    loop_ret: Option<(ValueRef, ValueRef)>,
 
     // True if this function has an immediate return value, false otherwise.
     // If this is false, the llretptr will alias the first argument of the

src/librustc/middle/trans/context.rs

@@ -13,7 +13,7 @@ use back::{upcall};
 use driver::session;
 use lib::llvm::{ContextRef, ModuleRef, ValueRef};
 use lib::llvm::{llvm, TargetData, TypeNames};
-use lib::llvm::{mk_target_data, False};
+use lib::llvm::mk_target_data;
 use metadata::common::LinkMeta;
 use middle::astencode;
 use middle::resolve;

src/librustc/middle/trans/controlflow.rs

@@ -20,7 +20,6 @@ use middle::trans::build::*;
 use middle::trans::callee;
 use middle::trans::common::*;
 use middle::trans::expr;
-use middle::trans::type_of::*;
 use middle::ty;
 use util::common::indenter;
 use util::ppaux;
@@ -338,29 +337,15 @@ pub fn trans_cont(bcx: @mut Block, label_opt: Option<ident>) -> @mut Block {
 pub fn trans_ret(bcx: @mut Block, e: Option<@ast::expr>) -> @mut Block {
     let _icx = push_ctxt("trans_ret");
     let mut bcx = bcx;
-    let dest = match bcx.fcx.loop_ret {
-      Some((flagptr, retptr)) => {
-        // This is a loop body return. Must set continue flag (our retptr)
-        // to false, return flag to true, and then store the value in the
-        // parent's retptr.
-        Store(bcx, C_bool(true), flagptr);
-        Store(bcx, C_bool(false), bcx.fcx.llretptr.unwrap());
-        expr::SaveIn(match e {
-          Some(x) => PointerCast(bcx, retptr,
-                                 type_of(bcx.ccx(), expr_ty(bcx, x)).ptr_to()),
-          None => retptr
-        })
-      }
-      None => match bcx.fcx.llretptr {
+    let dest = match bcx.fcx.llretptr {
         None => expr::Ignore,
         Some(retptr) => expr::SaveIn(retptr),
-      }
     };
     match e {
-      Some(x) => {
-        bcx = expr::trans_into(bcx, x, dest);
-      }
-      _ => ()
+        Some(x) => {
+            bcx = expr::trans_into(bcx, x, dest);
+        }
+        _ => ()
     }
     cleanup_and_leave(bcx, None, Some(bcx.fcx.get_llreturn()));
     Unreachable(bcx);

src/librustc/middle/trans/expr.rs

@@ -697,8 +697,7 @@ fn trans_rvalue_dps_unadjusted(bcx: @mut Block, expr: @ast::expr,
                    expr_to_str(expr, tcx.sess.intr()),
                    expr_ty.repr(tcx));
             return closure::trans_expr_fn(bcx, sigil, decl, body,
-                                          expr.id, expr.id,
-                                          None, dest);
+                                          expr.id, expr.id, dest);
         }
         ast::expr_do_body(blk) => {
             return trans_into(bcx, blk, dest);

src/librustc/middle/typeck/check/regionck.rs

@@ -427,7 +427,7 @@ fn visit_expr(expr: @ast::expr, (rcx, v): (@mut Rcx, rvt)) {
         }
 
         ast::expr_fn_block(*) => {
-            check_expr_fn_block(rcx, expr, v, false);
+            check_expr_fn_block(rcx, expr, v);
         }
 
         ast::expr_loop(ref body, _) => {
@@ -454,8 +454,7 @@ fn visit_expr(expr: @ast::expr, (rcx, v): (@mut Rcx, rvt)) {
 
 fn check_expr_fn_block(rcx: @mut Rcx,
                        expr: @ast::expr,
-                       v: rvt,
-                       is_loop_body: bool) {
+                       v: rvt) {
     let tcx = rcx.fcx.tcx();
     match expr.node {
         ast::expr_fn_block(_, ref body) => {
@@ -464,7 +463,7 @@ fn check_expr_fn_block(rcx: @mut Rcx,
                 ty::ty_closure(
                     ty::ClosureTy {
                         sigil: ast::BorrowedSigil, region: region, _}) => {
-                    if get_freevars(tcx, expr.id).is_empty() && !is_loop_body {
+                    if get_freevars(tcx, expr.id).is_empty() {
                         // No free variables means that the environment
                         // will be NULL at runtime and hence the closure
                         // has static lifetime.