check if the type of a local variable contains illegal region

also, try to suppress some of the duplicate errors---in general
more work is needed to make these error messages comprehensible
to ordinary humans

Author: nikomatsakis

src/rustc/middle/borrowck.rs

@@ -465,7 +465,7 @@ impl methods for check_loan_ctxt {
     fn walk_loans(scope_id: ast::node_id,
                   f: fn(loan) -> bool) {
         let mut scope_id = scope_id;
-        let parents = self.tcx().region_map.parents;
+        let region_map = self.tcx().region_map;
         let req_loan_map = self.req_loan_map;
 
         loop {
@@ -477,7 +477,7 @@ impl methods for check_loan_ctxt {
                 }
             }
 
-            alt parents.find(scope_id) {
+            alt region_map.find(scope_id) {
               none { ret; }
               some(next_scope_id) { scope_id = next_scope_id; }
             }
@@ -570,7 +570,7 @@ impl methods for check_loan_ctxt {
             some(loanss) { loanss }
         };
 
-        let par_scope_id = self.tcx().region_map.parents.get(scope_id);
+        let par_scope_id = self.tcx().region_map.get(scope_id);
         for self.walk_loans(par_scope_id) { |old_loan|
             for (*new_loanss).each { |new_loans|
                 for (*new_loans).each { |new_loan|

src/rustc/middle/region.rs

@@ -151,20 +151,16 @@ type binding = {node_id: ast::node_id,
                 name: str,
                 br: ty::bound_region};
 
-type region_map = {
-    // Mapping from a block/function expression to its parent.
-    parents: hashmap<ast::node_id,ast::node_id>,
-
-    // Mapping from arguments and local variables to the block in
-    // which they are declared. Arguments are considered to be declared
-    // within the body of the function.
-    local_blocks: hashmap<ast::node_id,ast::node_id>
-};
+// Mapping from a block/expr/binding to the innermost scope that
+// bounds its lifetime.  For a block/expression, this is the lifetime
+// in which it will be evaluated.  For a binding, this is the lifetime
+// in which is in scope.
+type region_map = hashmap<ast::node_id, ast::node_id>;
 
 type ctxt = {
     sess: session,
     def_map: resolve::def_map,
-    region_map: @region_map,
+    region_map: region_map,
 
     // These two fields (parent and closure_parent) specify the parent
     // scope of the current expression.  The parent scope is the
@@ -207,27 +203,27 @@ type ctxt = {
 
 // Returns true if `subscope` is equal to or is lexically nested inside
 // `superscope` and false otherwise.
-fn scope_contains(region_map: @region_map, superscope: ast::node_id,
+fn scope_contains(region_map: region_map, superscope: ast::node_id,
                   subscope: ast::node_id) -> bool {
     let mut subscope = subscope;
     while superscope != subscope {
-        alt region_map.parents.find(subscope) {
+        alt region_map.find(subscope) {
             none { ret false; }
             some(scope) { subscope = scope; }
         }
     }
     ret true;
 }
 
-fn nearest_common_ancestor(region_map: @region_map, scope_a: ast::node_id,
+fn nearest_common_ancestor(region_map: region_map, scope_a: ast::node_id,
                            scope_b: ast::node_id) -> option<ast::node_id> {
 
-    fn ancestors_of(region_map: @region_map, scope: ast::node_id)
+    fn ancestors_of(region_map: region_map, scope: ast::node_id)
                     -> [ast::node_id] {
         let mut result = [scope];
         let mut scope = scope;
         loop {
-            alt region_map.parents.find(scope) {
+            alt region_map.find(scope) {
                 none { ret result; }
                 some(superscope) {
                     result += [superscope];
@@ -285,7 +281,7 @@ fn record_parent(cx: ctxt, child_id: ast::node_id) {
       none { /* no-op */ }
       some(parent_id) {
         #debug["parent of node %d is node %d", child_id, parent_id];
-        cx.region_map.parents.insert(child_id, parent_id);
+        cx.region_map.insert(child_id, parent_id);
       }
     }
 }
@@ -314,8 +310,7 @@ fn resolve_pat(pat: @ast::pat, cx: ctxt, visitor: visit::vt<ctxt>) {
           }
           _ {
             /* This names a local. Bind it to the containing scope. */
-            let local_blocks = cx.region_map.local_blocks;
-            local_blocks.insert(pat.id, parent_id(cx, pat.span));
+            record_parent(cx, pat.id);
           }
         }
       }
@@ -357,8 +352,7 @@ fn resolve_expr(expr: @ast::expr, cx: ctxt, visitor: visit::vt<ctxt>) {
 }
 
 fn resolve_local(local: @ast::local, cx: ctxt, visitor: visit::vt<ctxt>) {
-    cx.region_map.local_blocks.insert(
-        local.node.id, parent_id(cx, local.span));
+    record_parent(cx, local.node.id);
     visit::visit_local(local, cx, visitor);
 }
 
@@ -391,19 +385,17 @@ fn resolve_fn(fk: visit::fn_kind, decl: ast::fn_decl, body: ast::blk,
            cx.closure_parent, fn_cx.parent];
 
     for decl.inputs.each { |input|
-        cx.region_map.local_blocks.insert(
-            input.id, body.node.id);
+        cx.region_map.insert(input.id, body.node.id);
     }
 
     visit::visit_fn(fk, decl, body, sp, id, fn_cx, visitor);
 }
 
 fn resolve_crate(sess: session, def_map: resolve::def_map, crate: @ast::crate)
-        -> @region_map {
+        -> region_map {
     let cx: ctxt = {sess: sess,
                     def_map: def_map,
-                    region_map: @{parents: map::int_hash(),
-                                  local_blocks: map::int_hash()},
+                    region_map: map::int_hash(),
                     parent: none,
                     closure_parent: none};
     let visitor = visit::mk_vt(@{

src/rustc/middle/ty.rs

@@ -206,7 +206,7 @@ type ctxt =
       mut next_id: uint,
       sess: session::session,
       def_map: resolve::def_map,
-      region_map: @middle::region::region_map,
+      region_map: middle::region::region_map,
 
       // Stores the types for various nodes in the AST.  Note that this table
       // is not guaranteed to be populated until after typeck.  See
@@ -459,7 +459,7 @@ fn new_ty_hash<V: copy>() -> map::hashmap<t, V> {
 
 fn mk_ctxt(s: session::session, dm: resolve::def_map, amap: ast_map::map,
            freevars: freevars::freevar_map,
-           region_map: @middle::region::region_map) -> ctxt {
+           region_map: middle::region::region_map) -> ctxt {
     let interner = map::hashmap({|&&k: intern_key|
         hash_type_structure(k.struct) +
             option::map_default(k.o_def_id, 0u, ast_util::hash_def_id)
@@ -688,7 +688,7 @@ fn default_arg_mode_for_ty(ty: ty::t) -> ast::rmode {
 // Returns the narrowest lifetime enclosing the evaluation of the expression
 // with id `id`.
 fn encl_region(cx: ctxt, id: ast::node_id) -> ty::region {
-    alt cx.region_map.parents.find(id) {
+    alt cx.region_map.find(id) {
       some(encl_scope) {ty::re_scope(encl_scope)}
       none {ty::re_static}
     }

src/rustc/middle/typeck.rs

@@ -1577,9 +1577,8 @@ fn region_of(fcx: @fn_ctxt, expr: @ast::expr) -> ty::region {
         alt defn {
           ast::def_local(local_id, _) |
           ast::def_upvar(local_id, _, _) {
-            let local_blocks = fcx.ccx.tcx.region_map.local_blocks;
-            let local_block_id = local_blocks.get(local_id);
-            ty::re_scope(local_block_id)
+            let local_scope = fcx.ccx.tcx.region_map.get(local_id);
+            ty::re_scope(local_scope)
           }
           _ {
             ty::re_static
@@ -2629,8 +2628,7 @@ fn check_decl_local(fcx: @fn_ctxt, local: @ast::local) -> bool {
     }
 
     let region =
-        ty::re_scope(
-            fcx.ccx.tcx.region_map.local_blocks.get(local.node.id));
+        ty::re_scope(fcx.ccx.tcx.region_map.get(local.node.id));
     let pcx = {
         fcx: fcx,
         map: pat_id_map(fcx.ccx.tcx.def_map, local.node.pat),

src/rustc/middle/typeck/regionck.rs

@@ -16,21 +16,24 @@ the region scope `r`.
 import util::ppaux;
 import syntax::print::pprust;
 
+type rcx = @{fcx: @fn_ctxt, mut errors_reported: uint};
+type rvt = visit::vt<rcx>;
+
 fn regionck_expr(fcx: @fn_ctxt, e: @ast::expr) {
-    let v = regionck_visitor(fcx);
-    v.visit_expr(e, fcx, v);
+    let rcx = @{fcx:fcx, mut errors_reported: 0u};
+    let v = regionck_visitor();
+    v.visit_expr(e, rcx, v);
 }
 
 fn regionck_fn(fcx: @fn_ctxt,
                _decl: ast::fn_decl,
                blk: ast::blk) {
-    let v = regionck_visitor(fcx);
-    v.visit_block(blk, fcx, v);
+    let rcx = @{fcx:fcx, mut errors_reported: 0u};
+    let v = regionck_visitor();
+    v.visit_block(blk, rcx, v);
 }
 
-type rvt = visit::vt<@fn_ctxt>;
-
-fn regionck_visitor(_fcx: @fn_ctxt) -> rvt {
+fn regionck_visitor() -> rvt {
     visit::mk_vt(@{visit_item: visit_item,
                    visit_stmt: visit_stmt,
                    visit_expr: visit_expr,
@@ -40,67 +43,106 @@ fn regionck_visitor(_fcx: @fn_ctxt) -> rvt {
                    with *visit::default_visitor()})
 }
 
-fn visit_item(_item: @ast::item, &&_fcx: @fn_ctxt, _v: rvt) {
+fn visit_item(_item: @ast::item, &&_rcx: rcx, _v: rvt) {
     // Ignore items
 }
 
-fn visit_local(l: @ast::local, &&fcx: @fn_ctxt, v: rvt) {
-    visit::visit_local(l, fcx, v);
+fn visit_local(l: @ast::local, &&rcx: rcx, v: rvt) {
+    let e = rcx.errors_reported;
+    v.visit_pat(l.node.pat, rcx, v);
+    if e != rcx.errors_reported {
+        ret; // if decl has errors, skip initializer expr
+    }
+
+    v.visit_ty(l.node.ty, rcx, v);
+    for l.node.init.each { |i|
+        v.visit_expr(i.expr, rcx, v);
+    }
 }
 
-fn visit_pat(p: @ast::pat, &&fcx: @fn_ctxt, v: rvt) {
-    visit::visit_pat(p, fcx, v);
+fn visit_pat(p: @ast::pat, &&rcx: rcx, v: rvt) {
+    let fcx = rcx.fcx;
+    alt p.node {
+      ast::pat_ident(path, _)
+      if !pat_util::pat_is_variant(fcx.ccx.tcx.def_map, p) {
+        #debug["visit_pat binding=%s", path.idents[0]];
+        visit_node(p.id, p.span, rcx);
+      }
+      _ {}
+    }
+
+    visit::visit_pat(p, rcx, v);
 }
 
-fn visit_block(b: ast::blk, &&fcx: @fn_ctxt, v: rvt) {
-    visit::visit_block(b, fcx, v);
+fn visit_block(b: ast::blk, &&rcx: rcx, v: rvt) {
+    visit::visit_block(b, rcx, v);
 }
 
-fn visit_expr(e: @ast::expr, &&fcx: @fn_ctxt, v: rvt) {
+fn visit_expr(e: @ast::expr, &&rcx: rcx, v: rvt) {
     #debug["visit_expr(e=%s)", pprust::expr_to_str(e)];
 
-    visit_ty(fcx.expr_ty(e), e.id, e.span, fcx);
-    visit::visit_expr(e, fcx, v);
+    alt e.node {
+      ast::expr_path(*) {
+        // Avoid checking the use of local variables, as we already
+        // check their definitions.  The def'n always encloses the
+        // use.  So if the def'n is enclosed by the region, then the
+        // uses will also be enclosed (and otherwise, an error will
+        // have been reported at the def'n site).
+        alt lookup_def(rcx.fcx, e.span, e.id) {
+          ast::def_local(*) | ast::def_arg(*) | ast::def_upvar(*) { ret; }
+          _ { }
+        }
+      }
+      _ { }
+    }
+
+    if !visit_node(e.id, e.span, rcx) { ret; }
+    visit::visit_expr(e, rcx, v);
 }
 
-fn visit_stmt(s: @ast::stmt, &&fcx: @fn_ctxt, v: rvt) {
-    visit::visit_stmt(s, fcx, v);
+fn visit_stmt(s: @ast::stmt, &&rcx: rcx, v: rvt) {
+    visit::visit_stmt(s, rcx, v);
 }
 
-fn visit_ty(ty: ty::t,
-            id: ast::node_id,
-            span: span,
-            fcx: @fn_ctxt) {
+// checks the type of the node `id` and reports an error if it
+// references a region that is not in scope for that node.  Returns
+// false if an error is reported; this is used to cause us to cut off
+// region checking for that subtree to avoid reporting tons of errors.
+fn visit_node(id: ast::node_id, span: span, rcx: rcx) -> bool {
+    let fcx = rcx.fcx;
 
     // Try to resolve the type.  If we encounter an error, then typeck
     // is going to fail anyway, so just stop here and let typeck
     // report errors later on in the writeback phase.
-    let ty = alt infer::resolve_deep(fcx.infcx, ty, false) {
-      result::err(_) { ret; }
+    let ty0 = fcx.node_ty(id);
+    let ty = alt infer::resolve_deep(fcx.infcx, ty0, false) {
+      result::err(_) { ret true; }
       result::ok(ty) { ty }
     };
 
     // find the region where this expr evaluation is taking place
     let tcx = fcx.ccx.tcx;
     let encl_region = ty::encl_region(tcx, id);
 
-    #debug["visit_ty(ty=%s, id=%d, encl_region=%s)",
+    #debug["visit_node(ty=%s, id=%d, encl_region=%s, ty0=%s)",
            ppaux::ty_to_str(tcx, ty),
            id,
-           ppaux::region_to_str(tcx, encl_region)];
+           ppaux::region_to_str(tcx, encl_region),
+           ppaux::ty_to_str(tcx, ty0)];
 
     // Otherwise, look at the type and see if it is a region pointer.
-    if !ty::type_has_regions(ty) { ret; }
+    let e = rcx.errors_reported;
     ty::walk_regions_and_ty(
         tcx, ty,
-        { |r| constrain_region(fcx, encl_region, span, r); },
+        { |r| constrain_region(rcx, encl_region, span, r); },
         { |t| ty::type_has_regions(t) });
+    ret (e == rcx.errors_reported);
 
-    fn constrain_region(fcx: @fn_ctxt,
+    fn constrain_region(rcx: rcx,
                         encl_region: ty::region,
                         span: span,
                         region: ty::region) {
-        let tcx = fcx.ccx.tcx;
+        let tcx = rcx.fcx.ccx.tcx;
 
         #debug["constrain_region(encl_region=%s, region=%s)",
                ppaux::region_to_str(tcx, encl_region),
@@ -117,13 +159,14 @@ fn visit_ty(ty: ty::t,
           _ {}
         }
 
-        alt fcx.mk_subr(encl_region, region) {
+        alt rcx.fcx.mk_subr(encl_region, region) {
           result::err(_) {
             tcx.sess.span_err(
                 span,
                 #fmt["reference is not valid outside \
                       of its lifetime, %s",
                      ppaux::region_to_str(tcx, region)]);
+            rcx.errors_reported += 1u;
           }
           result::ok(()) {
           }

src/test/compile-fail/regions-borrow.rs

@@ -5,5 +5,4 @@ fn main() {
     let r = foo(p);
     //!^ ERROR reference is not valid
     assert *p == *r;
-    //!^ ERROR reference is not valid
 }

src/test/compile-fail/regions-escape-loop-via-variable.rs

@@ -0,0 +1,14 @@
+fn main() {
+    let x = 3;
+
+    // Here, the variable `p` gets inferred to a type with a lifetime
+    // of the loop body.  The regionck then determines that this type
+    // is invalid.
+    let mut p = //! ERROR reference is not valid
+        &x;
+
+    loop {
+        let x = 1 + *p;
+        p = &x;
+    }
+}

src/test/compile-fail/regions-leaking-ptr.rs

@@ -12,8 +12,6 @@ fn broken() -> int {
     //!^ ERROR reference is not valid
     //!^^ ERROR reference is not valid
     //!^^^ ERROR reference is not valid
-    //!^^^^ ERROR reference is not valid
-    //!^^^^^ ERROR reference is not valid
 }
 
 fn main() { }