Skip to content

Commit 707dc1f

Browse files
committed
Auto merge of #30761 - nagisa:mir-fix-destination, r=michaelwoerister
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
2 parents d70ab2b + 2f86c16 commit 707dc1f

File tree

3 files changed

+13
-4
lines changed

3 files changed

+13
-4
lines changed

src/librustc/mir/repr.rs

+11-2
Original file line numberDiff line numberDiff line change
@@ -314,10 +314,19 @@ impl<'tcx> CallKind<'tcx> {
314314
}
315315
}
316316

317-
pub fn destination(&self) -> Option<Lvalue<'tcx>> {
317+
pub fn destination(&self) -> Option<&Lvalue<'tcx>> {
318318
match *self {
319319
CallKind::Converging { ref destination, .. } |
320-
CallKind::ConvergingCleanup { ref destination, .. } => Some(destination.clone()),
320+
CallKind::ConvergingCleanup { ref destination, .. } => Some(destination),
321+
CallKind::Diverging |
322+
CallKind::DivergingCleanup(_) => None
323+
}
324+
}
325+
326+
pub fn destination_mut(&mut self) -> Option<&mut Lvalue<'tcx>> {
327+
match *self {
328+
CallKind::Converging { ref mut destination, .. } |
329+
CallKind::ConvergingCleanup { ref mut destination, .. } => Some(destination),
321330
CallKind::Diverging |
322331
CallKind::DivergingCleanup(_) => None
323332
}

src/librustc_mir/transform/erase_regions.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ impl<'a, 'tcx> EraseRegions<'a, 'tcx> {
9494
*switch_ty = self.tcx.erase_regions(switch_ty);
9595
},
9696
Terminator::Call { ref mut func, ref mut args, ref mut kind } => {
97-
if let Some(ref mut destination) = kind.destination() {
97+
if let Some(destination) = kind.destination_mut() {
9898
self.erase_regions_lvalue(destination);
9999
}
100100
self.erase_regions_operand(func);

src/librustc_trans/trans/mir/block.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
100100
let mut llargs = Vec::with_capacity(args.len() + 1);
101101

102102
// Prepare the return value destination
103-
let (ret_dest_ty, must_copy_dest) = if let Some(ref d) = kind.destination() {
103+
let (ret_dest_ty, must_copy_dest) = if let Some(d) = kind.destination() {
104104
let dest = self.trans_lvalue(bcx, d);
105105
let ret_ty = dest.ty.to_ty(bcx.tcx());
106106
if type_of::return_uses_outptr(bcx.ccx(), ret_ty) {

0 commit comments

Comments
 (0)