Skip to content

Commit

Permalink
Rollup merge of rust-lang#30761 - nagisa:mir-fix-destination, r=micha…
Browse files Browse the repository at this point in the history
…elwoerister

Previously it was returning a clone, mostly for the two reasons:

* Cloning Lvalue is very cheap most of the time (i.e. when Lvalue is not a Projection);
* There’s users who want &mut lvalue and there’s users who want &lvalue. Returning a value allows
  to make either one easier when pattern matching (i.e. Some(ref dest) or Some(ref mut dest)).

However, I’m now convinced this is an invalid approach. Namely the users which want a mutable
reference may modify the Lvalue in-place, but the changes won’t be reflected in the final MIR,
since the Lvalue modified is merely a clone.

Instead, we have two accessors `destination` and `destination_mut` which return a reference to the
destination in desired mode.

r? @nikomatsakis
  • Loading branch information
nagisa committed Jan 11, 2016
2 parents caf6095 + 2f86c16 commit 0490606
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 4 deletions.
13 changes: 11 additions & 2 deletions src/librustc/mir/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,19 @@ impl<'tcx> CallKind<'tcx> {
}
}

pub fn destination(&self) -> Option<Lvalue<'tcx>> {
pub fn destination(&self) -> Option<&Lvalue<'tcx>> {
match *self {
CallKind::Converging { ref destination, .. } |
CallKind::ConvergingCleanup { ref destination, .. } => Some(destination.clone()),
CallKind::ConvergingCleanup { ref destination, .. } => Some(destination),
CallKind::Diverging |
CallKind::DivergingCleanup(_) => None
}
}

pub fn destination_mut(&mut self) -> Option<&mut Lvalue<'tcx>> {
match *self {
CallKind::Converging { ref mut destination, .. } |
CallKind::ConvergingCleanup { ref mut destination, .. } => Some(destination),
CallKind::Diverging |
CallKind::DivergingCleanup(_) => None
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/erase_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl<'a, 'tcx> EraseRegions<'a, 'tcx> {
*switch_ty = self.tcx.erase_regions(switch_ty);
},
Terminator::Call { ref mut func, ref mut args, ref mut kind } => {
if let Some(ref mut destination) = kind.destination() {
if let Some(destination) = kind.destination_mut() {
self.erase_regions_lvalue(destination);
}
self.erase_regions_operand(func);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/trans/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
let mut llargs = Vec::with_capacity(args.len() + 1);

// Prepare the return value destination
let (ret_dest_ty, must_copy_dest) = if let Some(ref d) = kind.destination() {
let (ret_dest_ty, must_copy_dest) = if let Some(d) = kind.destination() {
let dest = self.trans_lvalue(bcx, d);
let ret_ty = dest.ty.to_ty(bcx.tcx());
if type_of::return_uses_outptr(bcx.ccx(), ret_ty) {
Expand Down

0 comments on commit 0490606

Please sign in to comment.