From 9094aabb12573c7a4de11144f3b80bf7126e6e1b Mon Sep 17 00:00:00 2001 From: Brian Koropoff Date: Tue, 7 Oct 2014 20:04:45 -0700 Subject: [PATCH 1/4] Fix soundness bug in treatment of closure upvars by regionck - Unify the representations of `cat_upvar` and `cat_copied_upvar` - In `link_reborrowed_region`, account for the ability of upvars to change their mutability due to later processing. A map of recursive region links we may want to establish in the future is maintained, with the links being established when the kind of the borrow is adjusted. - When categorizing upvars, add an explicit deref that represents the closure environment pointer for closures that do not take the environment by value. The region for the implicit pointer is an anonymous free region type introduced for this purpose. This creates the necessary constraint to prevent unsound reborrows from the environment. - Add a note to categorizations to make it easier to tell when extra dereferences have been inserted by an upvar without having to perform deep pattern matching. - Adjust borrowck to deal with the changes. Where `cat_upvar` and `cat_copied_upvar` were previously treated differently, they are now both treated roughly like local variables within the closure body, as the explicit derefs now ensure proper behavior. However, error diagnostics had to be changed to explicitly look through the extra dereferences to avoid producing confusing messages about references not present in the source code. Closes issue #17403. Remaining work: - The error diagnostics that result from failed region inference are pretty inscrutible and should be improved. Code like the following is now rejected: let mut x = 0u; let f = || &mut x; let y = f(); let z = f(); // multiple mutable references to the same location This also breaks code that uses a similar construction even if it does not go on to violate aliasability semantics. Such code will need to be reworked in some way, such as by using a capture-by-value closure type. [breaking-change] --- src/librustc/metadata/tydecode.rs | 1 + src/librustc/metadata/tyencode.rs | 3 + src/librustc/middle/astencode.rs | 3 +- src/librustc/middle/borrowck/check_loans.rs | 47 +- .../borrowck/gather_loans/gather_moves.rs | 12 +- .../middle/borrowck/gather_loans/lifetime.rs | 4 +- .../borrowck/gather_loans/move_error.rs | 10 +- .../borrowck/gather_loans/restrictions.rs | 10 +- src/librustc/middle/borrowck/mod.rs | 54 +-- src/librustc/middle/check_static.rs | 2 - src/librustc/middle/mem_categorization.rs | 405 +++++++++++------- src/librustc/middle/trans/_match.rs | 2 +- src/librustc/middle/ty.rs | 4 + src/librustc/middle/typeck/check/regionck.rs | 146 +++++-- src/librustc/util/ppaux.rs | 6 +- 15 files changed, 443 insertions(+), 266 deletions(-) diff --git a/src/librustc/metadata/tydecode.rs b/src/librustc/metadata/tydecode.rs index c8d56c61d2b9a..fb2c8aace9514 100644 --- a/src/librustc/metadata/tydecode.rs +++ b/src/librustc/metadata/tydecode.rs @@ -277,6 +277,7 @@ fn parse_bound_region(st: &mut PState, conv: conv_did) -> ty::BoundRegion { assert_eq!(next(st), '|'); ty::BrFresh(id) } + 'e' => ty::BrEnv, _ => fail!("parse_bound_region: bad input") } } diff --git a/src/librustc/metadata/tyencode.rs b/src/librustc/metadata/tyencode.rs index e3d8d0e53757d..ee31149165211 100644 --- a/src/librustc/metadata/tyencode.rs +++ b/src/librustc/metadata/tyencode.rs @@ -175,6 +175,9 @@ fn enc_bound_region(w: &mut SeekableMemWriter, cx: &ctxt, br: ty::BoundRegion) { ty::BrFresh(id) => { mywrite!(w, "f{}|", id); } + ty::BrEnv => { + mywrite!(w, "e|"); + } } } diff --git a/src/librustc/middle/astencode.rs b/src/librustc/middle/astencode.rs index ec693679b1539..32fc045012d8c 100644 --- a/src/librustc/middle/astencode.rs +++ b/src/librustc/middle/astencode.rs @@ -516,7 +516,8 @@ impl tr for ty::BoundRegion { fn tr(&self, dcx: &DecodeContext) -> ty::BoundRegion { match *self { ty::BrAnon(_) | - ty::BrFresh(_) => *self, + ty::BrFresh(_) | + ty::BrEnv => *self, ty::BrNamed(id, ident) => ty::BrNamed(dcx.tr_def_id(id), ident), } diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 4eba46b469c7d..5f09cafb5e26e 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -775,21 +775,32 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { } // Otherwise, just a plain error. - match opt_loan_path(&assignee_cmt) { - Some(lp) => { - self.bccx.span_err( - assignment_span, - format!("cannot assign to {} {} `{}`", - assignee_cmt.mutbl.to_user_str(), - self.bccx.cmt_to_string(&*assignee_cmt), - self.bccx.loan_path_to_string(&*lp)).as_slice()); - } - None => { + match assignee_cmt.note { + mc::NoteClosureEnv(upvar_id) => { self.bccx.span_err( assignment_span, - format!("cannot assign to {} {}", - assignee_cmt.mutbl.to_user_str(), + format!("cannot assign to {}", self.bccx.cmt_to_string(&*assignee_cmt)).as_slice()); + self.bccx.span_note( + self.tcx().map.span(upvar_id.closure_expr_id), + "consider changing this closure to take self by mutable reference"); + } + _ => match opt_loan_path(&assignee_cmt) { + Some(lp) => { + self.bccx.span_err( + assignment_span, + format!("cannot assign to {} {} `{}`", + assignee_cmt.mutbl.to_user_str(), + self.bccx.cmt_to_string(&*assignee_cmt), + self.bccx.loan_path_to_string(&*lp)).as_slice()); + } + None => { + self.bccx.span_err( + assignment_span, + format!("cannot assign to {} {}", + assignee_cmt.mutbl.to_user_str(), + self.bccx.cmt_to_string(&*assignee_cmt)).as_slice()); + } } } return; @@ -805,16 +816,12 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { loop { debug!("mark_variable_as_used_mut(cmt={})", cmt.repr(this.tcx())); match cmt.cat.clone() { - mc::cat_copied_upvar(mc::CopiedUpvar { upvar_id: id, .. }) | + mc::cat_upvar(mc::Upvar { id: ty::UpvarId { var_id: id, .. }, .. }) | mc::cat_local(id) => { this.tcx().used_mut_nodes.borrow_mut().insert(id); return; } - mc::cat_upvar(..) => { - return; - } - mc::cat_rvalue(..) | mc::cat_static_item | mc::cat_deref(_, _, mc::UnsafePtr(..)) | @@ -854,12 +861,6 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { check_for_aliasability_violation(this, span, b.clone()); } - mc::cat_copied_upvar(mc::CopiedUpvar { - kind: mc::Unboxed(ty::FnUnboxedClosureKind), ..}) => { - // Prohibit writes to capture-by-move upvars in non-once closures - check_for_aliasability_violation(this, span, guarantor.clone()); - } - _ => {} } diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs index cd67169ff5690..7c037cf50ff29 100644 --- a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -133,19 +133,13 @@ fn check_and_get_illegal_move_origin(bccx: &BorrowckCtxt, mc::cat_deref(_, _, mc::BorrowedPtr(..)) | mc::cat_deref(_, _, mc::Implicit(..)) | mc::cat_deref(_, _, mc::UnsafePtr(..)) | - mc::cat_upvar(..) | mc::cat_static_item => { + mc::cat_static_item => { Some(cmt.clone()) } - mc::cat_copied_upvar(mc::CopiedUpvar { kind: kind, .. }) => { - match kind.onceness() { - ast::Once => None, - ast::Many => Some(cmt.clone()) - } - } - mc::cat_rvalue(..) | - mc::cat_local(..) => { + mc::cat_local(..) | + mc::cat_upvar(..) => { None } diff --git a/src/librustc/middle/borrowck/gather_loans/lifetime.rs b/src/librustc/middle/borrowck/gather_loans/lifetime.rs index 8ec58fe0eeedc..5018fe0f96e07 100644 --- a/src/librustc/middle/borrowck/gather_loans/lifetime.rs +++ b/src/librustc/middle/borrowck/gather_loans/lifetime.rs @@ -67,7 +67,6 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> { match cmt.cat { mc::cat_rvalue(..) | - mc::cat_copied_upvar(..) | // L-Local mc::cat_local(..) | // L-Local mc::cat_upvar(..) | mc::cat_deref(_, _, mc::BorrowedPtr(..)) | // L-Deref-Borrowed @@ -165,8 +164,7 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> { mc::cat_rvalue(temp_scope) => { temp_scope } - mc::cat_upvar(..) | - mc::cat_copied_upvar(_) => { + mc::cat_upvar(..) => { ty::ReScope(self.item_scope_id) } mc::cat_static_item => { diff --git a/src/librustc/middle/borrowck/gather_loans/move_error.rs b/src/librustc/middle/borrowck/gather_loans/move_error.rs index 29677cf897144..b1757a4eb2cbc 100644 --- a/src/librustc/middle/borrowck/gather_loans/move_error.rs +++ b/src/librustc/middle/borrowck/gather_loans/move_error.rs @@ -115,15 +115,7 @@ fn report_cannot_move_out_of(bccx: &BorrowckCtxt, move_from: mc::cmt) { mc::cat_deref(_, _, mc::BorrowedPtr(..)) | mc::cat_deref(_, _, mc::Implicit(..)) | mc::cat_deref(_, _, mc::UnsafePtr(..)) | - mc::cat_upvar(..) | mc::cat_static_item => { - bccx.span_err( - move_from.span, - format!("cannot move out of {}", - bccx.cmt_to_string(&*move_from)).as_slice()); - } - - mc::cat_copied_upvar(mc::CopiedUpvar { kind: kind, .. }) - if kind.onceness() == ast::Many => { + mc::cat_static_item => { bccx.span_err( move_from.span, format!("cannot move out of {}", diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index bf1b4b7e476ec..15a7437b6a87a 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -73,15 +73,9 @@ impl<'a, 'tcx> RestrictionsContext<'a, 'tcx> { SafeIf(lp.clone(), vec![lp]) } - mc::cat_upvar(upvar_id, _, _) => { + mc::cat_upvar(mc::Upvar { id, .. }) => { // R-Variable, captured into closure - let lp = Rc::new(LpUpvar(upvar_id)); - SafeIf(lp.clone(), vec![lp]) - } - - mc::cat_copied_upvar(mc::CopiedUpvar { upvar_id, .. }) => { - // R-Variable, copied/moved into closure - let lp = Rc::new(LpVar(upvar_id)); + let lp = Rc::new(LpUpvar(id)); SafeIf(lp.clone(), vec![lp]) } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index ec09e9e72d7ba..b4828e42cef86 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -359,21 +359,12 @@ pub fn opt_loan_path(cmt: &mc::cmt) -> Option> { None } - mc::cat_copied_upvar(mc::CopiedUpvar { kind: kind, .. }) - if kind.onceness() == ast::Many => { - None - } - mc::cat_local(id) => { Some(Rc::new(LpVar(id))) } - mc::cat_upvar(ty::UpvarId {var_id: id, closure_expr_id: proc_id}, _, _) | - mc::cat_copied_upvar(mc::CopiedUpvar { upvar_id: id, - kind: _, - capturing_proc: proc_id }) => { - let upvar_id = ty::UpvarId{ var_id: id, closure_expr_id: proc_id }; - Some(Rc::new(LpUpvar(upvar_id))) + mc::cat_upvar(mc::Upvar { id, .. }) => { + Some(Rc::new(LpUpvar(id))) } mc::cat_deref(ref cmt_base, _, pk) => { @@ -630,17 +621,22 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { pub fn bckerr_to_string(&self, err: &BckError) -> String { match err.code { err_mutbl => { - let descr = match opt_loan_path(&err.cmt) { - None => { - format!("{} {}", - err.cmt.mutbl.to_user_str(), - self.cmt_to_string(&*err.cmt)) + let descr = match err.cmt.note { + mc::NoteClosureEnv(_) => { + self.cmt_to_string(&*err.cmt) } - Some(lp) => { - format!("{} {} `{}`", - err.cmt.mutbl.to_user_str(), - self.cmt_to_string(&*err.cmt), - self.loan_path_to_string(&*lp)) + _ => match opt_loan_path(&err.cmt) { + None => { + format!("{} {}", + err.cmt.mutbl.to_user_str(), + self.cmt_to_string(&*err.cmt)) + } + Some(lp) => { + format!("{} {} `{}`", + err.cmt.mutbl.to_user_str(), + self.cmt_to_string(&*err.cmt), + self.loan_path_to_string(&*lp)) + } } }; @@ -732,8 +728,8 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { } mc::AliasableClosure(id) => { self.tcx.sess.span_err(span, - format!("{} in a free variable from an \ - immutable unboxed closure", prefix).as_slice()); + format!("{} in a captured outer \ + variable in an `Fn` closure", prefix).as_slice()); span_note!(self.tcx.sess, self.tcx.map.span(id), "consider changing this closure to take self by mutable reference"); } @@ -760,7 +756,17 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { pub fn note_and_explain_bckerr(&self, err: BckError) { let code = err.code; match code { - err_mutbl(..) => { } + err_mutbl(..) => { + match err.cmt.note { + mc::NoteClosureEnv(upvar_id) => { + self.tcx.sess.span_note( + self.tcx.map.span(upvar_id.closure_expr_id), + "consider changing this closure to take \ + self by mutable reference"); + } + _ => {} + } + } err_out_of_scope(super_scope, sub_scope) => { note_and_explain_region( diff --git a/src/librustc/middle/check_static.rs b/src/librustc/middle/check_static.rs index 9cc1f92dc9304..97acc74b8c69e 100644 --- a/src/librustc/middle/check_static.rs +++ b/src/librustc/middle/check_static.rs @@ -264,7 +264,6 @@ impl euv::Delegate for GlobalChecker { mc::cat_interior(ref cmt, _) => cur = cmt, mc::cat_rvalue(..) | - mc::cat_copied_upvar(..) | mc::cat_upvar(..) | mc::cat_local(..) => break, } @@ -299,7 +298,6 @@ impl euv::Delegate for GlobalChecker { mc::cat_downcast(..) | mc::cat_discr(..) | - mc::cat_copied_upvar(..) | mc::cat_upvar(..) | mc::cat_local(..) => unreachable!(), } diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index c5993dcb39de8..e43996559417b 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -82,9 +82,7 @@ use std::rc::Rc; pub enum categorization { cat_rvalue(ty::Region), // temporary val, argument is its scope cat_static_item, - cat_copied_upvar(CopiedUpvar), // upvar copied into proc env - cat_upvar(ty::UpvarId, ty::UpvarBorrow, - Option), // by ref upvar from stack or unboxed closure + cat_upvar(Upvar), // upvar referenced by closure env cat_local(ast::NodeId), // local variable cat_deref(cmt, uint, PointerKind), // deref of a ptr cat_interior(cmt, InteriorKind), // something interior: field, tuple, etc @@ -94,28 +92,14 @@ pub enum categorization { // (*1) downcast is only required if the enum has more than one variant } +// Represents any kind of upvar #[deriving(Clone, PartialEq, Show)] -pub enum CopiedUpvarKind { - Boxed(ast::Onceness), - Unboxed(ty::UnboxedClosureKind) -} - -impl CopiedUpvarKind { - pub fn onceness(&self) -> ast::Onceness { - match *self { - Boxed(onceness) => onceness, - Unboxed(ty::FnUnboxedClosureKind) | - Unboxed(ty::FnMutUnboxedClosureKind) => ast::Many, - Unboxed(ty::FnOnceUnboxedClosureKind) => ast::Once - } - } -} - -#[deriving(Clone, PartialEq, Show)] -pub struct CopiedUpvar { - pub upvar_id: ast::NodeId, - pub kind: CopiedUpvarKind, - pub capturing_proc: ast::NodeId, +pub struct Upvar { + pub id: ty::UpvarId, + // Unboxed closure kinds are used even for old-style closures for simplicity + pub kind: ty::UnboxedClosureKind, + // Is this from an unboxed closure? Used only for diagnostics. + pub is_unboxed: bool } // different kinds of pointers: @@ -154,6 +138,18 @@ pub enum MutabilityCategory { McInherited, // Inherited from the fact that owner is mutable. } +// A note about the provenance of a `cmt`. This is used for +// special-case handling of upvars such as mutability inference. +// Upvar categorization can generate a variable number of nested +// derefs. The note allows detecting them without deep pattern +// matching on the categorization. +#[deriving(Clone, PartialEq, Show)] +pub enum Note { + NoteClosureEnv(ty::UpvarId), // Deref through closure env + NoteUpvarRef(ty::UpvarId), // Deref through by-ref upvar + NoteNone // Nothing special +} + // `cmt`: "Category, Mutability, and Type". // // a complete categorization of a value indicating where it originated @@ -174,7 +170,8 @@ pub struct cmt_ { pub span: Span, // span of same expr/pat pub cat: categorization, // categorization of expr pub mutbl: MutabilityCategory, // mutability of expr as lvalue - pub ty: ty::t // type of the expr (*see WARNING above*) + pub ty: ty::t, // type of the expr (*see WARNING above*) + pub note: Note, // Note about the provenance of this cmt } pub type cmt = Rc; @@ -560,7 +557,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span:span, cat:cat_static_item, mutbl: McImmutable, - ty:expr_ty + ty:expr_ty, + note: NoteNone })) } @@ -570,7 +568,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span:span, cat:cat_static_item, mutbl: if mutbl { McDeclared } else { McImmutable}, - ty:expr_ty + ty:expr_ty, + note: NoteNone })) } @@ -578,55 +577,29 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { let ty = if_ok!(self.node_ty(fn_node_id)); match ty::get(ty).sty { ty::ty_closure(ref closure_ty) => { - // Decide whether to use implicit reference or by copy/move - // capture for the upvar. This, combined with the onceness, - // determines whether the closure can move out of it. - let var_is_refd = match (closure_ty.store, closure_ty.onceness) { - // Many-shot stack closures can never move out. - (ty::RegionTraitStore(..), ast::Many) => true, - // 1-shot stack closures can move out. - (ty::RegionTraitStore(..), ast::Once) => false, - // Heap closures always capture by copy/move, and can - // move out if they are once. - (ty::UniqTraitStore, _) => false, - + // Translate old closure type info into unboxed + // closure kind/capture mode + let (mode, kind) = match (closure_ty.store, closure_ty.onceness) { + // stack closure + (ty::RegionTraitStore(..), ast::Many) => { + (ast::CaptureByRef, ty::FnMutUnboxedClosureKind) + } + // proc or once closure + (_, ast::Once) => { + (ast::CaptureByValue, ty::FnOnceUnboxedClosureKind) + } + // There should be no such old closure type + (ty::UniqTraitStore, ast::Many) => { + self.tcx().sess.span_bug(span, "Impossible closure type"); + } }; - if var_is_refd { - self.cat_upvar(id, span, var_id, fn_node_id, None) - } else { - Ok(Rc::new(cmt_ { - id:id, - span:span, - cat:cat_copied_upvar(CopiedUpvar { - upvar_id: var_id, - kind: Boxed(closure_ty.onceness), - capturing_proc: fn_node_id, - }), - mutbl: MutabilityCategory::from_local(self.tcx(), var_id), - ty:expr_ty - })) - } + self.cat_upvar(id, span, var_id, fn_node_id, kind, mode, false) } ty::ty_unboxed_closure(closure_id, _) => { - let unboxed_closures = self.typer - .unboxed_closures() - .borrow(); + let unboxed_closures = self.typer.unboxed_closures().borrow(); let kind = unboxed_closures.get(&closure_id).kind; - if self.typer.capture_mode(fn_node_id) == ast::CaptureByRef { - self.cat_upvar(id, span, var_id, fn_node_id, Some(kind)) - } else { - Ok(Rc::new(cmt_ { - id: id, - span: span, - cat: cat_copied_upvar(CopiedUpvar { - upvar_id: var_id, - kind: Unboxed(kind), - capturing_proc: fn_node_id, - }), - mutbl: MutabilityCategory::from_local(self.tcx(), var_id), - ty: expr_ty - })) - } + let mode = self.typer.capture_mode(fn_node_id); + self.cat_upvar(id, span, var_id, fn_node_id, kind, mode, true) } _ => { self.tcx().sess.span_bug( @@ -644,61 +617,171 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span: span, cat: cat_local(vid), mutbl: MutabilityCategory::from_local(self.tcx(), vid), - ty: expr_ty + ty: expr_ty, + note: NoteNone })) } } } + // Categorize an upvar, complete with invisible derefs of closure + // environment and upvar reference as appropriate. fn cat_upvar(&self, id: ast::NodeId, span: Span, var_id: ast::NodeId, fn_node_id: ast::NodeId, - kind: Option) + kind: ty::UnboxedClosureKind, + mode: ast::CaptureClause, + is_unboxed: bool) -> McResult { - /*! - * Upvars through a closure are in fact indirect - * references. That is, when a closure refers to a - * variable from a parent stack frame like `x = 10`, - * that is equivalent to `*x_ = 10` where `x_` is a - * borrowed pointer (`&mut x`) created when the closure - * was created and store in the environment. This - * equivalence is expose in the mem-categorization. - */ - + // An upvar can have up to 3 components. The base is a + // `cat_upvar`. Next, we add a deref through the implicit + // environment pointer with an anonymous free region 'env and + // appropriate borrow kind for closure kinds that take self by + // reference. Finally, if the upvar was captured + // by-reference, we add a deref through that reference. The + // region of this reference is an inference variable 'up that + // was previously generated and recorded in the upvar borrow + // map. The borrow kind bk is inferred by based on how the + // upvar is used. + // + // This results in the following table for concrete closure + // types: + // + // | move | ref + // ---------------+----------------------+------------------------------- + // Fn | copied -> &'env | upvar -> &'env -> &'up bk + // FnMut | copied -> &'env mut | upvar -> &'env mut -> &'up bk + // FnOnce | copied | upvar -> &'up bk + // old stack | N/A | upvar -> &'env mut -> &'up bk + // old proc/once | copied | N/A let upvar_id = ty::UpvarId { var_id: var_id, closure_expr_id: fn_node_id }; - let upvar_borrow = self.typer.upvar_borrow(upvar_id); + // Do we need to deref through an env reference? + let has_env_deref = kind != ty::FnOnceUnboxedClosureKind; - let var_ty = if_ok!(self.node_ty(var_id)); + // Mutability of original variable itself + let var_mutbl = MutabilityCategory::from_local(self.tcx(), var_id); - // We can't actually represent the types of all upvars - // as user-describable types, since upvars support const - // and unique-imm borrows! Therefore, we cheat, and just - // give err type. Nobody should be inspecting this type anyhow. - let upvar_ty = ty::mk_err(); + // Mutability of environment dereference + let env_mutbl = match kind { + ty::FnOnceUnboxedClosureKind => var_mutbl, + ty::FnMutUnboxedClosureKind => McInherited, + ty::FnUnboxedClosureKind => McImmutable + }; - let base_cmt = Rc::new(cmt_ { - id:id, - span:span, - cat:cat_upvar(upvar_id, upvar_borrow, kind), - mutbl:McImmutable, - ty:upvar_ty, + // Look up the node ID of the closure body so we can construct + // a free region within it + let fn_body_id = { + let fn_expr = match self.tcx().map.find(fn_node_id) { + Some(ast_map::NodeExpr(e)) => e, + _ => unreachable!() + }; + + match fn_expr.node { + ast::ExprFnBlock(_, _, ref body) | + ast::ExprProc(_, ref body) | + ast::ExprUnboxedFn(_, _, _, ref body) => body.id, + _ => unreachable!() + } + }; + + // Region of environment pointer + let env_region = ty::ReFree(ty::FreeRegion { + scope_id: fn_body_id, + bound_region: ty::BrEnv }); - let ptr = BorrowedPtr(upvar_borrow.kind, upvar_borrow.region); + let env_ptr = BorrowedPtr(if env_mutbl.is_mutable() { + ty::MutBorrow + } else { + ty::ImmBorrow + }, env_region); - let deref_cmt = Rc::new(cmt_ { - id:id, - span:span, - cat:cat_deref(base_cmt, 0, ptr), - mutbl:MutabilityCategory::from_borrow_kind(upvar_borrow.kind), - ty:var_ty, - }); + let var_ty = if_ok!(self.node_ty(var_id)); + + // First, switch by capture mode + Ok(match mode { + ast::CaptureByValue => { + let mut base = cmt_ { + id: id, + span: span, + cat: cat_upvar(Upvar { + id: upvar_id, + kind: kind, + is_unboxed: is_unboxed + }), + mutbl: var_mutbl, + ty: var_ty, + note: NoteNone + }; - Ok(deref_cmt) + if has_env_deref { + // We need to add the env deref. This means that + // the above is actually immutable and has a ref + // type. However, nothing should actually look at + // the type, so we can get away with stuffing a + // `ty_err` in there instead of bothering to + // construct a proper one. + base.mutbl = McImmutable; + base.ty = ty::mk_err(); + Rc::new(cmt_ { + id: id, + span: span, + cat: cat_deref(Rc::new(base), 0, env_ptr), + mutbl: env_mutbl, + ty: var_ty, + note: NoteClosureEnv(upvar_id) + }) + } else { + Rc::new(base) + } + }, + ast::CaptureByRef => { + // The type here is actually a ref (or ref of a ref), + // but we can again get away with not constructing one + // properly since it will never be used. + let mut base = cmt_ { + id: id, + span: span, + cat: cat_upvar(Upvar { + id: upvar_id, + kind: kind, + is_unboxed: is_unboxed + }), + mutbl: McImmutable, + ty: ty::mk_err(), + note: NoteNone + }; + + // As in the by-value case, add env deref if needed + if has_env_deref { + base = cmt_ { + id: id, + span: span, + cat: cat_deref(Rc::new(base), 0, env_ptr), + mutbl: env_mutbl, + ty: ty::mk_err(), + note: NoteClosureEnv(upvar_id) + }; + } + + // Look up upvar borrow so we can get its region + let upvar_borrow = self.typer.upvar_borrow(upvar_id); + let ptr = BorrowedPtr(upvar_borrow.kind, upvar_borrow.region); + + Rc::new(cmt_ { + id: id, + span: span, + cat: cat_deref(Rc::new(base), 0, ptr), + mutbl: MutabilityCategory::from_borrow_kind(upvar_borrow.kind), + ty: var_ty, + note: NoteUpvarRef(upvar_id) + }) + } + }) } pub fn cat_rvalue_node(&self, @@ -729,7 +812,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span:span, cat:cat_rvalue(temp_scope), mutbl:McDeclared, - ty:expr_ty + ty:expr_ty, + note: NoteNone }) } @@ -744,7 +828,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span: node.span(), mutbl: base_cmt.mutbl.inherit(), cat: cat_interior(base_cmt, InteriorField(NamedField(f_name.name))), - ty: f_ty + ty: f_ty, + note: NoteNone }) } @@ -759,7 +844,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span: node.span(), mutbl: base_cmt.mutbl.inherit(), cat: cat_interior(base_cmt, InteriorField(PositionalField(f_idx))), - ty: f_ty + ty: f_ty, + note: NoteNone }) } @@ -834,7 +920,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span: node.span(), cat: cat, mutbl: m, - ty: deref_ty + ty: deref_ty, + note: NoteNone }) } @@ -895,7 +982,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span:elt.span(), cat:cat_interior(of_cmt, InteriorElement(element_kind(vec_ty))), mutbl:mutbl, - ty:element_ty + ty:element_ty, + note: NoteNone }) } } @@ -921,7 +1009,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { ty: match ty::deref(base_cmt.ty, false) { Some(mt) => mt.ty, None => self.tcx().sess.bug("Found non-derefable type") - } + }, + note: NoteNone }) } @@ -988,7 +1077,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span: node.span(), mutbl: base_cmt.mutbl.inherit(), cat: cat_interior(base_cmt, interior), - ty: interior_ty + ty: interior_ty, + note: NoteNone }) } @@ -1002,7 +1092,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { span: node.span(), mutbl: base_cmt.mutbl.inherit(), cat: cat_downcast(base_cmt), - ty: downcast_ty + ty: downcast_ty, + note: NoteNone }) } @@ -1184,13 +1275,26 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { } pub fn cmt_to_string(&self, cmt: &cmt_) -> String { + fn upvar_to_string(upvar: &Upvar, is_copy: bool) -> String { + if upvar.is_unboxed { + let kind = match upvar.kind { + ty::FnUnboxedClosureKind => "Fn", + ty::FnMutUnboxedClosureKind => "FnMut", + ty::FnOnceUnboxedClosureKind => "FnOnce" + }; + format!("captured outer variable in an `{}` closure", kind) + } else { + (match (upvar.kind, is_copy) { + (ty::FnOnceUnboxedClosureKind, true) => "captured outer variable in a proc", + _ => "captured outer variable" + }).to_string() + } + } + match cmt.cat { cat_static_item => { "static item".to_string() } - cat_copied_upvar(_) => { - "captured outer variable in a proc".to_string() - } cat_rvalue(..) => { "non-lvalue".to_string() } @@ -1202,12 +1306,14 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { _ => "local variable".to_string() } } - cat_deref(ref base, _, pk) => { - match base.cat { - cat_upvar(..) => { - "captured outer variable".to_string() + cat_deref(_, _, pk) => { + let upvar = cmt.upvar(); + match upvar.as_ref().map(|i| &i.cat) { + Some(&cat_upvar(ref var)) => { + upvar_to_string(var, false) } - _ => { + Some(_) => unreachable!(), + None => { match pk { Implicit(..) => { "dereference (dereference is implicit, due to indexing)".to_string() @@ -1230,8 +1336,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> { cat_interior(_, InteriorElement(OtherElement)) => { "indexed content".to_string() } - cat_upvar(..) => { - "captured outer variable".to_string() + cat_upvar(ref var) => { + upvar_to_string(var, true) } cat_discr(ref cmt, _) => { self.cmt_to_string(&**cmt) @@ -1250,7 +1356,7 @@ pub enum InteriorSafety { pub enum AliasableReason { AliasableBorrowed, - AliasableClosure(ast::NodeId), // Aliasable due to capture by unboxed closure expr + AliasableClosure(ast::NodeId), // Aliasable due to capture Fn closure env AliasableOther, AliasableStatic(InteriorSafety), AliasableStaticMut(InteriorSafety), @@ -1265,7 +1371,6 @@ impl cmt_ { match self.cat { cat_rvalue(..) | cat_static_item | - cat_copied_upvar(..) | cat_local(..) | cat_deref(_, _, UnsafePtr(..)) | cat_deref(_, _, BorrowedPtr(..)) | @@ -1307,27 +1412,11 @@ impl cmt_ { cat_rvalue(..) | cat_local(..) | + cat_upvar(..) | cat_deref(_, _, UnsafePtr(..)) => { // yes, it's aliasable, but... None } - cat_copied_upvar(CopiedUpvar {kind: kind, capturing_proc: id, ..}) => { - match kind { - Boxed(ast::Once) | - Unboxed(ty::FnOnceUnboxedClosureKind) | - Unboxed(ty::FnMutUnboxedClosureKind) => None, - Boxed(_) => Some(AliasableOther), - Unboxed(_) => Some(AliasableClosure(id)) - } - } - - cat_upvar(ty::UpvarId { closure_expr_id: id, .. }, _, - Some(ty::FnUnboxedClosureKind)) => { - Some(AliasableClosure(id)) - } - - cat_upvar(..) => None, - cat_static_item(..) => { let int_safe = if ty::type_interior_is_unsafe(ctxt, self.ty) { InteriorUnsafe @@ -1342,10 +1431,33 @@ impl cmt_ { } } - cat_deref(_, _, BorrowedPtr(ty::ImmBorrow, _)) | - cat_deref(_, _, Implicit(ty::ImmBorrow, _)) => { - Some(AliasableBorrowed) + cat_deref(ref base, _, BorrowedPtr(ty::ImmBorrow, _)) | + cat_deref(ref base, _, Implicit(ty::ImmBorrow, _)) => { + match base.cat { + cat_upvar(Upvar{ id, .. }) => Some(AliasableClosure(id.closure_expr_id)), + _ => Some(AliasableBorrowed) + } + } + } + } + + // Digs down through one or two layers of deref and grabs the cmt + // for the upvar if a note indicates there is one. + pub fn upvar(&self) -> Option { + match self.note { + NoteClosureEnv(..) | NoteUpvarRef(..) => { + Some(match self.cat { + cat_deref(ref inner, _, _) => { + match inner.cat { + cat_deref(ref inner, _, _) => inner.clone(), + cat_upvar(..) => inner.clone(), + _ => unreachable!() + } + } + _ => unreachable!() + }) } + NoteNone => None } } } @@ -1365,7 +1477,6 @@ impl Repr for categorization { match *self { cat_static_item | cat_rvalue(..) | - cat_copied_upvar(..) | cat_local(..) | cat_upvar(..) => { format!("{}", *self) diff --git a/src/librustc/middle/trans/_match.rs b/src/librustc/middle/trans/_match.rs index c31a3730a7c36..0ff9f9f26e867 100644 --- a/src/librustc/middle/trans/_match.rs +++ b/src/librustc/middle/trans/_match.rs @@ -1270,7 +1270,7 @@ impl euv::Delegate for ReassignmentChecker { fn mutate(&mut self, _: ast::NodeId, _: Span, cmt: mc::cmt, _: euv::MutateMode) { match cmt.cat { - mc::cat_copied_upvar(mc::CopiedUpvar { upvar_id: vid, .. }) | + mc::cat_upvar(mc::Upvar { id: ty::UpvarId { var_id: vid, .. }, .. }) | mc::cat_local(vid) => self.reassigned = self.node == vid, _ => {} } diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index b84bfe9522446..832e9a79acae0 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -863,6 +863,10 @@ pub enum BoundRegion { /// Fresh bound identifiers created during GLB computations. BrFresh(uint), + + // Anonymous region for the implicit env pointer parameter + // to a closure + BrEnv } mod primitives { diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index b810ea3d94de6..b595b9b84aed8 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -140,7 +140,9 @@ use syntax::codemap::Span; use syntax::visit; use syntax::visit::Visitor; -use std::cell::RefCell; +use std::cell::{RefCell}; +use std::collections::HashMap; +use std::collections::hashmap::{Vacant, Occupied}; /////////////////////////////////////////////////////////////////////////// // PUBLIC ENTRY POINTS @@ -211,6 +213,19 @@ macro_rules! ignore_err( ) ) +// Stores parameters for a potential call to link_region() +// to perform if an upvar reference is marked unique/mutable after +// it has already been processed before. +struct MaybeLink { + span: Span, + borrow_region: ty::Region, + borrow_kind: ty::BorrowKind, + borrow_cmt: mc::cmt +} + +// A map associating an upvar ID to a vector of the above +type MaybeLinkMap = RefCell>>; + pub struct Rcx<'a, 'tcx: 'a> { fcx: &'a FnCtxt<'a, 'tcx>, @@ -218,6 +233,10 @@ pub struct Rcx<'a, 'tcx: 'a> { // id of innermost fn or loop repeating_scope: ast::NodeId, + + // Possible region links we will establish if an upvar + // turns out to be unique/mutable + maybe_links: MaybeLinkMap } fn region_of_def(fcx: &FnCtxt, def: def::Def) -> ty::Region { @@ -250,7 +269,8 @@ impl<'a, 'tcx> Rcx<'a, 'tcx> { initial_repeating_scope: ast::NodeId) -> Rcx<'a, 'tcx> { Rcx { fcx: fcx, repeating_scope: initial_repeating_scope, - region_param_pairs: Vec::new() } + region_param_pairs: Vec::new(), + maybe_links: RefCell::new(HashMap::new()) } } pub fn tcx(&self) -> &'a ty::ctxt<'tcx> { @@ -848,14 +868,13 @@ fn check_expr_fn_block(rcx: &mut Rcx, }); } ty::ty_unboxed_closure(_, region) => { - let bounds = ty::region_existential_bound(region); if tcx.capture_modes.borrow().get_copy(&expr.id) == ast::CaptureByRef { ty::with_freevars(tcx, expr.id, |freevars| { if !freevars.is_empty() { // Variables being referenced must be constrained and registered // in the upvar borrow map constrain_free_variables_in_by_ref_closure( - rcx, bounds.region_bound, expr, freevars); + rcx, region, expr, freevars); } }) } @@ -1463,7 +1482,8 @@ fn link_region(rcx: &Rcx, mc::BorrowedPtr(ref_kind, ref_region)) => { match link_reborrowed_region(rcx, span, borrow_region, borrow_kind, - ref_cmt, ref_region, ref_kind) { + ref_cmt, ref_region, ref_kind, + borrow_cmt.note) { Some((c, k)) => { borrow_cmt = c; borrow_kind = k; @@ -1486,9 +1506,8 @@ fn link_region(rcx: &Rcx, mc::cat_deref(_, _, mc::UnsafePtr(..)) | mc::cat_static_item | - mc::cat_copied_upvar(..) | - mc::cat_local(..) | mc::cat_upvar(..) | + mc::cat_local(..) | mc::cat_rvalue(..) => { // These are all "base cases" with independent lifetimes // that are not subject to inference @@ -1504,7 +1523,8 @@ fn link_reborrowed_region(rcx: &Rcx, borrow_kind: ty::BorrowKind, ref_cmt: mc::cmt, ref_region: ty::Region, - ref_kind: ty::BorrowKind) + mut ref_kind: ty::BorrowKind, + note: mc::Note) -> Option<(mc::cmt, ty::BorrowKind)> { /*! @@ -1550,9 +1570,12 @@ fn link_reborrowed_region(rcx: &Rcx, * recurse and process `ref_cmt` (see case 2 above). */ - // Detect references to an upvar `x`: - let cause = match ref_cmt.cat { - mc::cat_upvar(ref upvar_id, _, _) => { + // Possible upvar ID we may need later to create an entry in the + // maybe link map. + + // Detect by-ref upvar `x`: + let cause = match note { + mc::NoteUpvarRef(ref upvar_id) => { let mut upvar_borrow_map = rcx.fcx.inh.upvar_borrow_map.borrow_mut(); match upvar_borrow_map.find_mut(upvar_id) { @@ -1560,10 +1583,15 @@ fn link_reborrowed_region(rcx: &Rcx, // Adjust mutability that we infer for the upvar // so it can accommodate being borrowed with // mutability `kind`: - adjust_upvar_borrow_kind_for_loan(*upvar_id, + adjust_upvar_borrow_kind_for_loan(rcx, + *upvar_id, upvar_borrow, borrow_kind); + // The mutability of the upvar may have been modified + // by the above adjustment, so update our local variable. + ref_kind = upvar_borrow.kind; + infer::ReborrowUpvar(span, *upvar_id) } None => { @@ -1575,7 +1603,12 @@ fn link_reborrowed_region(rcx: &Rcx, } } } - + mc::NoteClosureEnv(ref upvar_id) => { + // We don't have any mutability changes to propagate, but + // we do want to note that an upvar reborrow caused this + // link + infer::ReborrowUpvar(span, *upvar_id) + } _ => { infer::Reborrow(span) } @@ -1586,6 +1619,21 @@ fn link_reborrowed_region(rcx: &Rcx, ref_region.repr(rcx.tcx())); rcx.fcx.mk_subr(cause, borrow_region, ref_region); + // If we end up needing to recurse and establish a region link + // with `ref_cmt`, calculate what borrow kind we will end up + // needing. This will be used below. + // + // One interesting twist is that we can weaken the borrow kind + // when we recurse: to reborrow an `&mut` referent as mutable, + // borrowck requires a unique path to the `&mut` reference but not + // necessarily a *mutable* path. + let new_borrow_kind = match borrow_kind { + ty::ImmBorrow => + ty::ImmBorrow, + ty::MutBorrow | ty::UniqueImmBorrow => + ty::UniqueImmBorrow + }; + // Decide whether we need to recurse and link any regions within // the `ref_cmt`. This is concerned for the case where the value // being reborrowed is in fact a borrowed pointer found within @@ -1624,23 +1672,36 @@ fn link_reborrowed_region(rcx: &Rcx, // else the user is borrowed imm memory as mut memory, // which means they'll get an error downstream in borrowck // anyhow.) + // + // If mutability was inferred from an upvar, we may be + // forced to revisit this decision later if processing + // another borrow or nested closure ends up coverting the + // upvar borrow kind to mutable/unique. Record the + // information needed to perform the recursive link in the + // maybe link map. + match note { + mc::NoteUpvarRef(upvar_id) => { + let link = MaybeLink { + span: span, + borrow_region: borrow_region, + borrow_kind: new_borrow_kind, + borrow_cmt: ref_cmt + }; + + match rcx.maybe_links.borrow_mut().entry(upvar_id) { + Vacant(entry) => { entry.set(vec![link]); } + Occupied(entry) => { entry.into_mut().push(link); } + } + }, + _ => {} + } + return None; } ty::MutBorrow | ty::UniqueImmBorrow => { // The reference being reborrowed is either an `&mut T` or // `&uniq T`. This is the case where recursion is needed. - // - // One interesting twist is that we can weaken the borrow - // kind when we recurse: to reborrow an `&mut` referent as - // mutable, borrowck requires a unique path to the `&mut` - // reference but not necessarily a *mutable* path. - let new_borrow_kind = match borrow_kind { - ty::ImmBorrow => - ty::ImmBorrow, - ty::MutBorrow | ty::UniqueImmBorrow => - ty::UniqueImmBorrow - }; return Some((ref_cmt, new_borrow_kind)); } } @@ -1685,8 +1746,8 @@ fn adjust_upvar_borrow_kind_for_mut(rcx: &Rcx, mc::cat_deref(base, _, mc::BorrowedPtr(..)) | mc::cat_deref(base, _, mc::Implicit(..)) => { - match base.cat { - mc::cat_upvar(ref upvar_id, _, _) => { + match cmt.note { + mc::NoteUpvarRef(ref upvar_id) => { // if this is an implicit deref of an // upvar, then we need to modify the // borrow_kind of the upvar to make sure it @@ -1694,7 +1755,7 @@ fn adjust_upvar_borrow_kind_for_mut(rcx: &Rcx, let mut upvar_borrow_map = rcx.fcx.inh.upvar_borrow_map.borrow_mut(); let ub = upvar_borrow_map.get_mut(upvar_id); - return adjust_upvar_borrow_kind(*upvar_id, ub, ty::MutBorrow); + return adjust_upvar_borrow_kind(rcx, *upvar_id, ub, ty::MutBorrow); } _ => {} @@ -1710,7 +1771,6 @@ fn adjust_upvar_borrow_kind_for_mut(rcx: &Rcx, mc::cat_deref(_, _, mc::UnsafePtr(..)) | mc::cat_static_item | mc::cat_rvalue(_) | - mc::cat_copied_upvar(_) | mc::cat_local(_) | mc::cat_upvar(..) => { return; @@ -1738,15 +1798,15 @@ fn adjust_upvar_borrow_kind_for_unique(rcx: &Rcx, cmt: mc::cmt) { mc::cat_deref(base, _, mc::BorrowedPtr(..)) | mc::cat_deref(base, _, mc::Implicit(..)) => { - match base.cat { - mc::cat_upvar(ref upvar_id, _, _) => { + match cmt.note { + mc::NoteUpvarRef(ref upvar_id) => { // if this is an implicit deref of an // upvar, then we need to modify the // borrow_kind of the upvar to make sure it // is inferred to unique if necessary let mut ub = rcx.fcx.inh.upvar_borrow_map.borrow_mut(); let ub = ub.get_mut(upvar_id); - return adjust_upvar_borrow_kind(*upvar_id, ub, ty::UniqueImmBorrow); + return adjust_upvar_borrow_kind(rcx, *upvar_id, ub, ty::UniqueImmBorrow); } _ => {} @@ -1760,7 +1820,6 @@ fn adjust_upvar_borrow_kind_for_unique(rcx: &Rcx, cmt: mc::cmt) { mc::cat_deref(_, _, mc::UnsafePtr(..)) | mc::cat_static_item | mc::cat_rvalue(_) | - mc::cat_copied_upvar(_) | mc::cat_local(_) | mc::cat_upvar(..) => { return; @@ -1786,22 +1845,24 @@ fn link_upvar_borrow_kind_for_nested_closures(rcx: &mut Rcx, let inner_borrow = upvar_borrow_map.get_copy(&inner_upvar_id); match upvar_borrow_map.find_mut(&outer_upvar_id) { Some(outer_borrow) => { - adjust_upvar_borrow_kind(outer_upvar_id, outer_borrow, inner_borrow.kind); + adjust_upvar_borrow_kind(rcx, outer_upvar_id, outer_borrow, inner_borrow.kind); } None => { /* outer closure is not a stack closure */ } } } -fn adjust_upvar_borrow_kind_for_loan(upvar_id: ty::UpvarId, +fn adjust_upvar_borrow_kind_for_loan(rcx: &Rcx, + upvar_id: ty::UpvarId, upvar_borrow: &mut ty::UpvarBorrow, kind: ty::BorrowKind) { debug!("adjust_upvar_borrow_kind_for_loan: upvar_id={} kind={} -> {}", upvar_id, upvar_borrow.kind, kind); - adjust_upvar_borrow_kind(upvar_id, upvar_borrow, kind) + adjust_upvar_borrow_kind(rcx, upvar_id, upvar_borrow, kind) } -fn adjust_upvar_borrow_kind(upvar_id: ty::UpvarId, +fn adjust_upvar_borrow_kind(rcx: &Rcx, + upvar_id: ty::UpvarId, upvar_borrow: &mut ty::UpvarBorrow, kind: ty::BorrowKind) { /*! @@ -1821,6 +1882,19 @@ fn adjust_upvar_borrow_kind(upvar_id: ty::UpvarId, (ty::ImmBorrow, ty::MutBorrow) | (ty::UniqueImmBorrow, ty::MutBorrow) => { upvar_borrow.kind = kind; + + // Check if there are any region links we now need to + // establish due to adjusting the borrow kind of the upvar + match rcx.maybe_links.borrow_mut().entry(upvar_id) { + Occupied(entry) => { + for MaybeLink { span, borrow_region, + borrow_kind, borrow_cmt } in entry.take().into_iter() + { + link_region(rcx, span, borrow_region, borrow_kind, borrow_cmt); + } + } + Vacant(_) => {} + } } // Take LHS: (ty::ImmBorrow, ty::ImmBorrow) | diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index 404864cec04e4..085295cad7d2a 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -15,7 +15,7 @@ use middle::subst; use middle::ty::{BoundRegion, BrAnon, BrNamed}; use middle::ty::{ReEarlyBound, BrFresh, ctxt}; use middle::ty::{ReFree, ReScope, ReInfer, ReStatic, Region, ReEmpty}; -use middle::ty::{ReSkolemized, ReVar}; +use middle::ty::{ReSkolemized, ReVar, BrEnv}; use middle::ty::{mt, t, ParamTy}; use middle::ty::{ty_bool, ty_char, ty_bot, ty_struct, ty_enum}; use middle::ty::{ty_err, ty_str, ty_vec, ty_float, ty_bare_fn, ty_closure}; @@ -183,8 +183,7 @@ pub fn bound_region_to_string(cx: &ctxt, BrNamed(_, name) => { format!("{}{}{}", prefix, token::get_name(name), space_str) } - BrAnon(_) => prefix.to_string(), - BrFresh(_) => prefix.to_string(), + BrAnon(_) | BrFresh(_) | BrEnv => prefix.to_string() } } @@ -769,6 +768,7 @@ impl Repr for ty::BoundRegion { format!("BrNamed({}, {})", id.repr(tcx), token::get_name(name)) } ty::BrFresh(id) => format!("BrFresh({})", id), + ty::BrEnv => "BrEnv".to_string() } } } From a8f90bcb1871a8658375195f676d3706ab29a3a5 Mon Sep 17 00:00:00 2001 From: Brian Koropoff Date: Sat, 11 Oct 2014 20:10:14 -0700 Subject: [PATCH 2/4] Update test for issue 17780 since diagnostic message have changed The test was also renamed to be more descriptive. --- ....rs => borrow-immutable-upvar-mutation.rs} | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) rename src/test/compile-fail/{issue-17780.rs => borrow-immutable-upvar-mutation.rs} (50%) diff --git a/src/test/compile-fail/issue-17780.rs b/src/test/compile-fail/borrow-immutable-upvar-mutation.rs similarity index 50% rename from src/test/compile-fail/issue-17780.rs rename to src/test/compile-fail/borrow-immutable-upvar-mutation.rs index 2072b2ee2d2c5..f748c40065447 100644 --- a/src/test/compile-fail/issue-17780.rs +++ b/src/test/compile-fail/borrow-immutable-upvar-mutation.rs @@ -10,41 +10,32 @@ #![feature(unboxed_closures, overloaded_calls)] +// Tests that we can't assign to or mutably borrow upvars from `Fn` +// closures (issue #17780) + fn set(x: &mut uint) { *x = 5; } fn main() { // By-ref captures { let mut x = 0u; - let _f = |&:| x = 42; - //~^ ERROR cannot assign to data in a free - // variable from an immutable unboxed closure + let _f = |&:| x = 42; //~ ERROR cannot assign let mut y = 0u; - let _g = |&:| set(&mut y); - //~^ ERROR cannot borrow data mutably in a free - // variable from an immutable unboxed closure + let _g = |&:| set(&mut y); //~ ERROR cannot borrow let mut z = 0u; - let _h = |&mut:| { set(&mut z); |&:| z = 42; }; - //~^ ERROR cannot assign to data in a - // free variable from an immutable unboxed closure + let _h = |&mut:| { set(&mut z); |&:| z = 42; }; //~ ERROR cannot assign } // By-value captures { let mut x = 0u; - let _f = move |&:| x = 42; - //~^ ERROR cannot assign to data in a free - // variable from an immutable unboxed closure + let _f = move |&:| x = 42; //~ ERROR cannot assign let mut y = 0u; - let _g = move |&:| set(&mut y); - //~^ ERROR cannot borrow data mutably in a free - // variable from an immutable unboxed closure + let _g = move |&:| set(&mut y); //~ ERROR cannot borrow let mut z = 0u; - let _h = move |&mut:| { set(&mut z); move |&:| z = 42; }; - //~^ ERROR cannot assign to data in a free - // variable from an immutable unboxed closure + let _h = move |&mut:| { set(&mut z); move |&:| z = 42; }; //~ ERROR cannot assign } } From a5e1aeb14020a7501c1f1a4780f0626cb6f80a3f Mon Sep 17 00:00:00 2001 From: Brian Koropoff Date: Wed, 8 Oct 2014 00:08:30 -0700 Subject: [PATCH 3/4] Add regression test for issue #17403 --- ...regions-return-ref-to-upvar-issue-17403.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/test/compile-fail/regions-return-ref-to-upvar-issue-17403.rs diff --git a/src/test/compile-fail/regions-return-ref-to-upvar-issue-17403.rs b/src/test/compile-fail/regions-return-ref-to-upvar-issue-17403.rs new file mode 100644 index 0000000000000..aedaced5794a7 --- /dev/null +++ b/src/test/compile-fail/regions-return-ref-to-upvar-issue-17403.rs @@ -0,0 +1,30 @@ +// 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. + +// Test that closures cannot subvert aliasing restrictions + +#![feature(overloaded_calls, unboxed_closures)] + +fn main() { + // Unboxed closure case + { + let mut x = 0u; + let mut f = |&mut:| &mut x; //~ ERROR cannot infer + let x = f(); + let y = f(); + } + // Boxed closure case + { + let mut x = 0u; + let f = || &mut x; //~ ERROR cannot infer + let x = f(); + let y = f(); + } +} From fdd69accd0c9366b882a0e0d93ddb94eee307431 Mon Sep 17 00:00:00 2001 From: Brian Koropoff Date: Tue, 14 Oct 2014 18:35:40 -0700 Subject: [PATCH 4/4] Add failure tests for moving out of unboxed closure environments --- .../unboxed-closure-illegal-move.rs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 src/test/compile-fail/unboxed-closure-illegal-move.rs diff --git a/src/test/compile-fail/unboxed-closure-illegal-move.rs b/src/test/compile-fail/unboxed-closure-illegal-move.rs new file mode 100644 index 0000000000000..9e981f2c9bb02 --- /dev/null +++ b/src/test/compile-fail/unboxed-closure-illegal-move.rs @@ -0,0 +1,44 @@ +// 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. + +#![feature(unboxed_closures)] + +// Tests that we can't move out of an unboxed closure environment +// if the upvar is captured by ref or the closure takes self by +// reference. + +fn main() { + // By-ref cases + { + let x = box 0u; + let f = |&:| drop(x); //~ cannot move + } + { + let x = box 0u; + let f = |&mut:| drop(x); //~ cannot move + } + { + let x = box 0u; + let f = |:| drop(x); //~ cannot move + } + // By-value cases + { + let x = box 0u; + let f = move |&:| drop(x); //~ cannot move + } + { + let x = box 0u; + let f = move |&mut:| drop(x); //~ cannot move + } + { + let x = box 0u; + let f = move |:| drop(x); // this one is ok + } +}