Make resolve recognize upvars

Upvars are now marked with def_upvar throughout, not just when going
through freevars::lookup_def. This makes things less error-prone. One
thing to watch out for is that def_upvar is used in `for each` bodies
too, when they refer to a local outside the body.

16 files changed, 221 insertions, 255 deletions

diff --git a/src/comp/driver/rustc.rs b/src/comp/driver/rustc.rs
index c281fc6a9b6..6096695eb2c 100644
--- a/src/comp/driver/rustc.rs
+++ b/src/comp/driver/rustc.rs
@@ -155,7 +155,7 @@ fn compile_input(sess: session::session, cfg: ast::crate_cfg, input: &istr,
              bind resolve::resolve_crate(sess, ast_map, crate));
     let freevars =
         time(time_passes, ~"freevar finding",
-             bind freevars::annotate_freevars(sess, def_map, crate));
+             bind freevars::annotate_freevars(def_map, crate));
     let ty_cx = ty::mk_ctxt(sess, def_map, ext_map, ast_map, freevars);
     time(time_passes, ~"typechecking",
          bind typeck::check_crate(ty_cx, crate));
@@ -240,7 +240,7 @@ fn pretty_print_input(sess: session::session, cfg: ast::crate_cfg,
         let amap = middle::ast_map::map_crate(*crate);
         let {def_map: def_map, ext_map: ext_map} =
             resolve::resolve_crate(sess, amap, crate);
-        let freevars = freevars::annotate_freevars(sess, def_map, crate);
+        let freevars = freevars::annotate_freevars(def_map, crate);
         let ty_cx = ty::mk_ctxt(sess, def_map, ext_map, amap, freevars);
         typeck::check_crate(ty_cx, crate);
         ann = {pre: ann_paren_for_expr, post: bind ann_typed_post(ty_cx, _)};
diff --git a/src/comp/middle/alias.rs b/src/comp/middle/alias.rs
index dd30cb8839c..f43ab93d4c1 100644
--- a/src/comp/middle/alias.rs
+++ b/src/comp/middle/alias.rs
@@ -51,7 +51,7 @@ fn check_crate(tcx: ty::ctxt, crate: &@ast::crate) {
     let cx = @{tcx: tcx,
                local_map: std::map::new_int_hash(),
                mutable next_local: 0u};
-    let v = @{visit_fn: bind visit_fn(cx, _, _, _, _, _, _, _),
+    let v = @{visit_fn: visit_fn,
               visit_expr: bind visit_expr(cx, _, _, _),
               visit_decl: bind visit_decl(cx, _, _, _)
               with *visit::default_visitor::<scope>()};
@@ -59,39 +59,15 @@ fn check_crate(tcx: ty::ctxt, crate: &@ast::crate) {
     tcx.sess.abort_if_errors();
 }
 
-fn visit_fn(cx: &@ctx, f: &ast::_fn, _tp: &[ast::ty_param], _sp: &span,
-            _name: &fn_ident, id: ast::node_id, sc: &scope, v: &vt<scope>) {
+fn visit_fn(f: &ast::_fn, _tp: &[ast::ty_param], _sp: &span,
+            _name: &fn_ident, _id: ast::node_id, sc: &scope, v: &vt<scope>) {
     visit::visit_fn_decl(f.decl, sc, v);
-    let scope =
-        alt f.proto {
-
-          // Blocks need to obey any restrictions from the enclosing scope.
-          ast::proto_block. {
-            sc
-          }
-
-          // Closures need to prohibit writing to any of the upvars.
-          // This doesn't seem like a particularly clean way to do this.
-          ast::proto_closure. {
-            let dnums = [];
-            for each nid in freevars::get_freevar_defs(cx.tcx, id).keys() {
-                dnums += [nid];
-            };
-            // I'm not sure if there is anything sensical to put here
-            @[@{root_var: none,
-                local_id: cx.next_local,
-                bindings: dnums,
-                unsafe_ty: none,
-                depends_on: [],
-                mutable ok: valid}]
-          }
-
-          // Non capturing functions start out fresh.
-          _ {
-            @[]
-          }
-        };
-
+    let scope = alt f.proto {
+      // Blocks need to obey any restrictions from the enclosing scope.
+      ast::proto_block. | ast::proto_closure. { sc }
+      // Non capturing functions start out fresh.
+      _ { @[] }
+    };
     v.visit_block(f.body, scope, v);
 }
 
@@ -489,7 +465,8 @@ fn ty_can_unsafely_include(cx: &ctx, needle: ty::t, haystack: ty::t,
 
 fn def_is_local(d: &ast::def, objfields_count: bool) -> bool {
     ret alt d {
-      ast::def_local(_) | ast::def_arg(_, _) | ast::def_binding(_) { true }
+      ast::def_local(_) | ast::def_arg(_, _) | ast::def_binding(_) |
+      ast::def_upvar(_, _, _) { true }
       ast::def_obj_field(_, _) { objfields_count }
       _ { false }
     };
diff --git a/src/comp/middle/freevars.rs b/src/comp/middle/freevars.rs
index 1a26fe476ef..a449c7e3439 100644
--- a/src/comp/middle/freevars.rs
+++ b/src/comp/middle/freevars.rs
@@ -15,23 +15,13 @@ import middle::resolve;
 import syntax::codemap::span;
 
 export annotate_freevars;
-export freevar_set;
 export freevar_map;
-export get_freevar_info;
 export get_freevars;
-export get_freevar_defs;
 export has_freevars;
-export is_freevar_of;
-export def_lookup;
 
-// Throughout the compiler, variables are generally dealt with using the
-// node_ids of the reference sites and not the def_id of the definition
-// site. Thus we store a set are the definitions along with a vec of one
-// "canonical" referencing node_id per free variable. The set is useful for
-// testing membership, the list of referencing sites is what you want for most
-// other things.
-type freevar_set = hashset<ast::node_id>;
-type freevar_info = {defs: freevar_set, refs: @[ast::node_id]};
+// A vector of defs representing the free variables referred to in a function.
+// (The def_upvar will already have been stripped).
+type freevar_info = @[ast::def];
 type freevar_map = hashmap<ast::node_id, freevar_info>;
 
 // Searches through part of the AST for all references to locals or
@@ -39,67 +29,54 @@ type freevar_map = hashmap<ast::node_id, freevar_info>;
 // Since we want to be able to collect upvars in some arbitrary piece
 // of the AST, we take a walker function that we invoke with a visitor
 // in order to start the search.
-fn collect_freevars(def_map: &resolve::def_map, sess: &session::session,
-                    walker: &fn(&visit::vt<()>),
-                    initial_decls: [ast::node_id]) -> freevar_info {
-    let decls = new_int_hash();
-    for decl: ast::node_id in initial_decls { set_add(decls, decl); }
+fn collect_freevars(def_map: &resolve::def_map,
+                    walker: &fn(&visit::vt<int>)) -> freevar_info {
+    let seen = new_int_hash();
     let refs = @mutable [];
 
-    let walk_fn =
-        lambda (f: &ast::_fn, _tps: &[ast::ty_param], _sp: &span,
-                _i: &ast::fn_ident, _nid: ast::node_id) {
-            for a: ast::arg in f.decl.inputs { set_add(decls, a.id); }
-        };
-    let walk_expr =
-        lambda (expr: &@ast::expr) {
-            alt expr.node {
-              ast::expr_path(path) {
-                if !def_map.contains_key(expr.id) {
-                    sess.span_fatal(expr.span,
-                                    ~"internal error in collect_freevars");
-                }
-                alt def_map.get(expr.id) {
-                  ast::def_arg(did, _) { *refs += [expr.id]; }
-                  ast::def_local(did) { *refs += [expr.id]; }
-                  ast::def_binding(did) { *refs += [expr.id]; }
-                  _ {/* no-op */ }
+    fn ignore_item(_i: &@ast::item, _depth: &int, _v: &visit::vt<int>) {}
+
+    let walk_expr = lambda(expr: &@ast::expr, depth: &int,
+                           v: &visit::vt<int>) {
+        alt expr.node {
+          ast::expr_fn(f) {
+            if f.proto == ast::proto_block ||
+               f.proto == ast::proto_closure {
+                visit::visit_expr(expr, depth + 1, v);
+            }
+          }
+          ast::expr_for_each(dcl, x, b) {
+            v.visit_local(dcl, depth, v);
+            v.visit_expr(x, depth, v);
+            v.visit_block(b, depth + 1, v);
+          }
+          ast::expr_path(path) {
+            let def = def_map.get(expr.id), i = 0;
+            while i < depth {
+                alt {def} {
+                  ast::def_upvar(_, inner, _) {
+                    def = *inner;
+                  }
+                  _ { break; }
                 }
-              }
-              _ { }
+                i += 1;
             }
-        };
-    let walk_local =
-        lambda (local: &@ast::local) {
-            for each b: @ast::pat in ast_util::pat_bindings(local.node.pat) {
-                set_add(decls, b.id);
+            if i == depth { // Made it to end of loop
+                let dnum = ast_util::def_id_of_def(def).node;
+                if !seen.contains_key(dnum) {
+                    *refs += [def];
+                    seen.insert(dnum, ());
+                }
             }
-        };
-    let walk_pat =
-        lambda (p: &@ast::pat) {
-            alt p.node { ast::pat_bind(_) { set_add(decls, p.id); } _ { } }
-        };
-
-    walker(visit::mk_simple_visitor(@{visit_local: walk_local,
-                                      visit_pat: walk_pat,
-                                      visit_expr: walk_expr,
-                                      visit_fn: walk_fn
-                                         with
-                                         *visit::default_simple_visitor()}));
-    // Calculate (refs - decls). This is the set of captured upvars.
-    // We build a vec of the node ids of the uses and a set of the
-    // node ids of the definitions.
-    let canonical_refs = [];
-    let defs = new_int_hash();
-    for ref_id_: ast::node_id in *refs {
-        let ref_id = ref_id_;
-        let def_id = ast_util::def_id_of_def(def_map.get(ref_id)).node;
-        if !decls.contains_key(def_id) && !defs.contains_key(def_id) {
-            canonical_refs += [ref_id];
-            set_add(defs, def_id);
+          }
+          _ { visit::visit_expr(expr, depth, v); }
         }
-    }
-    ret {defs: defs, refs: @canonical_refs};
+    };
+
+    walker(visit::mk_vt(@{visit_item: ignore_item,
+                          visit_expr: walk_expr
+                          with *visit::default_visitor()}));
+    ret @*refs;
 }
 
 // Build a map from every function and for-each body to a set of the
@@ -107,35 +84,30 @@ fn collect_freevars(def_map: &resolve::def_map, sess: &session::session,
 // efficient as it fully recomputes the free variables at every
 // node of interest rather than building up the free variables in
 // one pass. This could be improved upon if it turns out to matter.
-fn annotate_freevars(sess: &session::session, def_map: &resolve::def_map,
+fn annotate_freevars(def_map: &resolve::def_map,
                      crate: &@ast::crate) -> freevar_map {
     let freevars = new_int_hash();
 
-    let walk_fn =
-        lambda (f: &ast::_fn, tps: &[ast::ty_param], sp: &span,
-                i: &ast::fn_ident, nid: ast::node_id) {
-            let start_walk =
-                lambda (v: &visit::vt<()>) {
-                    v.visit_fn(f, tps, sp, i, nid, (), v);
-                };
-            let vars = collect_freevars(def_map, sess, start_walk, []);
-            freevars.insert(nid, vars);
-        };
-    let walk_expr =
-        lambda (expr: &@ast::expr) {
-            alt expr.node {
-              ast::expr_for_each(local, _, body) {
-                let start_walk =
-                    lambda (v: &visit::vt<()>) {
-                        v.visit_block(body, (), v);
-                    };
-                let bound = ast_util::pat_binding_ids(local.node.pat);
-                let vars = collect_freevars(def_map, sess, start_walk, bound);
-                freevars.insert(body.node.id, vars);
-              }
-              _ { }
-            }
+    let walk_fn = lambda (f: &ast::_fn, tps: &[ast::ty_param], sp: &span,
+                          i: &ast::fn_ident, nid: ast::node_id) {
+        let start_walk = lambda (v: &visit::vt<int>) {
+            v.visit_fn(f, tps, sp, i, nid, 1, v);
         };
+        let vars = collect_freevars(def_map, start_walk);
+        freevars.insert(nid, vars);
+    };
+    let walk_expr = lambda (expr: &@ast::expr) {
+        alt expr.node {
+          ast::expr_for_each(local, _, body) {
+            let start_walk = lambda (v: &visit::vt<int>) {
+                v.visit_block(body, 1, v);
+            };
+            let vars = collect_freevars(def_map, start_walk);
+            freevars.insert(body.node.id, vars);
+          }
+          _ { }
+        }
+    };
 
     let visitor =
         visit::mk_simple_visitor(@{visit_fn: walk_fn, visit_expr: walk_expr
@@ -145,7 +117,7 @@ fn annotate_freevars(sess: &session::session, def_map: &resolve::def_map,
     ret freevars;
 }
 
-fn get_freevar_info(tcx: &ty::ctxt, fid: ast::node_id) -> freevar_info {
+fn get_freevars(tcx: &ty::ctxt, fid: ast::node_id) -> freevar_info {
     alt tcx.freevars.find(fid) {
       none. {
         fail "get_freevars: " + istr::to_estr(int::str(fid))
@@ -154,31 +126,9 @@ fn get_freevar_info(tcx: &ty::ctxt, fid: ast::node_id) -> freevar_info {
       some(d) { ret d; }
     }
 }
-fn get_freevar_defs(tcx: &ty::ctxt, fid: ast::node_id) -> freevar_set {
-    ret get_freevar_info(tcx, fid).defs;
-}
-fn get_freevars(tcx: &ty::ctxt, fid: ast::node_id) -> @[ast::node_id] {
-    ret get_freevar_info(tcx, fid).refs;
-}
 fn has_freevars(tcx: &ty::ctxt, fid: ast::node_id) -> bool {
-    ret get_freevar_defs(tcx, fid).size() != 0u;
+    ret std::vec::len(*get_freevars(tcx, fid)) != 0u;
 }
-fn is_freevar_of(tcx: &ty::ctxt, def: ast::node_id, f: ast::node_id) -> bool {
-    ret get_freevar_defs(tcx, f).contains_key(def);
-}
-fn def_lookup(tcx: &ty::ctxt, f: ast::node_id, id: ast::node_id) ->
-   option::t<ast::def> {
-    alt tcx.def_map.find(id) {
-      none. { ret none; }
-      some(d) {
-        let did = ast_util::def_id_of_def(d);
-        if f != -1 && is_freevar_of(tcx, did.node, f) {
-            ret some(ast::def_upvar(did, @d));
-        } else { ret some(d); }
-      }
-    }
-}
-
 
 // Local Variables:
 // mode: rust
diff --git a/src/comp/middle/mut.rs b/src/comp/middle/mut.rs
index cb7a85639ec..b2420252efa 100644
--- a/src/comp/middle/mut.rs
+++ b/src/comp/middle/mut.rs
@@ -238,14 +238,18 @@ fn check_call(cx: &@ctx, f: &@expr, args: &[@expr]) {
 }
 
 fn is_immutable_def(def: &def) -> option::t<istr> {
-    ret alt def {
+    alt def {
       def_fn(_, _) | def_mod(_) | def_native_mod(_) | def_const(_) |
       def_use(_) { some(~"static item") }
       def_obj_field(_, imm.) { some(~"immutable object field") }
       def_arg(_, alias(false)) { some(~"immutable alias") }
+      def_upvar(_, inner, mut) {
+        if !mut { some(~"upvar") }
+        else { is_immutable_def(*inner) }
+      }
       def_binding(_) { some(~"binding") }
       _ { none }
-    };
+    }
 }
 
 // Local Variables:
diff --git a/src/comp/middle/resolve.rs b/src/comp/middle/resolve.rs
index 61bddcf4b94..ca688f533ec 100644
--- a/src/comp/middle/resolve.rs
+++ b/src/comp/middle/resolve.rs
@@ -55,7 +55,7 @@ tag scope {
     scope_item(@ast::item);
     scope_fn(ast::fn_decl, ast::proto, [ast::ty_param]);
     scope_native_item(@ast::native_item);
-    scope_loop(@ast::local); // there's only 1 decl per loop.
+    scope_loop(@ast::local, bool); // there's only 1 decl per loop.
 
     scope_block(ast::blk, @mutable uint, @mutable uint);
     scope_arm(ast::arm);
@@ -404,14 +404,12 @@ fn visit_arm_with_scope(a: &ast::arm, sc: &scopes, v: &vt<scopes>) {
 fn visit_expr_with_scope(x: &@ast::expr, sc: &scopes, v: &vt<scopes>) {
     alt x.node {
       ast::expr_for(decl, coll, blk) | ast::expr_for_each(decl, coll, blk) {
-        let new_sc = cons::<scope>(scope_loop(decl), @sc);
+        let f_e = alt x.node { expr_for_each(_, _, _) { true } _ { false } };
+        let new_sc = cons(scope_loop(decl, f_e), @sc);
         v.visit_expr(coll, sc, v);
         v.visit_local(decl, new_sc, v);
         v.visit_block(blk, new_sc, v);
       }
-      ast::expr_fn(f) {
-        visit::visit_expr(x, cons(scope_fn(f.decl, f.proto, []), @sc), v);
-      }
       _ { visit::visit_expr(x, sc, v); }
     }
 }
@@ -622,9 +620,18 @@ fn scope_is_fn(sc: &scope) -> bool {
         };
 }
 
+fn scope_closes(sc: &scope) -> option::t<bool> {
+    alt sc {
+      scope_fn(_, ast::proto_block., _) | scope_loop(_, true) { some(true) }
+      scope_fn(_, ast::proto_closure., _) { some(false) }
+      _ { none }
+    }
+}
+
 fn def_is_local(d: &def) -> bool {
     ret alt d {
-      ast::def_arg(_, _) | ast::def_local(_) | ast::def_binding(_) { true }
+      ast::def_arg(_, _) | ast::def_local(_) | ast::def_binding(_) |
+      ast::def_upvar(_, _, _) { true }
       _ { false }
     };
 }
@@ -675,7 +682,7 @@ fn lookup_in_scope(e: &env, sc: scopes, sp: &span, name: &ident,
           scope_fn(decl, _, ty_params) {
             ret lookup_in_fn(name, decl, ty_params, ns);
           }
-          scope_loop(local) {
+          scope_loop(local, _) {
             if ns == ns_value {
                 alt lookup_in_pat(name, local.node.pat) {
                   some(did) { ret some(ast::def_binding(did)); }
@@ -698,8 +705,8 @@ fn lookup_in_scope(e: &env, sc: scopes, sp: &span, name: &ident,
         ret none::<def>;
     }
     let left_fn = false;
+    let closing = [];
     // Used to determine whether obj fields are in scope
-
     let left_fn_level2 = false;
     while true {
         alt { sc } {
@@ -708,27 +715,38 @@ fn lookup_in_scope(e: &env, sc: scopes, sp: &span, name: &ident,
             let fnd = in_scope(e, sp, name, hd, ns);
             if !is_none(fnd) {
                 let df = option::get(fnd);
-                if left_fn && def_is_local(df) ||
+                let local = def_is_local(df);
+                if left_fn && local ||
                        left_fn_level2 && def_is_obj_field(df) ||
                        scope_is_fn(hd) && left_fn && def_is_ty_arg(df) {
-                    let msg =
-                        alt ns {
-                          ns_type. {
-                            ~"Attempt to use a type \
-                                argument out of scope"
-                          }
-                          _ {
-                            ~"attempted dynamic \
-                                       environment-capture"
-                          }
-                        };
+                    let msg = alt ns {
+                      ns_type. {
+                        ~"Attempt to use a type argument out of scope"
+                      }
+                      _ {
+                        ~"attempted dynamic environment-capture"
+                      }
+                    };
                     e.sess.span_fatal(sp, msg);
+                } else if local {
+                    let i = vec::len(closing);
+                    while i > 0u {
+                        i -= 1u;
+                        df = ast::def_upvar(ast_util::def_id_of_def(df),
+                                            @df, closing[i]);
+                        fnd = some(df);
+                    }
                 }
                 ret fnd;
             }
-            if left_fn { left_fn_level2 = true; }
-            if (ns == ns_value || ns == ns_type) && !left_fn {
+            if left_fn {
+                left_fn_level2 = true;
+            } else if ns == ns_value || ns == ns_type {
                 left_fn = scope_is_fn(hd);
+                alt scope_closes(hd) {
+                  some(mut) { closing += [mut]; }
+                  _ {}
+                }
             }
             sc = *tl;
           }
@@ -1177,6 +1195,7 @@ fn ns_for_def(d: def) -> namespace {
           ast::def_const(_) { ns_value }
           ast::def_arg(_, _) { ns_value }
           ast::def_local(_) { ns_value }
+          ast::def_upvar(_, _, _) { ns_value }
           ast::def_variant(_, _) { ns_value }
           ast::def_ty(_) { ns_type }
           ast::def_binding(_) { ns_type }
diff --git a/src/comp/middle/trans.rs b/src/comp/middle/trans.rs
index 8b0cfe19501..699c23d4fa2 100644
--- a/src/comp/middle/trans.rs
+++ b/src/comp/middle/trans.rs
@@ -2824,8 +2824,8 @@ fn build_environment(bcx: @block_ctxt, lltydescs: [ValueRef],
 
 // Given a context and a list of upvars, build a closure. This just
 // collects the upvars and packages them up for build_environment.
-fn build_closure(cx: &@block_ctxt, upvars: &@[ast::node_id], copying: bool) ->
-   {ptr: ValueRef, ptrty: ty::t, bcx: @block_ctxt} {
+fn build_closure(cx: &@block_ctxt, upvars: &@[ast::def], copying: bool)
+    -> {ptr: ValueRef, ptrty: ty::t, bcx: @block_ctxt} {
     let closure_vals: [lval_result] = [];
     let closure_tys: [ty::t] = [];
     // If we need to, package up the iterator body to call
@@ -2834,8 +2834,9 @@ fn build_closure(cx: &@block_ctxt, upvars: &@[ast::node_id], copying: bool) ->
         closure_tys += [option::get(cx.fcx.iterbodyty)];
     }
     // Package up the upvars
-    for nid: ast::node_id in *upvars {
-        closure_vals += [trans_var(cx, cx.sp, nid)];
+    for def in *upvars {
+        closure_vals += [trans_local_var(cx, def)];
+        let nid = ast_util::def_id_of_def(def).node;
         let ty = ty::node_id_to_monotype(bcx_tcx(cx), nid);
         if !copying { ty = ty::mk_mut_ptr(bcx_tcx(cx), ty); }
         closure_tys += [ty];
@@ -2882,7 +2883,7 @@ fn find_environment_tydescs(bcx: &@block_ctxt, envty: ty::t,
 // and a list of upvars, generate code to load and populate the environment
 // with the upvars and type descriptors.
 fn load_environment(enclosing_cx: &@block_ctxt, fcx: &@fn_ctxt, envty: ty::t,
-                    upvars: &@[ast::node_id], copying: bool) {
+                    upvars: &@[ast::def], copying: bool) {
     let bcx = new_raw_block_ctxt(fcx, fcx.llcopyargs);
 
     let ty = ty::mk_imm_box(bcx_tcx(bcx), envty);
@@ -2913,14 +2914,13 @@ fn load_environment(enclosing_cx: &@block_ctxt, fcx: &@fn_ctxt, envty: ty::t,
         i += 1u;
     }
 
-    // Load the acutal upvars.
-    for upvar_id: ast::node_id in *upvars {
+    // Load the actual upvars.
+    for upvar_def in *upvars {
         let upvarptr = GEP_tup_like(bcx, ty, llclosure, path + [i as int]);
         bcx = upvarptr.bcx;
         let llupvarptr = upvarptr.val;
         if !copying { llupvarptr = Load(bcx, llupvarptr); }
-        let def_id = ast_util::def_id_of_def(bcx_tcx(bcx).
-                                             def_map.get(upvar_id));
+        let def_id = ast_util::def_id_of_def(upvar_def);
         fcx.llupvars.insert(def_id.node, llupvarptr);
         i += 1u;
     }
@@ -3134,34 +3134,44 @@ fn lookup_discriminant(lcx: &@local_ctxt, vid: &ast::def_id) -> ValueRef {
     }
 }
 
-fn trans_var(cx: &@block_ctxt, sp: &span, id: ast::node_id) -> lval_result {
-    let ccx = bcx_ccx(cx);
-    alt freevars::def_lookup(bcx_tcx(cx), cx.fcx.id, id) {
-      some(ast::def_upvar(did, _)) {
+fn trans_local_var(cx: &@block_ctxt, def: &ast::def) -> lval_result {
+    alt def {
+      ast::def_upvar(did, _, _) {
         assert (cx.fcx.llupvars.contains_key(did.node));
         ret lval_mem(cx, cx.fcx.llupvars.get(did.node));
       }
-      some(ast::def_arg(did, _)) {
+      ast::def_arg(did, _) {
         assert (cx.fcx.llargs.contains_key(did.node));
         ret lval_mem(cx, cx.fcx.llargs.get(did.node));
       }
-      some(ast::def_local(did)) {
+      ast::def_local(did) {
         assert (cx.fcx.lllocals.contains_key(did.node));
         ret lval_mem(cx, cx.fcx.lllocals.get(did.node));
       }
-      some(ast::def_binding(did)) {
+      ast::def_binding(did) {
         assert (cx.fcx.lllocals.contains_key(did.node));
         ret lval_mem(cx, cx.fcx.lllocals.get(did.node));
       }
-      some(ast::def_obj_field(did, _)) {
+      ast::def_obj_field(did, _) {
         assert (cx.fcx.llobjfields.contains_key(did.node));
         ret lval_mem(cx, cx.fcx.llobjfields.get(did.node));
       }
-      some(ast::def_fn(did, _)) {
+      _ {
+        bcx_ccx(cx).sess.span_unimpl
+            (cx.sp, ~"unsupported def type in trans_local_def");
+      }
+    }
+}
+
+fn trans_var(cx: &@block_ctxt, sp: &span, def: &ast::def,
+             id: ast::node_id) -> lval_result {
+    let ccx = bcx_ccx(cx);
+    alt def {
+      ast::def_fn(did, _) {
         let tyt = ty::lookup_item_type(ccx.tcx, did);
         ret lval_generic_fn(cx, tyt, did, id);
       }
-      some(ast::def_variant(tid, vid)) {
+      ast::def_variant(tid, vid) {
         let v_tyt = ty::lookup_item_type(ccx.tcx, vid);
         alt ty::struct(ccx.tcx, v_tyt.ty) {
           ty::ty_fn(_, _, _, _, _) {
@@ -3188,7 +3198,7 @@ fn trans_var(cx: &@block_ctxt, sp: &span, id: ast::node_id) -> lval_result {
           }
         }
       }
-      some(ast::def_const(did)) {
+      ast::def_const(did) {
         if did.crate == ast::local_crate {
             assert (ccx.consts.contains_key(did.node));
             ret lval_mem(cx, ccx.consts.get(did.node));
@@ -3203,17 +3213,17 @@ fn trans_var(cx: &@block_ctxt, sp: &span, id: ast::node_id) -> lval_result {
                                            tp));
         }
       }
-      some(ast::def_native_fn(did)) {
+      ast::def_native_fn(did) {
         let tyt = ty::lookup_item_type(ccx.tcx, did);
         ret lval_generic_fn(cx, tyt, did, id);
       }
-      _ { ccx.sess.span_unimpl(cx.sp, ~"def variant in trans"); }
+      _ { ret trans_local_var(cx, def); }
     }
 }
 
 fn trans_path(cx: &@block_ctxt, p: &ast::path, id: ast::node_id) ->
    lval_result {
-    ret trans_var(cx, p.span, id);
+    ret trans_var(cx, p.span, bcx_tcx(cx).def_map.get(id), id);
 }
 
 fn trans_field(cx: &@block_ctxt, sp: &span, v: ValueRef, t0: ty::t,
diff --git a/src/comp/middle/tstate/auxiliary.rs b/src/comp/middle/tstate/auxiliary.rs
index be5d149135d..4f4af857374 100644
--- a/src/comp/middle/tstate/auxiliary.rs
+++ b/src/comp/middle/tstate/auxiliary.rs
@@ -531,16 +531,6 @@ fn constraints_expr(cx: &ty::ctxt, e: @expr) -> [@ty::constr] {
     }
 }
 
-fn node_id_to_def_upvar_strict(cx: &fn_ctxt, id: node_id) -> def {
-    alt freevars::def_lookup(cx.ccx.tcx, cx.id, id) {
-      none. {
-        log_err "node_id_to_def: node_id "
-            + istr::to_estr(int::str(id)) + " has no def";
-        fail;
-      }
-      some(d) { ret d; }
-    }
-}
 fn node_id_to_def_strict(cx: &ty::ctxt, id: node_id) -> def {
     alt cx.def_map.find(id) {
       none. {
@@ -555,9 +545,6 @@ fn node_id_to_def_strict(cx: &ty::ctxt, id: node_id) -> def {
 fn node_id_to_def(ccx: &crate_ctxt, id: node_id) -> option::t<def> {
     ret ccx.tcx.def_map.find(id);
 }
-fn node_id_to_def_upvar(cx: &fn_ctxt, id: node_id) -> option::t<def> {
-    ret freevars::def_lookup(cx.ccx.tcx, cx.id, id);
-}
 
 fn norm_a_constraint(id: def_id, c: &constraint) -> [norm_constraint] {
     alt c {
@@ -620,21 +607,11 @@ fn expr_to_constr_arg(tcx: ty::ctxt, e: &@expr) -> @constr_arg_use {
     alt e.node {
       expr_path(p) {
         alt tcx.def_map.find(e.id) {
-          some(def_local(l_id)) {
-            ret @respan(p.span,
-                        carg_ident({ident: p.node.idents[0],
-                                    node: l_id.node}));
-          }
-          some(def_arg(a_id, _)) {
+          some(def_local(id)) | some(def_arg(id, _)) | some(def_binding(id)) |
+          some(def_upvar(id, _, _)) {
             ret @respan(p.span,
-                        carg_ident({ident: p.node.idents[0],
-                                    node: a_id.node}));
+                        carg_ident({ident: p.node.idents[0], node: id.node}));
           }
-          some (def_binding(b_id)) {
-            ret @respan(p.span,
-                        carg_ident({ident: p.node.idents[0],
-                                    node: b_id.node}));
-           }
           some(_) {
             tcx.sess.bug(~"exprs_to_constr_args: non-local variable " +
                              ~"as pred arg");
@@ -848,8 +825,9 @@ tag if_ty { if_check; plain_if; }
 fn local_node_id_to_def_id_strict(fcx: &fn_ctxt, sp: &span, i: &node_id) ->
    def_id {
     alt local_node_id_to_def(fcx, i) {
-      some(def_local(d_id)) { ret d_id; }
-      some(def_arg(a_id, _)) { ret a_id; }
+      some(def_local(id)) | some(def_arg(id, _)) | some(def_upvar(id, _, _)) {
+        ret id;
+      }
       some(_) {
         fcx.ccx.tcx.sess.span_fatal(sp,
                                     ~"local_node_id_to_def_id: id \
@@ -870,17 +848,16 @@ fn local_node_id_to_def(fcx: &fn_ctxt, i: &node_id) -> option::t<def> {
 
 fn local_node_id_to_def_id(fcx: &fn_ctxt, i: &node_id) -> option::t<def_id> {
     alt local_node_id_to_def(fcx, i) {
-      some(def_local(d_id)) { some(d_id) }
-      some(def_arg(a_id, _)) { some(a_id) }
+      some(def_local(id)) | some(def_arg(id, _)) | some(def_binding(id)) |
+      some(def_upvar(id, _, _)) { some(id) }
       _ { none }
     }
 }
 
 fn local_node_id_to_local_def_id(fcx: &fn_ctxt, i: &node_id) ->
    option::t<node_id> {
-    alt local_node_id_to_def(fcx, i) {
-      some(def_local(d_id)) { some(d_id.node) }
-      some(def_arg(a_id, _)) { some(a_id.node) }
+    alt local_node_id_to_def_id(fcx, i) {
+      some(did) { some(did.node) }
       _ { none }
     }
 }
diff --git a/src/comp/middle/tstate/pre_post_conditions.rs b/src/comp/middle/tstate/pre_post_conditions.rs
index ce53a204df9..189b5249143 100644
--- a/src/comp/middle/tstate/pre_post_conditions.rs
+++ b/src/comp/middle/tstate/pre_post_conditions.rs
@@ -210,7 +210,7 @@ fn join_then_else(fcx: &fn_ctxt, antec: &@expr, conseq: &blk,
 
 fn gen_if_local(fcx: &fn_ctxt, lhs: @expr, rhs: @expr, larger_id: node_id,
                 new_var: node_id, pth: &path) {
-    alt node_id_to_def_upvar(fcx, new_var) {
+    alt node_id_to_def(fcx.ccx, new_var) {
       some(d) {
         alt d {
           def_local(d_id) {
@@ -249,7 +249,7 @@ fn handle_update(fcx: &fn_ctxt, parent: &@expr, lhs: &@expr, rhs: &@expr,
           }
           _ {
             // pure and assign_op require the lhs to be init'd
-            let df = node_id_to_def_upvar_strict(fcx, lhs.id);
+            let df = node_id_to_def_strict(fcx.ccx.tcx, lhs.id);
             alt df {
               def_local(d_id) {
                 let i =
@@ -291,13 +291,16 @@ fn handle_update(fcx: &fn_ctxt, parent: &@expr, lhs: &@expr, rhs: &@expr,
     }
 }
 
-/* FIXME: Can't deinitialize an upvar -- tests for that? */
 fn handle_var(fcx: &fn_ctxt, rslt: &pre_and_post, id: node_id, name: ident) {
-    let df = node_id_to_def_upvar_strict(fcx, id);
-    alt df {
+    handle_var_def(fcx, rslt, node_id_to_def_strict(fcx.ccx.tcx, id), name);
+}
+
+fn handle_var_def(fcx: &fn_ctxt, rslt: &pre_and_post, def: &def,
+                  name: ident) {
+    alt def {
       def_local(d_id) | def_arg(d_id, _) {
-        let i = bit_num(fcx, ninit(d_id.node, name));
         use_var(fcx, d_id.node);
+        let i = bit_num(fcx, ninit(d_id.node, name));
         require_and_preserve(i, rslt);
       }
       _ {/* nothing to check */ }
@@ -369,8 +372,9 @@ fn find_pre_post_expr(fcx: &fn_ctxt, e: @expr) {
       expr_fn(f) {
         let rslt = expr_pp(fcx.ccx, e);
         clear_pp(rslt);
-        let upvars = freevars::get_freevars(fcx.ccx.tcx, e.id);
-        for id: node_id in *upvars { handle_var(fcx, rslt, id, ~"upvar"); }
+        for def in *freevars::get_freevars(fcx.ccx.tcx, e.id) {
+            handle_var_def(fcx, rslt, def, ~"upvar");
+        }
       }
       expr_block(b) {
         find_pre_post_block(fcx, b);
@@ -474,6 +478,11 @@ fn find_pre_post_expr(fcx: &fn_ctxt, e: @expr) {
       }
       expr_for_each(d, index, body) {
         find_pre_post_loop(fcx, d, index, body, e.id);
+        let rslt = expr_pp(fcx.ccx, e);
+        clear_pp(rslt);
+        for def in *freevars::get_freevars(fcx.ccx.tcx, body.node.id) {
+            handle_var_def(fcx, rslt, def, ~"upvar");
+        }
       }
       expr_index(val, sub) { find_pre_post_exprs(fcx, [val, sub], e.id); }
       expr_alt(ex, alts) {
diff --git a/src/comp/middle/tstate/states.rs b/src/comp/middle/tstate/states.rs
index 915a9d6b941..fc65961d692 100644
--- a/src/comp/middle/tstate/states.rs
+++ b/src/comp/middle/tstate/states.rs
@@ -228,7 +228,7 @@ fn find_pre_post_state_loop(fcx: &fn_ctxt, pres: prestate, l: &@local,
 fn gen_if_local(fcx: &fn_ctxt, p: &poststate, e: &@expr) -> bool {
     alt e.node {
       expr_path(pth) {
-        alt freevars::def_lookup(fcx.ccx.tcx, fcx.id, e.id) {
+        alt fcx.ccx.tcx.def_map.find(e.id) {
           some(def_local(loc)) {
             ret set_in_poststate_ident(fcx, loc.node,
                                        path_to_ident(fcx.ccx.tcx, pth), p);
diff --git a/src/comp/middle/ty.rs b/src/comp/middle/ty.rs
index fc728f9ea65..f24eee8123c 100644
--- a/src/comp/middle/ty.rs
+++ b/src/comp/middle/ty.rs
@@ -1790,11 +1790,7 @@ mod unify {
     tag union_result { unres_ok; unres_err(type_err); }
     tag fixup_result {
         fix_ok(t); // fixup succeeded
-
-
-
         fix_err(int); // fixup failed because a type variable was unresolved
-
     }
     type var_bindings =
         {sets: ufind::ufind, types: smallintmap::smallintmap<t>};
@@ -2605,6 +2601,7 @@ fn def_has_ty_params(def: &ast::def) -> bool {
       ast::def_const(_) { ret false; }
       ast::def_arg(_, _) { ret false; }
       ast::def_local(_) { ret false; }
+      ast::def_upvar(_, _, _) { ret false; }
       ast::def_variant(_, _) { ret true; }
       ast::def_ty(_) { ret false; }
       ast::def_ty_arg(_, _) { ret false; }
diff --git a/src/comp/middle/typeck.rs b/src/comp/middle/typeck.rs
index 9d44afdbfbd..21137bdb652 100644
--- a/src/comp/middle/typeck.rs
+++ b/src/comp/middle/typeck.rs
@@ -148,6 +148,9 @@ fn ty_param_kinds_and_ty_for_def(fcx: &@fn_ctxt, sp: &span, defn: &ast::def)
       ast::def_ty(_) {
         fcx.ccx.tcx.sess.span_fatal(sp, ~"expected value but found type");
       }
+      ast::def_upvar(_, inner, _) {
+        ret ty_param_kinds_and_ty_for_def(fcx, sp, *inner);
+      }
       _ {
         // FIXME: handle other names.
         fcx.ccx.tcx.sess.unimpl(~"definition variant");
@@ -1128,6 +1131,14 @@ mod writeback {
     fn visit_expr(e: &@ast::expr, wbcx: &wb_ctxt, v: &wb_vt) {
         if !wbcx.success { ret; }
         resolve_type_vars_for_node(wbcx, e.span, e.id);
+        alt e.node {
+          ast::expr_fn(f) {
+            for input in f.decl.inputs {
+                resolve_type_vars_for_node(wbcx, e.span, input.id);
+            }
+          }
+          _ {}
+        }
         visit::visit_expr(e, wbcx, v);
     }
     fn visit_block(b: &ast::blk, wbcx: &wb_ctxt, v: &wb_vt) {
@@ -2683,6 +2694,7 @@ fn check_fn(ccx: &@crate_ctxt, f: &ast::_fn, id: &ast::node_id,
           next_var_id: gather_result.next_var_id,
           mutable fixups: fixups,
           ccx: ccx};
+
     check_constraints(fcx, decl.constraints, decl.inputs);
     check_block(fcx, body);
 
@@ -2700,6 +2712,13 @@ fn check_fn(ccx: &@crate_ctxt, f: &ast::_fn, id: &ast::node_id,
       none. {}
     }
 
+    let args = ty::ty_fn_args(ccx.tcx, ty::node_id_to_type(ccx.tcx, id));
+    let i = 0u;
+    for arg: ty::arg in args {
+        write::ty_only_fixup(fcx, f.decl.inputs[i].id, arg.ty);
+        i += 1u;
+    }
+
     // If we don't have any enclosing function scope, it is time to
     // force any remaining type vars to be resolved.
     // If we have an enclosing function scope, our type variables will be
diff --git a/src/comp/syntax/ast.rs b/src/comp/syntax/ast.rs
index f19bad3b478..582da17f465 100644
--- a/src/comp/syntax/ast.rs
+++ b/src/comp/syntax/ast.rs
@@ -44,11 +44,7 @@ tag def {
     def_use(def_id);
     def_native_ty(def_id);
     def_native_fn(def_id);
-
-    /* A "fake" def for upvars. This never appears in the def_map, but
-     * freevars::def_lookup will return it for a def that is an upvar.
-     * It contains the actual def. */
-    def_upvar(def_id, @def);
+    def_upvar(def_id, @def, bool /* writable */);
 }
 
 // The set of meta_items that define the compilation environment of the crate,
diff --git a/src/comp/syntax/ast_util.rs b/src/comp/syntax/ast_util.rs
index 2bad0d1c403..1052ef195fa 100644
--- a/src/comp/syntax/ast_util.rs
+++ b/src/comp/syntax/ast_util.rs
@@ -42,7 +42,7 @@ fn def_id_of_def(d: def) -> def_id {
       def_use(id) { ret id; }
       def_native_ty(id) { ret id; }
       def_native_fn(id) { ret id; }
-      def_upvar(id, _) { ret id; }
+      def_upvar(id, _, _) { ret id; }
     }
 }
 
@@ -216,3 +216,12 @@ fn ternary_to_if(e: &@expr) -> @expr {
       _ { fail; }
     }
 }
+
+// Local Variables:
+// mode: rust
+// fill-column: 78;
+// indent-tabs-mode: nil
+// c-basic-offset: 4
+// buffer-file-coding-system: utf-8-unix
+// compile-command: "make -k -C $RBUILD 2>&1 | sed -e 's/\\/x\\//x:\\//g'";
+// End:
diff --git a/src/comp/syntax/parse/parser.rs b/src/comp/syntax/parse/parser.rs
index 94f2dd3c99f..369e07080f4 100644
--- a/src/comp/syntax/parse/parser.rs
+++ b/src/comp/syntax/parse/parser.rs
@@ -173,7 +173,6 @@ fn bad_expr_word_table() -> hashmap<istr, ()> {
     words.insert(~"prove", ());
     words.insert(~"native", ());
     words.insert(~"fn", ());
-    words.insert(~"block", ());
     words.insert(~"lambda", ());
     words.insert(~"pure", ());
     words.insert(~"iter", ());
diff --git a/src/test/compile-fail/lambda-mutate-nested.rs b/src/test/compile-fail/lambda-mutate-nested.rs
index 409b0e59950..05a3cc95e20 100644
--- a/src/test/compile-fail/lambda-mutate-nested.rs
+++ b/src/test/compile-fail/lambda-mutate-nested.rs
@@ -1,4 +1,4 @@
-// error-pattern:assigning to immutable alias
+// error-pattern:assigning to upvar
 // Make sure that nesting a block within a lambda doesn't let us
 // mutate upvars from a lambda.
 fn main() {
diff --git a/src/test/compile-fail/lambda-mutate.rs b/src/test/compile-fail/lambda-mutate.rs
index cb891aad374..3d7199e4b1d 100644
--- a/src/test/compile-fail/lambda-mutate.rs
+++ b/src/test/compile-fail/lambda-mutate.rs
@@ -1,4 +1,4 @@
-// error-pattern:assigning to immutable alias
+// error-pattern:assigning to upvar
 // Make sure we can't write to upvars from lambdas
 fn main() {
     let i = 0;