From e0f59e835e062d27543ae5085b49effb13294b61 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 22 May 2012 21:36:50 -0700 Subject: [PATCH] modify borrowck to allow arbitrary borrows in pure scopes --- src/rustc/middle/borrowck.rs | 153 +++++++++++++++--- src/test/compile-fail/borrowck-lend-args.rs | 5 +- .../compile-fail/borrowck-pat-enum-in-box.rs | 27 +++- src/test/compile-fail/borrowck-pat-enum.rs | 25 +-- .../compile-fail/borrowck-uniq-via-box.rs | 28 ++-- .../compile-fail/borrowck-uniq-via-ref.rs | 28 ++-- src/test/compile-fail/impure-pred.rs | 6 +- .../compile-fail/pure-modifies-aliased.rs | 6 +- 8 files changed, 212 insertions(+), 66 deletions(-) diff --git a/src/rustc/middle/borrowck.rs b/src/rustc/middle/borrowck.rs index 595ad2d0917..bb65df934f7 100644 --- a/src/rustc/middle/borrowck.rs +++ b/src/rustc/middle/borrowck.rs @@ -38,7 +38,8 @@ fn check_crate(tcx: ty::ctxt, let req_maps = if msg_level > 0u { gather_loans(bccx, crate) } else { - {req_loan_map: int_hash()} + {req_loan_map: int_hash(), + pure_map: int_hash()} }; check_loans(bccx, req_maps, crate); ret (bccx.root_map, bccx.mutbl_map); @@ -165,14 +166,16 @@ fn root_map() -> root_map { // sure that all of these loans are honored. type req_maps = { - req_loan_map: hashmap + req_loan_map: hashmap, + pure_map: hashmap }; enum gather_loan_ctxt = @{bccx: borrowck_ctxt, req_maps: req_maps}; fn gather_loans(bccx: borrowck_ctxt, crate: @ast::crate) -> req_maps { let glcx = gather_loan_ctxt(@{bccx: bccx, - req_maps: {req_loan_map: int_hash()}}); + req_maps: {req_loan_map: int_hash(), + pure_map: int_hash()}}); let v = visit::mk_vt(@{visit_expr: req_loans_in_expr with *visit::default_visitor()}); visit::visit_crate(*crate, glcx, v); @@ -293,15 +296,40 @@ impl methods for gather_loan_ctxt { // it dynamically (or see that it is preserved by virtue of being // rooted in some immutable path) none { - self.bccx.report_if_err( - self.check_mutbl(req_mutbl, cmt).chain { |_ok| - let opt_scope_id = alt scope_r { - ty::re_scope(scope_id) { some(scope_id) } - _ { none } - }; + let opt_scope_id = alt scope_r { + ty::re_scope(scope_id) { some(scope_id) } + _ { none } + }; + let result = { + self.check_mutbl(req_mutbl, cmt).chain { |_ok| self.bccx.preserve(cmt, opt_scope_id) - }) + } + }; + + alt result { + ok(()) { + // we were able guarantee the validity of the ptr, + // perhaps by rooting or because it is immutably + // rooted. good. + } + err(e) { + // not able to guarantee the validity of the ptr. + // rather than report an error, presuming that the + // borrow is for a limited scope, we'll make one last + // ditch effort and require that the scope where the + // borrow occurs be pure. + alt opt_scope_id { + some(scope_id) { + self.req_maps.pure_map.insert(scope_id, e); + } + none { + // otherwise, fine, I give up. + self.bccx.report(e); + } + } + } + } } } } @@ -475,16 +503,30 @@ enum check_loan_ctxt = @{ // we are in a ctor, we track the self id mut in_ctor: bool, - mut is_pure: bool + mut is_pure: purity_cause }; +// if we are enforcing purity, why are we doing so? +enum purity_cause { + // not enforcing purity: + pc_impure, + + // enforcing purity because fn was declared pure: + pc_declaration, + + // enforce purity because we need to guarantee the + // validity of some alias; `bckerr` describes the + // reason we needed to enforce purity. + pc_cmt(bckerr) +} + fn check_loans(bccx: borrowck_ctxt, req_maps: req_maps, crate: @ast::crate) { let clcx = check_loan_ctxt(@{bccx: bccx, req_maps: req_maps, mut in_ctor: false, - mut is_pure: false}); + mut is_pure: pc_impure}); let vt = visit::mk_vt(@{visit_expr: check_loans_in_expr, visit_block: check_loans_in_block, visit_fn: check_loans_in_fn @@ -595,17 +637,15 @@ impl methods for check_loan_ctxt { }; alt callee_purity { - ast::crust_fn | ast::pure_fn { /*ok*/ } - ast::impure_fn { - self.bccx.span_err( - expr.span, - "pure function calls function \ - not known to be pure"); + ast::crust_fn | ast::pure_fn { + /*ok*/ } - ast::unsafe_fn { + ast::impure_fn | ast::unsafe_fn { self.bccx.span_err( expr.span, - "pure function calls unsafe function"); + "access to non-pure functions \ + prohibited in a pure context"); + self.report_why_pure(); } } } @@ -613,6 +653,21 @@ impl methods for check_loan_ctxt { } } + fn check_for_purity_requirement(scope_id: ast::node_id) { + // if we are not already enforcing purity, check whether the + // gather pass thought we needed to enforce purity for this + // scope. + alt self.is_pure { + pc_declaration | pc_cmt(*) { } + pc_impure { + alt self.req_maps.pure_map.find(scope_id) { + none {} + some(e) {self.is_pure = pc_cmt(e)} + } + } + } + } + fn check_for_conflicting_loans(scope_id: ast::node_id) { let new_loanss = alt self.req_maps.req_loan_map.find(scope_id) { none { ret; } @@ -683,11 +738,12 @@ impl methods for check_loan_ctxt { // if this is a pure function, only loan-able state can be // assigned, because it is uniquely tied to this function and // is not visible from the outside - if self.is_pure && cmt.lp.is_none() { + if self.is_pure != pc_impure && cmt.lp.is_none() { self.bccx.span_err( ex.span, - #fmt["%s prohibited in pure functions", + #fmt["%s prohibited in a pure context", at.ing_form(self.bccx.cmt_to_str(cmt))]); + self.report_why_pure(); } // check for a conflicting loan as well, except in the case of @@ -720,6 +776,23 @@ impl methods for check_loan_ctxt { self.bccx.add_to_mutbl_map(cmt); } + fn report_why_pure() { + alt self.is_pure { + pc_impure { + self.tcx().sess.bug("report_why_pure() called when impure"); + } + pc_declaration { + // fn was declared pure; no need to report this, I think + } + pc_cmt(e) { + self.tcx().sess.span_note( + e.cmt.span, + #fmt["pure context is required due to an illegal borrow: %s", + self.bccx.bckerr_code_to_str(e.code)]); + } + } + } + fn check_move_out(ex: @ast::expr) { let cmt = self.bccx.cat_expr(ex); self.check_move_out_from_cmt(cmt); @@ -788,8 +861,11 @@ fn check_loans_in_fn(fk: visit::fn_kind, decl: ast::fn_decl, body: ast::blk, _ { self.in_ctor = false; } }; + // NDM this doesn't seem algother right, what about fn items + // nested in pure fns? etc? + alt decl.purity { - ast::pure_fn { self.is_pure = true; } + ast::pure_fn { self.is_pure = pc_declaration; } _ { } } @@ -802,6 +878,16 @@ fn check_loans_in_expr(expr: @ast::expr, &&self: check_loan_ctxt, vt: visit::vt) { self.check_for_conflicting_loans(expr.id); + save_and_restore(self.is_pure) {|| + self.check_for_purity_requirement(expr.id); + check_loans_in_expr_1(expr, self, vt); + } +} + +// avoid rightward drift by breaking this out into its own fn +fn check_loans_in_expr_1(expr: @ast::expr, + &&self: check_loan_ctxt, + vt: visit::vt) { alt expr.node { ast::expr_swap(l, r) { self.check_assignment(at_swap, l); @@ -844,7 +930,7 @@ fn check_loans_in_expr(expr: @ast::expr, } } ast::expr_call(f, args, _) { - if self.is_pure { + if self.is_pure != pc_impure { self.check_pure(f); for args.each { |arg| self.check_pure(arg) } } @@ -873,12 +959,27 @@ fn check_loans_in_block(blk: ast::blk, vt: visit::vt) { save_and_restore(self.is_pure) {|| self.check_for_conflicting_loans(blk.node.id); + self.check_for_purity_requirement(blk.node.id); alt blk.node.rules { ast::default_blk { } - ast::unchecked_blk | - ast::unsafe_blk { self.is_pure = false; } + ast::unchecked_blk { + alt self.is_pure { + pc_impure | pc_declaration { + self.is_pure = pc_impure; + } + pc_cmt(_) { + // unchecked does not override purity requirements due + // to borrows; unchecked didn't seem strong enough to + // justify potential memory unsafety to me + } + } + } + ast::unsafe_blk { + // unsafe blocks override everything + self.is_pure = pc_impure; + } } visit::visit_block(blk, self, vt); diff --git a/src/test/compile-fail/borrowck-lend-args.rs b/src/test/compile-fail/borrowck-lend-args.rs index fa9c9946da7..c442db5cdc2 100644 --- a/src/test/compile-fail/borrowck-lend-args.rs +++ b/src/test/compile-fail/borrowck-lend-args.rs @@ -4,11 +4,12 @@ fn borrow(_v: &int) {} fn borrow_from_arg_imm_ref(&&v: ~int) { - borrow(v); // ERROR unique value in aliasable, mutable location + borrow(v); } fn borrow_from_arg_mut_ref(&v: ~int) { - borrow(v); //! ERROR unique value in aliasable, mutable location + borrow(v); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn borrow_from_arg_move(-v: ~int) { diff --git a/src/test/compile-fail/borrowck-pat-enum-in-box.rs b/src/test/compile-fail/borrowck-pat-enum-in-box.rs index 162a7942030..1a57b255102 100644 --- a/src/test/compile-fail/borrowck-pat-enum-in-box.rs +++ b/src/test/compile-fail/borrowck-pat-enum-in-box.rs @@ -10,13 +10,32 @@ fn match_imm_box(v: &const @option) -> int { fn match_const_box(v: &const @const option) -> int { alt *v { - @some(i) { - //!^ ERROR enum variant in aliasable, mutable location - i - } + @some(i) { i } // ok because this is pure @none {0} } } +pure fn pure_process(_i: int) {} + +fn match_const_box_and_do_pure_things(v: &const @const option) { + alt *v { + @some(i) { + pure_process(i) + } + @none {} + } +} + +fn process(_i: int) {} + +fn match_const_box_and_do_bad_things(v: &const @const option) { + alt *v { + @some(i) { //! NOTE pure context is required due to an illegal borrow: enum variant in aliasable, mutable location + process(i) //! ERROR access to non-pure functions prohibited in a pure context + } + @none {} + } +} + fn main() { } diff --git a/src/test/compile-fail/borrowck-pat-enum.rs b/src/test/compile-fail/borrowck-pat-enum.rs index 0c8ca4f3ea0..fa2ccbedad5 100644 --- a/src/test/compile-fail/borrowck-pat-enum.rs +++ b/src/test/compile-fail/borrowck-pat-enum.rs @@ -4,7 +4,6 @@ fn match_ref(&&v: option) -> int { alt v { some(i) { - //^ ERROR enum variant in aliasable, mutable location i } none {0} @@ -20,25 +19,33 @@ fn match_ref_unused(&&v: option) { fn match_const_reg(v: &const option) -> int { alt *v { - some(i) { - //!^ ERROR enum variant in aliasable, mutable location - i - } + some(i) {i} // OK because this is pure none {0} } } +fn impure(_i: int) { +} + fn match_const_reg_unused(v: &const option) { alt *v { - some(_) {} + some(_) {impure(0)} // OK because nothing is captured none {} } } -fn match_imm_reg(v: &option) -> int { +fn match_const_reg_impure(v: &const option) { alt *v { - some(i) {i} - none {0} + some(i) {impure(i)} //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: enum variant in aliasable, mutable location + none {} + } +} + +fn match_imm_reg(v: &option) { + alt *v { + some(i) {impure(i)} // OK because immutable + none {} } } diff --git a/src/test/compile-fail/borrowck-uniq-via-box.rs b/src/test/compile-fail/borrowck-uniq-via-box.rs index e15eba4c618..261af1870a8 100644 --- a/src/test/compile-fail/borrowck-uniq-via-box.rs +++ b/src/test/compile-fail/borrowck-uniq-via-box.rs @@ -4,20 +4,23 @@ fn borrow(_v: &int) {} fn box_mut(v: @mut ~int) { - borrow(*v); //! ERROR illegal borrow: unique value in aliasable, mutable location - + borrow(*v); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_rec_mut(v: @{mut f: ~int}) { - borrow(v.f); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_mut_rec(v: @mut {f: ~int}) { - borrow(v.f); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_mut_recs(v: @mut {f: {g: {h: ~int}}}) { - borrow(v.f.g.h); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f.g.h); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_imm(v: @~int) { @@ -33,23 +36,28 @@ fn box_imm_recs(v: @{f: {g: {h: ~int}}}) { } fn box_const(v: @const ~int) { - borrow(*v); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(*v); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_rec_const(v: @{const f: ~int}) { - borrow(v.f); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_recs_const(v: @{f: {g: {const h: ~int}}}) { - borrow(v.f.g.h); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f.g.h); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_const_rec(v: @const {f: ~int}) { - borrow(v.f); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_const_recs(v: @const {f: {g: {h: ~int}}}) { - borrow(v.f.g.h); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f.g.h); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn main() { diff --git a/src/test/compile-fail/borrowck-uniq-via-ref.rs b/src/test/compile-fail/borrowck-uniq-via-ref.rs index 91a47667dd1..b7e05f91c01 100644 --- a/src/test/compile-fail/borrowck-uniq-via-ref.rs +++ b/src/test/compile-fail/borrowck-uniq-via-ref.rs @@ -3,20 +3,23 @@ fn borrow(_v: &int) {} fn box_mut(v: &mut ~int) { - borrow(*v); //! ERROR illegal borrow: unique value in aliasable, mutable location - + borrow(*v); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_rec_mut(v: &{mut f: ~int}) { - borrow(v.f); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_mut_rec(v: &mut {f: ~int}) { - borrow(v.f); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_mut_recs(v: &mut {f: {g: {h: ~int}}}) { - borrow(v.f.g.h); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f.g.h); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_imm(v: &~int) { @@ -32,23 +35,28 @@ fn box_imm_recs(v: &{f: {g: {h: ~int}}}) { } fn box_const(v: &const ~int) { - borrow(*v); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(*v); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_rec_const(v: &{const f: ~int}) { - borrow(v.f); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_recs_const(v: &{f: {g: {const h: ~int}}}) { - borrow(v.f.g.h); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f.g.h); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_const_rec(v: &const {f: ~int}) { - borrow(v.f); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn box_const_recs(v: &const {f: {g: {h: ~int}}}) { - borrow(v.f.g.h); //! ERROR illegal borrow: unique value in aliasable, mutable location + borrow(v.f.g.h); //! ERROR access to non-pure functions prohibited in a pure context + //!^ NOTE pure context is required due to an illegal borrow: unique value in aliasable, mutable location } fn main() { diff --git a/src/test/compile-fail/impure-pred.rs b/src/test/compile-fail/impure-pred.rs index 4b7faaa0280..960dc02269b 100644 --- a/src/test/compile-fail/impure-pred.rs +++ b/src/test/compile-fail/impure-pred.rs @@ -1,9 +1,11 @@ // -*- rust -*- -// error-pattern: pure function calls function not known to be pure fn g() { } -pure fn f(q: int) -> bool { g(); ret true; } +pure fn f(_q: int) -> bool { + g(); //! ERROR access to non-pure functions prohibited in a pure context + ret true; +} fn main() { let x = 0; diff --git a/src/test/compile-fail/pure-modifies-aliased.rs b/src/test/compile-fail/pure-modifies-aliased.rs index 9e825b9daca..08852746d11 100644 --- a/src/test/compile-fail/pure-modifies-aliased.rs +++ b/src/test/compile-fail/pure-modifies-aliased.rs @@ -1,16 +1,16 @@ // Check that pure functions cannot modify aliased state. pure fn modify_in_ref(&&sum: {mut f: int}) { - sum.f = 3; //! ERROR assigning to mutable field prohibited in pure functions + sum.f = 3; //! ERROR assigning to mutable field prohibited in a pure context } pure fn modify_in_box(sum: @mut {f: int}) { - sum.f = 3; //! ERROR assigning to mutable field prohibited in pure functions + sum.f = 3; //! ERROR assigning to mutable field prohibited in a pure context } impl foo for int { pure fn modify_in_box_rec(sum: @{mut f: int}) { - sum.f = self; //! ERROR assigning to mutable field prohibited in pure functions + sum.f = self; //! ERROR assigning to mutable field prohibited in a pure context } }