From aacd18f4ed860420cb89cd103a9832ef47e838f3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 6 Aug 2012 18:14:46 -0700 Subject: [PATCH] first shot at integrating ref/value bindings into borrowck (more needed) --- src/rustc/middle/borrowck.rs | 6 ++ src/rustc/middle/borrowck/gather_loans.rs | 73 ++++++++++++------- src/rustc/middle/borrowck/loan.rs | 3 +- src/rustc/middle/borrowck/preserve.rs | 16 +--- src/rustc/middle/region.rs | 18 +++-- src/rustc/middle/ty.rs | 12 ++- .../borrowck-pat-by-value-binding.rs | 34 +++++++++ .../compile-fail/borrowck-ref-mut-of-imm.rs | 10 +++ 8 files changed, 120 insertions(+), 52 deletions(-) create mode 100644 src/test/compile-fail/borrowck-pat-by-value-binding.rs create mode 100644 src/test/compile-fail/borrowck-ref-mut-of-imm.rs diff --git a/src/rustc/middle/borrowck.rs b/src/rustc/middle/borrowck.rs index 3df0cb3b25e..50a903551cd 100644 --- a/src/rustc/middle/borrowck.rs +++ b/src/rustc/middle/borrowck.rs @@ -454,6 +454,12 @@ impl methods of get_type_for_node for ty::ctxt { } } +impl borrowck_ctxt { + fn is_subregion_of(r_sub: ty::region, r_sup: ty::region) -> bool { + region::is_subregion_of(self.tcx.region_map, r_sub, r_sup) + } +} + impl error_methods for borrowck_ctxt { fn report_if_err(bres: bckres<()>) { match bres { diff --git a/src/rustc/middle/borrowck/gather_loans.rs b/src/rustc/middle/borrowck/gather_loans.rs index 79696becfeb..23baa0976f8 100644 --- a/src/rustc/middle/borrowck/gather_loans.rs +++ b/src/rustc/middle/borrowck/gather_loans.rs @@ -9,6 +9,7 @@ import categorization::{public_methods, opt_deref_kind}; import loan::public_methods; import preserve::{public_methods, preserve_condition, pc_ok, pc_if_pure}; +import ty::ty_region; export gather_loans; @@ -105,10 +106,7 @@ fn req_loans_in_expr(ex: @ast::expr, // make sure that the thing we are pointing out stays valid // for the lifetime `scope_r` of the resulting ptr: - let scope_r = - match check ty::get(tcx.ty(ex)).struct { - ty::ty_rptr(r, _) => r - }; + let scope_r = ty_region(tcx.ty(ex)); self.guarantee_valid(base_cmt, mutbl, scope_r); visit::visit_expr(ex, self, vt); } @@ -474,37 +472,60 @@ impl methods for gather_loan_ctxt { } } - ast::pat_ident(_, _, none) if self.pat_is_variant(pat) => { - // nullary variant - debug!{"nullary variant"}; - } - ast::pat_ident(_, id, o_pat) => { - // XXX: Needs to take by-ref/by-val into account. + ast::pat_ident(bm, id, o_pat) if !self.pat_is_variant(pat) => { + match bm { + ast::bind_by_value => { + // copying does not borrow anything, so no check is required + } + ast::bind_by_ref(mutbl) => { + // ref x or ref x @ p --- creates a ptr which must + // remain valid for the scope of the alt - // x or x @ p --- `x` must remain valid for the scope of the alt - debug!{"defines identifier %s", pprust::path_to_str(id)}; + // find the region of the resulting pointer (note that + // the type of such a pattern will *always* be a + // region pointer) + let scope_r = ty_region(tcx.ty(pat)); - // Note: there is a discussion of the function of - // cat_discr in the method preserve(): - let cmt1 = self.bccx.cat_discr(cmt, alt_id); - let arm_scope = ty::re_scope(arm_id); + // if the scope of the region ptr turns out to be + // specific to this arm, wrap the categorization with + // a cat_discr() node. There is a detailed discussion + // of the function of this node in method preserve(): + let arm_scope = ty::re_scope(arm_id); + if self.bccx.is_subregion_of(scope_r, arm_scope) { + let cmt_discr = self.bccx.cat_discr(cmt, alt_id); + self.guarantee_valid(cmt_discr, mutbl, scope_r); + } else { + self.guarantee_valid(cmt, mutbl, scope_r); + } + } + ast::bind_by_implicit_ref => { + // Note: there is a discussion of the function of + // cat_discr in the method preserve(): + let cmt1 = self.bccx.cat_discr(cmt, alt_id); + let arm_scope = ty::re_scope(arm_id); - // Remember the mutability of the location that this - // binding refers to. This will be used later when - // categorizing the binding. This is a bit of a hack that - // would be better fixed by #2329; in that case we could - // allow the user to specify if they want an imm, const, - // or mut binding, or else just reflect the mutability - // through the type of the region pointer. - self.bccx.binding_map.insert(pat.id, cmt1.mutbl); - - self.guarantee_valid(cmt1, m_const, arm_scope); + // Remember the mutability of the location that this + // binding refers to. This will be used later when + // categorizing the binding. This is a bit of a hack that + // would be better fixed by #2329; in that case we could + // allow the user to specify if they want an imm, const, + // or mut binding, or else just reflect the mutability + // through the type of the region pointer. + self.bccx.binding_map.insert(pat.id, cmt1.mutbl); + self.guarantee_valid(cmt1, m_const, arm_scope); + } + } for o_pat.each |p| { self.gather_pat(cmt, p, arm_id, alt_id); } } + ast::pat_ident(*) => { + // nullary variant: ignore. + assert self.pat_is_variant(pat); + } + ast::pat_rec(field_pats, _) => { // {f1: p1, ..., fN: pN} for field_pats.each |fp| { diff --git a/src/rustc/middle/borrowck/loan.rs b/src/rustc/middle/borrowck/loan.rs index 59e3cd183fc..9411545754f 100644 --- a/src/rustc/middle/borrowck/loan.rs +++ b/src/rustc/middle/borrowck/loan.rs @@ -39,8 +39,7 @@ impl loan_methods for loan_ctxt { fn ok_with_loan_of(cmt: cmt, scope_ub: ty::region, mutbl: ast::mutability) -> bckres<()> { - let region_map = self.tcx().region_map; - if region::subregion(region_map, scope_ub, self.scope_region) { + if self.bccx.is_subregion_of(self.scope_region, scope_ub) { // Note: all cmt's that we deal with will have a non-none // lp, because the entry point into this routine, // `borrowck_ctxt::loan()`, rejects any cmt with a diff --git a/src/rustc/middle/borrowck/preserve.rs b/src/rustc/middle/borrowck/preserve.rs index a586aca24f3..7993a972517 100644 --- a/src/rustc/middle/borrowck/preserve.rs +++ b/src/rustc/middle/borrowck/preserve.rs @@ -232,16 +232,6 @@ impl private_methods for &preserve_ctxt { // node appears to draw the line between what will be rooted // in the *arm* vs the *alt*. - // current scope must be the arm, which is always a child of alt: - assert { - match check self.scope_region { - ty::re_scope(arm_id) => { - self.tcx().region_map.get(arm_id) == alt_id - } - _ => {false} - } - }; - let alt_rooting_ctxt = preserve_ctxt({scope_region: ty::re_scope(alt_id) with **self}); @@ -289,8 +279,7 @@ impl private_methods for &preserve_ctxt { /// is a subscope of `scope_ub`; if so, success. fn compare_scope(cmt: cmt, scope_ub: ty::region) -> bckres { - let region_map = self.tcx().region_map; - if region::subregion(region_map, scope_ub, self.scope_region) { + if self.bccx.is_subregion_of(self.scope_region, scope_ub) { ok(pc_ok) } else { err({cmt:cmt, code:err_out_of_scope(scope_ub, @@ -326,12 +315,11 @@ impl private_methods for &preserve_ctxt { // we can only root values if the desired region is some concrete // scope within the fn body ty::re_scope(scope_id) => { - let region_map = self.tcx().region_map; #debug["Considering root map entry for %s: \ node %d:%u -> scope_id %?, root_ub %?", self.bccx.cmt_to_repr(cmt), base.id, derefs, scope_id, self.root_ub]; - if region::subregion(region_map, root_region, self.scope_region) { + if self.bccx.is_subregion_of(self.scope_region, root_region) { #debug["Elected to root"]; let rk = {id: base.id, derefs: derefs}; self.bccx.root_map.insert(rk, scope_id); diff --git a/src/rustc/middle/region.rs b/src/rustc/middle/region.rs index 362c4be7abe..91ba7ea60d7 100644 --- a/src/rustc/middle/region.rs +++ b/src/rustc/middle/region.rs @@ -99,15 +99,17 @@ fn scope_contains(region_map: region_map, superscope: ast::node_id, /// Determines whether one region is a subregion of another. This is /// intended to run *after inference* and sadly the logic is somewhat /// duplicated with the code in infer.rs. -fn subregion(region_map: region_map, - super_region: ty::region, - sub_region: ty::region) -> bool { - super_region == sub_region || - match (super_region, sub_region) { - (ty::re_static, _) => {true} +fn is_subregion_of(region_map: region_map, + sub_region: ty::region, + super_region: ty::region) -> bool { + sub_region == super_region || + match (sub_region, super_region) { + (_, ty::re_static) => { + true + } - (ty::re_scope(super_scope), ty::re_scope(sub_scope)) | - (ty::re_free(super_scope, _), ty::re_scope(sub_scope)) => { + (ty::re_scope(sub_scope), ty::re_scope(super_scope)) | + (ty::re_scope(sub_scope), ty::re_free(super_scope, _)) => { scope_contains(region_map, super_scope, sub_scope) } diff --git a/src/rustc/middle/ty.rs b/src/rustc/middle/ty.rs index 91eb030ec03..60461db08c4 100644 --- a/src/rustc/middle/ty.rs +++ b/src/rustc/middle/ty.rs @@ -107,6 +107,7 @@ export tbox_has_flag; export ty_var_id; export ty_to_def_id; export ty_fn_args; +export ty_region; export kind, kind_implicitly_copyable, kind_send_copy, kind_copyable; export kind_noncopyable, kind_const; export kind_can_be_copied, kind_can_be_sent, kind_can_be_implicitly_copied; @@ -2297,8 +2298,15 @@ fn ty_fn_ret_style(fty: t) -> ast::ret_style { fn is_fn_ty(fty: t) -> bool { match get(fty).struct { - ty_fn(_) => return true, - _ => return false + ty_fn(_) => true, + _ => false + } +} + +fn ty_region(ty: t) -> region { + match get(ty).struct { + ty_rptr(r, _) => r, + s => fail fmt!{"ty_region() invoked on non-rptr: %?", s} } } diff --git a/src/test/compile-fail/borrowck-pat-by-value-binding.rs b/src/test/compile-fail/borrowck-pat-by-value-binding.rs new file mode 100644 index 00000000000..2b8ec9d7419 --- /dev/null +++ b/src/test/compile-fail/borrowck-pat-by-value-binding.rs @@ -0,0 +1,34 @@ +fn process(_t: T) {} + +fn match_const_opt_by_mut_ref(v: &const option) { + match *v { + some(ref mut i) => process(i), //~ ERROR illegal borrow + none => () + } +} + +fn match_const_opt_by_const_ref(v: &const option) { + match *v { + some(ref const i) => process(i), //~ ERROR illegal borrow unless pure + //~^ NOTE impure due to + none => () + } +} + +fn match_const_opt_by_imm_ref(v: &const option) { + match *v { + some(ref i) => process(i), //~ ERROR illegal borrow unless pure + //~^ NOTE impure due to + none => () + } +} + +fn match_const_opt_by_value(v: &const option) { + match *v { + some(copy i) => process(i), + none => () + } +} + +fn main() { +} diff --git a/src/test/compile-fail/borrowck-ref-mut-of-imm.rs b/src/test/compile-fail/borrowck-ref-mut-of-imm.rs new file mode 100644 index 00000000000..e4a33d58feb --- /dev/null +++ b/src/test/compile-fail/borrowck-ref-mut-of-imm.rs @@ -0,0 +1,10 @@ +fn destructure(x: option) -> int { + match x { + none => 0, + some(ref mut v) => *v //~ ERROR illegal borrow + } +} + +fn main() { + assert destructure(some(22)) == 22; +}