From 159a10da4c15e5d34e00d4018b352573cec7918f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 21 Apr 2014 22:02:19 -0700 Subject: [PATCH] rustc: Tweak the borrow on closure invocations 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 #12224 [breaking-change] --- src/librustc/middle/borrowck/check_loans.rs | 2 +- .../middle/borrowck/gather_loans/mod.rs | 20 ++++++ src/librustc/middle/borrowck/mod.rs | 9 +++ src/librustc/middle/typeck/check/regionck.rs | 49 ++++++++------ .../borrowck-call-is-borrow-issue-12224.rs | 64 +++++++++++++++++++ 5 files changed, 125 insertions(+), 19 deletions(-) create mode 100644 src/test/compile-fail/borrowck-call-is-borrow-issue-12224.rs diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index e09507f5d5f..d0f47966832 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -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)) } diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index 7f748dffd70..8bb95b798d0 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -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, ()); } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 06491d36b02..3de64f15191 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -202,6 +202,7 @@ pub enum LoanCause { AddrOf, AutoRef, RefBinding, + ClosureInvocation, } #[deriving(Eq, TotalEq, Hash)] @@ -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(..) => { @@ -677,6 +682,10 @@ impl<'a> BorrowckCtxt<'a> { BorrowViolation(RefBinding) => { "cannot borrow data mutably" } + + BorrowViolation(ClosureInvocation) => { + "closure invocation" + } }; match cause { diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index 94cbd3a3a75..b59da8910af 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -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), @@ -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. @@ -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(()) => {} } @@ -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(_) => {} @@ -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, @@ -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`. @@ -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)) => { @@ -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 => { @@ -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` @@ -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, diff --git a/src/test/compile-fail/borrowck-call-is-borrow-issue-12224.rs b/src/test/compile-fail/borrowck-call-is-borrow-issue-12224.rs new file mode 100644 index 00000000000..002ae5a7d28 --- /dev/null +++ b/src/test/compile-fail/borrowck-call-is-borrow-issue-12224.rs @@ -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 or the MIT license +// , 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() {}