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.

Author: marijnh

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, _)};

src/comp/middle/alias.rs

@@ -51,47 +51,23 @@ 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>()};
     visit::visit_crate(*crate, @[], visit::mk_vt(v));
     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 }
     };

src/comp/middle/freevars.rs

@@ -15,127 +15,99 @@ 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
 // upvars in this frame and returns the list of definition IDs thus found.
 // 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
 // freevars contained in it. The implementation is not particularly
 // 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

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: