Skip to content

Commit

Permalink
rustc: Tweak the borrow on closure invocations
Browse files Browse the repository at this point in the history
This alters the borrow checker's requirements on invoking closures from
requiring an immutable borrow to requiring a unique immutable borrow. This means
that it is illegal to invoke a closure through a `&` pointer because there is no
guarantee that is not aliased. This does not mean that a closure is required to
be in a mutable location, but rather a location which can be proven to be
unique (often through a mutable pointer).

For example, the following code is unsound and is no longer allowed:

    type Fn<'a> = ||:'a;

    fn call(f: |Fn|) {
        f(|| {
            f(|| {})
        });
    }

    fn main() {
        call(|a| {
            a();
        });
    }

There is no replacement for this pattern. For all closures which are stored in
structures, it was previously allowed to invoke the closure through `&self` but
it now requires invocation through `&mut self`.

The standard library has a good number of violations of this new rule, but the
fixes will be separated into multiple breaking change commits.

Closes rust-lang#12224

[breaking-change]
  • Loading branch information
alexcrichton committed Apr 23, 2014
1 parent 1e33589 commit 159a10d
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/librustc/middle/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ impl<'a> CheckLoanCtxt<'a> {
self.bccx.loan_path_to_str(&*old_loan.loan_path))
}

AddrOf | AutoRef | RefBinding => {
AddrOf | AutoRef | RefBinding | ClosureInvocation => {
format!("previous borrow of `{}` occurs here",
self.bccx.loan_path_to_str(&*old_loan.loan_path))
}
Expand Down
20 changes: 20 additions & 0 deletions src/librustc/middle/borrowck/gather_loans/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,26 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt,
visit::walk_expr(this, ex, ());
}

ast::ExprCall(f, _) => {
let expr_ty = ty::expr_ty_adjusted(tcx, f);
match ty::get(expr_ty).sty {
ty::ty_closure(~ty::ClosureTy {
store: ty::RegionTraitStore(..), ..
}) => {
let scope_r = ty::ReScope(ex.id);
let base_cmt = this.bccx.cat_expr(f);
this.guarantee_valid_kind(f.id,
f.span,
base_cmt,
ty::UniqueImmBorrow,
scope_r,
ClosureInvocation);
}
_ => {}
}
visit::walk_expr(this, ex, ());
}

_ => {
visit::walk_expr(this, ex, ());
}
Expand Down
9 changes: 9 additions & 0 deletions src/librustc/middle/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ pub enum LoanCause {
AddrOf,
AutoRef,
RefBinding,
ClosureInvocation,
}

#[deriving(Eq, TotalEq, Hash)]
Expand Down Expand Up @@ -629,6 +630,10 @@ impl<'a> BorrowckCtxt<'a> {
AddrOf | RefBinding | AutoRef => {
format!("cannot borrow {} as mutable", descr)
}
ClosureInvocation => {
self.tcx.sess.span_bug(err.span,
"err_mutbl with a closure invocation");
}
}
}
err_out_of_root_scope(..) => {
Expand Down Expand Up @@ -677,6 +682,10 @@ impl<'a> BorrowckCtxt<'a> {
BorrowViolation(RefBinding) => {
"cannot borrow data mutably"
}

BorrowViolation(ClosureInvocation) => {
"closure invocation"
}
};

match cause {
Expand Down
49 changes: 31 additions & 18 deletions src/librustc/middle/typeck/check/regionck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,15 @@ fn constrain_callee(rcx: &mut Rcx,
ty::ty_bare_fn(..) => { }
ty::ty_closure(ref closure_ty) => {
let region = match closure_ty.store {
ty::RegionTraitStore(r, _) => r,
ty::RegionTraitStore(r, _) => {
// While we're here, link the closure's region with a unique
// immutable borrow (gathered later in borrowck)
let mc = mc::MemCategorizationContext { typer: &*rcx };
let expr_cmt = ignore_err!(mc.cat_expr(callee_expr));
link_region(mc.typer, callee_expr.span, call_region,
ty::UniqueImmBorrow, expr_cmt);
r
}
ty::UniqTraitStore => ty::ReStatic
};
rcx.fcx.mk_subr(true, infer::InvokeClosure(callee_expr.span),
Expand Down Expand Up @@ -874,7 +882,8 @@ fn constrain_autoderefs(rcx: &mut Rcx,
{
let mc = mc::MemCategorizationContext { typer: &*rcx };
let self_cmt = ignore_err!(mc.cat_expr_autoderefd(deref_expr, i));
link_region(mc.typer, deref_expr.span, r, m, self_cmt);
link_region(mc.typer, deref_expr.span, r,
ty::BorrowKind::from_mutbl(m), self_cmt);
}

// Specialized version of constrain_call.
Expand Down Expand Up @@ -1092,7 +1101,8 @@ fn link_pattern(mc: mc::MemCategorizationContext<&Rcx>,
match mc.cat_slice_pattern(sub_cmt, slice_pat) {
Ok((slice_cmt, slice_mutbl, slice_r)) => {
link_region(mc.typer, sub_pat.span, slice_r,
slice_mutbl, slice_cmt);
ty::BorrowKind::from_mutbl(slice_mutbl),
slice_cmt);
}
Err(()) => {}
}
Expand All @@ -1118,17 +1128,20 @@ fn link_autoref(rcx: &Rcx,

match *autoref {
ty::AutoPtr(r, m) => {
link_region(mc.typer, expr.span, r, m, expr_cmt);
link_region(mc.typer, expr.span, r,
ty::BorrowKind::from_mutbl(m), expr_cmt);
}

ty::AutoBorrowVec(r, m) | ty::AutoBorrowVecRef(r, m) => {
let cmt_index = mc.cat_index(expr, expr_cmt, autoderefs+1);
link_region(mc.typer, expr.span, r, m, cmt_index);
link_region(mc.typer, expr.span, r,
ty::BorrowKind::from_mutbl(m), cmt_index);
}

ty::AutoBorrowObj(r, m) => {
let cmt_deref = mc.cat_deref_obj(expr, expr_cmt);
link_region(mc.typer, expr.span, r, m, cmt_deref);
link_region(mc.typer, expr.span, r,
ty::BorrowKind::from_mutbl(m), cmt_deref);
}

ty::AutoUnsafe(_) => {}
Expand All @@ -1150,7 +1163,7 @@ fn link_by_ref(rcx: &Rcx,
let mc = mc::MemCategorizationContext { typer: rcx };
let expr_cmt = ignore_err!(mc.cat_expr(expr));
let region_min = ty::ReScope(callee_scope);
link_region(mc.typer, expr.span, region_min, ast::MutImmutable, expr_cmt);
link_region(mc.typer, expr.span, region_min, ty::ImmBorrow, expr_cmt);
}

fn link_region_from_node_type(rcx: &Rcx,
Expand All @@ -1169,18 +1182,19 @@ fn link_region_from_node_type(rcx: &Rcx,
let tcx = rcx.fcx.ccx.tcx;
debug!("rptr_ty={}", ty_to_str(tcx, rptr_ty));
let r = ty::ty_region(tcx, span, rptr_ty);
link_region(rcx, span, r, mutbl, cmt_borrowed);
link_region(rcx, span, r, ty::BorrowKind::from_mutbl(mutbl),
cmt_borrowed);
}
}

fn link_region(rcx: &Rcx,
span: Span,
region_min: ty::Region,
mutbl: ast::Mutability,
kind: ty::BorrowKind,
cmt_borrowed: mc::cmt) {
/*!
* Informs the inference engine that a borrow of `cmt`
* must have mutability `mutbl` and lifetime `region_min`.
* must have the borrow kind `kind` and lifetime `region_min`.
* If `cmt` is a deref of a region pointer with
* lifetime `r_borrowed`, this will add the constraint that
* `region_min <= r_borrowed`.
Expand All @@ -1190,9 +1204,9 @@ fn link_region(rcx: &Rcx,
// for the lifetime `region_min` for the borrow to be valid:
let mut cmt_borrowed = cmt_borrowed;
loop {
debug!("link_region(region_min={}, mutbl={}, cmt_borrowed={})",
debug!("link_region(region_min={}, kind={}, cmt_borrowed={})",
region_min.repr(rcx.tcx()),
mutbl.repr(rcx.tcx()),
kind.repr(rcx.tcx()),
cmt_borrowed.repr(rcx.tcx()));
match cmt_borrowed.cat.clone() {
mc::cat_deref(base, _, mc::BorrowedPtr(_, r_borrowed)) => {
Expand All @@ -1214,7 +1228,7 @@ fn link_region(rcx: &Rcx,
adjust_upvar_borrow_kind_for_loan(
*upvar_id,
upvar_borrow,
mutbl);
kind);
infer::ReborrowUpvar(span, *upvar_id)
}
None => {
Expand All @@ -1236,7 +1250,7 @@ fn link_region(rcx: &Rcx,
r_borrowed.repr(rcx.tcx()));
rcx.fcx.mk_subr(true, cause, region_min, r_borrowed);

if mutbl == ast::MutMutable {
if kind != ty::ImmBorrow {
// If this is a mutable borrow, then the thing
// being borrowed will have to be unique.
// In user code, this means it must be an `&mut`
Expand Down Expand Up @@ -1428,12 +1442,11 @@ fn link_upvar_borrow_kind_for_nested_closures(rcx: &mut Rcx,

fn adjust_upvar_borrow_kind_for_loan(upvar_id: ty::UpvarId,
upvar_borrow: &mut ty::UpvarBorrow,
mutbl: ast::Mutability) {
kind: ty::BorrowKind) {
debug!("adjust_upvar_borrow_kind_for_loan: upvar_id={:?} kind={:?} -> {:?}",
upvar_id, upvar_borrow.kind, mutbl);
upvar_id, upvar_borrow.kind, kind);

adjust_upvar_borrow_kind(upvar_id, upvar_borrow,
ty::BorrowKind::from_mutbl(mutbl))
adjust_upvar_borrow_kind(upvar_id, upvar_borrow, kind)
}

fn adjust_upvar_borrow_kind(upvar_id: ty::UpvarId,
Expand Down
64 changes: 64 additions & 0 deletions src/test/compile-fail/borrowck-call-is-borrow-issue-12224.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Ensure that invoking a closure counts as a unique immutable borrow


type Fn<'a> = ||:'a;

struct Test<'a> {
f: ||: 'a
}

fn call(f: |Fn|) {
f(|| {
//~^ ERROR: closure requires unique access to `f` but it is already borrowed
f(|| {})
});
}

fn test1() {
call(|a| {
a();
});
}

fn test2(f: &||) {
(*f)(); //~ ERROR: closure invocation in a `&` reference
}

fn test3(f: &mut ||) {
(*f)();
}

fn test4(f: &Test) {
(f.f)() //~ ERROR: closure invocation in a `&` reference
}

fn test5(f: &mut Test) {
(f.f)()
}

fn test6() {
let f = || {};
(|| {
f();
})();
}

fn test7() {
fn foo(_: |g: |int|, b: int|) {}
let f = |g: |int|, b: int| {};
f(|a| { //~ ERROR: cannot borrow `f` as immutable because previous closure
foo(f); //~ ERROR: cannot move out of captured outer variable
}, 3);
}

fn main() {}

0 comments on commit 159a10d

Please sign in to comment.