Skip to content

Commit 816a4d3

Browse files
pcwaltonpnkfelix
pcwalton
authored and
pnkfelix
committed
librustc: Copy or move upvars by value [breaking-change].
Closes rust-lang#12831. This breaks many closures (about 10%) that look like this: let mut sum = 0; f.map(|x| sum += x); Instead, rewrite them like this: let mut sum = 0; { let sum_ptr = &mut sum; f.map(|x| *sum_ptr += x); } Or, ideally, switch to RAII- or iterator-like patterns. [breaking-change]
1 parent 152e018 commit 816a4d3

17 files changed

+176
-673
lines changed

‎src/librustc/middle/borrowck/check_loans.rs

+1-11
Original file line numberDiff line numberDiff line change
@@ -466,10 +466,6 @@ impl<'a> CheckLoanCtxt<'a> {
466466
return;
467467
}
468468

469-
mc::cat_upvar(..) => {
470-
return;
471-
}
472-
473469
mc::cat_deref(_, _, mc::GcPtr) => {
474470
assert_eq!(cmt.mutbl, mc::McImmutable);
475471
return;
@@ -736,19 +732,13 @@ impl<'a> CheckLoanCtxt<'a> {
736732
fn check_captured_variables(&self,
737733
closure_id: ast::NodeId,
738734
span: Span) {
739-
let freevar_mode = freevars::get_capture_mode(self.tcx(), closure_id);
740735
freevars::with_freevars(self.tcx(), closure_id, |freevars| {
741736
for freevar in freevars.iter() {
742737
let var_id = ast_util::def_id_of_def(freevar.def).node;
743738
let var_path = Rc::new(LpVar(var_id));
744739
self.check_if_path_is_moved(closure_id, span,
745740
MovedInCapture, &var_path);
746-
match freevar_mode {
747-
freevars::CaptureByRef => { }
748-
freevars::CaptureByValue => {
749-
check_by_move_capture(self, closure_id, freevar, &*var_path);
750-
}
751-
}
741+
check_by_move_capture(self, closure_id, freevar, &*var_path);
752742
}
753743
});
754744
return;

‎src/librustc/middle/borrowck/gather_loans/gather_moves.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ fn check_and_get_illegal_move_origin(bccx: &BorrowckCtxt,
128128
mc::cat_deref(_, _, mc::BorrowedPtr(..)) |
129129
mc::cat_deref(_, _, mc::GcPtr) |
130130
mc::cat_deref(_, _, mc::UnsafePtr(..)) |
131-
mc::cat_upvar(..) | mc::cat_static_item |
131+
mc::cat_static_item |
132132
mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, .. }) => {
133133
Some(cmt.clone())
134134
}

‎src/librustc/middle/borrowck/gather_loans/lifetime.rs

-2
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ impl<'a> GuaranteeLifetimeContext<'a> {
7272
mc::cat_copied_upvar(..) | // L-Local
7373
mc::cat_local(..) | // L-Local
7474
mc::cat_arg(..) | // L-Local
75-
mc::cat_upvar(..) |
7675
mc::cat_deref(_, _, mc::BorrowedPtr(..)) | // L-Deref-Borrowed
7776
mc::cat_deref(_, _, mc::UnsafePtr(..)) => {
7877
self.check_scope(self.scope(cmt))
@@ -168,7 +167,6 @@ impl<'a> GuaranteeLifetimeContext<'a> {
168167
mc::cat_rvalue(temp_scope) => {
169168
temp_scope
170169
}
171-
mc::cat_upvar(..) |
172170
mc::cat_copied_upvar(_) => {
173171
ty::ReScope(self.item_scope_id)
174172
}

‎src/librustc/middle/borrowck/gather_loans/move_error.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ fn report_cannot_move_out_of(bccx: &BorrowckCtxt, move_from: mc::cmt) {
126126
mc::cat_deref(_, _, mc::BorrowedPtr(..)) |
127127
mc::cat_deref(_, _, mc::GcPtr) |
128128
mc::cat_deref(_, _, mc::UnsafePtr(..)) |
129-
mc::cat_upvar(..) | mc::cat_static_item |
129+
mc::cat_static_item |
130130
mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, .. }) => {
131131
bccx.span_err(
132132
move_from.span,

‎src/librustc/middle/borrowck/gather_loans/restrictions.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@ impl<'a> RestrictionsContext<'a> {
7373
}
7474

7575
mc::cat_local(local_id) |
76-
mc::cat_arg(local_id) |
77-
mc::cat_upvar(ty::UpvarId {var_id: local_id, ..}, _) => {
76+
mc::cat_arg(local_id) => {
7877
// R-Variable
7978
let lp = Rc::new(LpVar(local_id));
8079
SafeIf(lp.clone(), vec!(Restriction {

‎src/librustc/middle/borrowck/mod.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,7 @@ pub fn opt_loan_path(cmt: &mc::cmt) -> Option<Rc<LoanPath>> {
225225

226226
mc::cat_local(id) |
227227
mc::cat_arg(id) |
228-
mc::cat_copied_upvar(mc::CopiedUpvar { upvar_id: id, .. }) |
229-
mc::cat_upvar(ty::UpvarId {var_id: id, ..}, _) => {
228+
mc::cat_copied_upvar(mc::CopiedUpvar { upvar_id: id, .. }) => {
230229
Some(Rc::new(LpVar(id)))
231230
}
232231

‎src/librustc/middle/expr_use_visitor.rs

+35-42
Original file line numberDiff line numberDiff line change
@@ -791,53 +791,46 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> {
791791

792792
let tcx = self.typer.tcx();
793793
freevars::with_freevars(tcx, closure_expr.id, |freevars| {
794-
match freevars::get_capture_mode(self.tcx(), closure_expr.id) {
795-
freevars::CaptureByRef => {
796-
self.walk_by_ref_captures(closure_expr, freevars);
797-
}
798-
freevars::CaptureByValue => {
799-
self.walk_by_value_captures(closure_expr, freevars);
794+
for freevar in freevars.iter() {
795+
let cmt_var = return_if_err!(self.cat_captured_var(
796+
closure_expr.id,
797+
closure_expr.span,
798+
freevar.def));
799+
let upvar_id = ty::UpvarId {
800+
var_id: ast_util::def_id_of_def(freevar.def).node,
801+
closure_expr_id: closure_expr.id,
802+
};
803+
match self.typer
804+
.tcx()
805+
.upvar_borrow_map
806+
.borrow()
807+
.find(&upvar_id) {
808+
None => {
809+
self.delegate_consume(closure_expr.id,
810+
freevar.span,
811+
cmt_var);
812+
}
813+
Some(upvar_borrow) => {
814+
let region =
815+
ty::ty_region(tcx,
816+
freevar.span,
817+
upvar_borrow.reborrowed_type);
818+
// Borrow `*x`, not `x`.
819+
let upvar_cmt = self.mc.cat_deref(closure_expr,
820+
cmt_var,
821+
0);
822+
self.delegate.borrow(closure_expr.id,
823+
closure_expr.span,
824+
upvar_cmt,
825+
region,
826+
ty::UniqueImmBorrow,
827+
ClosureCapture(freevar.span));
828+
}
800829
}
801830
}
802831
});
803832
}
804833

805-
fn walk_by_ref_captures(&mut self,
806-
closure_expr: &ast::Expr,
807-
freevars: &[freevars::freevar_entry]) {
808-
for freevar in freevars.iter() {
809-
let id_var = ast_util::def_id_of_def(freevar.def).node;
810-
let cmt_var = return_if_err!(self.cat_captured_var(closure_expr.id,
811-
closure_expr.span,
812-
freevar.def));
813-
814-
// Lookup the kind of borrow the callee requires, as
815-
// inferred by regionbk
816-
let upvar_id = ty::UpvarId { var_id: id_var,
817-
closure_expr_id: closure_expr.id };
818-
let upvar_borrow = self.tcx().upvar_borrow_map.borrow()
819-
.get_copy(&upvar_id);
820-
821-
self.delegate.borrow(closure_expr.id,
822-
closure_expr.span,
823-
cmt_var,
824-
upvar_borrow.region,
825-
upvar_borrow.kind,
826-
ClosureCapture(freevar.span));
827-
}
828-
}
829-
830-
fn walk_by_value_captures(&mut self,
831-
closure_expr: &ast::Expr,
832-
freevars: &[freevars::freevar_entry]) {
833-
for freevar in freevars.iter() {
834-
let cmt_var = return_if_err!(self.cat_captured_var(closure_expr.id,
835-
closure_expr.span,
836-
freevar.def));
837-
self.delegate_consume(closure_expr.id, freevar.span, cmt_var);
838-
}
839-
}
840-
841834
fn cat_captured_var(&mut self,
842835
closure_id: ast::NodeId,
843836
closure_span: Span,

‎src/librustc/middle/freevars.rs

-19
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,6 @@ use syntax::{ast, ast_util};
2222
use syntax::visit;
2323
use syntax::visit::Visitor;
2424

25-
#[deriving(Show)]
26-
pub enum CaptureMode {
27-
/// Copy/move the value from this llvm ValueRef into the environment.
28-
CaptureByValue,
29-
30-
/// Access by reference (used for stack closures).
31-
CaptureByRef
32-
}
33-
3425
// A vector of defs representing the free variables referred to in a function.
3526
// (The def_upvar will already have been stripped).
3627
#[deriving(Encodable, Decodable)]
@@ -142,13 +133,3 @@ pub fn with_freevars<T>(tcx: &ty::ctxt, fid: ast::NodeId, f: |&[freevar_entry]|
142133
}
143134
}
144135

145-
pub fn get_capture_mode(tcx: &ty::ctxt,
146-
closure_expr_id: ast::NodeId)
147-
-> CaptureMode
148-
{
149-
let fn_ty = ty::node_id_to_type(tcx, closure_expr_id);
150-
match ty::ty_closure_store(fn_ty) {
151-
ty::RegionTraitStore(..) => CaptureByRef,
152-
ty::UniqTraitStore => CaptureByValue
153-
}
154-
}

‎src/librustc/middle/liveness.rs

+3-12
Original file line numberDiff line numberDiff line change
@@ -485,25 +485,16 @@ fn visit_expr(ir: &mut IrMaps, expr: &Expr) {
485485
let mut call_caps = Vec::new();
486486
{
487487
let call_caps_ptr = &mut call_caps;
488-
let fv_mode = freevars::get_capture_mode(ir.tcx, expr.id);
489488
freevars::with_freevars(ir.tcx, expr.id, |freevars| {
490489
for fv in freevars.iter() {
491490
match moved_variable_node_id_from_def(fv.def) {
492491
Some(rv) => {
493492
let fv_ln = ir.add_live_node(FreeVarNode(fv.span));
494493
let fv_id = ast_util::def_id_of_def(fv.def).node;
495494
let fv_ty = ty::node_id_to_type(ir.tcx, fv_id);
496-
let is_move = match fv_mode {
497-
// var must be dead afterwards
498-
freevars::CaptureByValue => {
499-
ty::type_moves_by_default(ir.tcx, fv_ty)
500-
}
501-
502-
// var can still be used
503-
freevars::CaptureByRef => {
504-
false
505-
}
506-
};
495+
// var must be dead afterwards
496+
let is_move = ty::type_moves_by_default(ir.tcx,
497+
fv_ty);
507498
call_caps_ptr.push(CaptureInfo {
508499
ln: fv_ln,
509500
is_move: is_move,

0 commit comments

Comments
 (0)