diff --git a/src/rustc/middle/borrowck.rs b/src/rustc/middle/borrowck.rs index b9076cc435c5..0908bafb2e25 100644 --- a/src/rustc/middle/borrowck.rs +++ b/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| diff --git a/src/rustc/middle/region.rs b/src/rustc/middle/region.rs index f8fcfb481e15..b12836e61258 100644 --- a/src/rustc/middle/region.rs +++ b/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, - - // 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 -}; +// 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; 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,11 +203,11 @@ 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; } } @@ -219,15 +215,15 @@ fn scope_contains(region_map: @region_map, superscope: ast::node_id, 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 { - 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) { } _ { /* 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) { } fn resolve_local(local: @ast::local, cx: ctxt, visitor: visit::vt) { - 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(@{ diff --git a/src/rustc/middle/ty.rs b/src/rustc/middle/ty.rs index ca0636ec40dd..3b5093017dc7 100644 --- a/src/rustc/middle/ty.rs +++ b/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() -> map::hashmap { 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} } diff --git a/src/rustc/middle/typeck.rs b/src/rustc/middle/typeck.rs index 59a10be49efe..7009488272df 100644 --- a/src/rustc/middle/typeck.rs +++ b/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), diff --git a/src/rustc/middle/typeck/regionck.rs b/src/rustc/middle/typeck/regionck.rs index 2fda17235f25..27194934dbef 100644 --- a/src/rustc/middle/typeck/regionck.rs +++ b/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; + 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,43 +43,80 @@ 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 } }; @@ -84,23 +124,25 @@ fn visit_ty(ty: ty::t, 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(()) { } diff --git a/src/test/compile-fail/regions-borrow.rs b/src/test/compile-fail/regions-borrow.rs index d2fcac56bedb..5bcfc9841622 100644 --- a/src/test/compile-fail/regions-borrow.rs +++ b/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 } diff --git a/src/test/compile-fail/regions-escape-loop-via-variable.rs b/src/test/compile-fail/regions-escape-loop-via-variable.rs new file mode 100644 index 000000000000..bf0c625abab3 --- /dev/null +++ b/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; + } +} diff --git a/src/test/compile-fail/regions-leaking-ptr.rs b/src/test/compile-fail/regions-leaking-ptr.rs index e7b89b17c542..b44c38e68f81 100644 --- a/src/test/compile-fail/regions-leaking-ptr.rs +++ b/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() { } \ No newline at end of file