Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
librustc: Don't reuse same alloca for match on struct/tuple field whi…
…ch we reassign to in match body.
  • Loading branch information
luqmana committed Dec 5, 2014
1 parent d9c7c00 commit 2dccb5a
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 14 deletions.
57 changes: 43 additions & 14 deletions src/librustc_trans/trans/_match.rs
Expand Up @@ -1226,30 +1226,50 @@ pub fn trans_match<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,

/// Checks whether the binding in `discr` is assigned to anywhere in the expression `body`
fn is_discr_reassigned(bcx: Block, discr: &ast::Expr, body: &ast::Expr) -> bool {
match discr.node {
let (vid, field) = match discr.node {
ast::ExprPath(..) => match bcx.def(discr.id) {
def::DefLocal(vid) | def::DefUpvar(vid, _, _) => {
let mut rc = ReassignmentChecker {
node: vid,
reassigned: false
};
{
let mut visitor = euv::ExprUseVisitor::new(&mut rc, bcx);
visitor.walk_expr(body);
}
rc.reassigned
}
_ => false
def::DefLocal(vid) | def::DefUpvar(vid, _, _) => (vid, None),
_ => return false
},
ast::ExprField(ref base, field) => {
let vid = match bcx.tcx().def_map.borrow().get(&base.id) {
Some(&def::DefLocal(vid)) | Some(&def::DefUpvar(vid, _, _)) => vid,
_ => return false
};
(vid, Some(mc::NamedField(field.node.name)))
},
ast::ExprTupField(ref base, field) => {
let vid = match bcx.tcx().def_map.borrow().get(&base.id) {
Some(&def::DefLocal(vid)) | Some(&def::DefUpvar(vid, _, _)) => vid,
_ => return false
};
(vid, Some(mc::PositionalField(field.node)))
},
_ => false
_ => return false
};

let mut rc = ReassignmentChecker {
node: vid,
field: field,
reassigned: false
};
{
let mut visitor = euv::ExprUseVisitor::new(&mut rc, bcx);
visitor.walk_expr(body);
}
rc.reassigned
}

struct ReassignmentChecker {
node: ast::NodeId,
field: Option<mc::FieldName>,
reassigned: bool
}

// Determine if the expression we're matching on is reassigned to within
// the body of the match's arm.
// We only care for the `mutate` callback since this check only matters
// for cases where the matched value is moved.
impl<'tcx> euv::Delegate<'tcx> for ReassignmentChecker {
fn consume(&mut self, _: ast::NodeId, _: Span, _: mc::cmt, _: euv::ConsumeMode) {}
fn matched_pat(&mut self, _: &ast::Pat, _: mc::cmt, _: euv::MatchMode) {}
Expand All @@ -1262,6 +1282,15 @@ impl<'tcx> euv::Delegate<'tcx> for ReassignmentChecker {
match cmt.cat {
mc::cat_upvar(mc::Upvar { id: ty::UpvarId { var_id: vid, .. }, .. }) |
mc::cat_local(vid) => self.reassigned = self.node == vid,
mc::cat_interior(ref base_cmt, mc::InteriorField(field)) => {
match base_cmt.cat {
mc::cat_upvar(mc::Upvar { id: ty::UpvarId { var_id: vid, .. }, .. }) |
mc::cat_local(vid) => {
self.reassigned = self.node == vid && Some(field) == self.field
},
_ => {}
}
},
_ => {}
}
}
Expand Down
42 changes: 42 additions & 0 deletions src/test/run-pass/issue-19367.rs
@@ -0,0 +1,42 @@
// 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.

#![feature(tuple_indexing)]
struct S {
o: Option<String>
}

// Make sure we don't reuse the same alloca when matching
// on field of struct or tuple which we reassign in the match body.

fn main() {
let mut a = (0i, Some("right".into_string()));
let b = match a.1 {
Some(v) => {
a.1 = Some("wrong".into_string());
v
}
None => String::new()
};
println!("{}", b);
assert_eq!(b, "right");


let mut s = S{ o: Some("right".into_string()) };
let b = match s.o {
Some(v) => {
s.o = Some("wrong".into_string());
v
}
None => String::new(),
};
println!("{}", b);
assert_eq!(b, "right");
}

13 comments on commit 2dccb5a

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from nikomatsakis
at luqmana@2dccb5a

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging luqmana/rust/mfb = 2dccb5a into auto

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

luqmana/rust/mfb = 2dccb5a merged ok, testing candidate = a099e59a

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from nikomatsakis
at luqmana@2dccb5a

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging luqmana/rust/mfb = 2dccb5a into auto

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

luqmana/rust/mfb = 2dccb5a merged ok, testing candidate = 84266845

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from nikomatsakis
at luqmana@2dccb5a

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging luqmana/rust/mfb = 2dccb5a into auto

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

luqmana/rust/mfb = 2dccb5a merged ok, testing candidate = 77cd5cc

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 2dccb5a Dec 7, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 77cd5cc

Please sign in to comment.