From 2f86c1605c3a82d0c82c36f1dc84441237a9f5c1 Mon Sep 17 00:00:00 2001
From: Simonas Kazlauskas <git@kazlauskas.me>
Date: Thu, 7 Jan 2016 15:08:02 +0200
Subject: [PATCH] Change destination accessor to return references
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously it was returning a value, 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.
---
 src/librustc/mir/repr.rs                    | 13 +++++++++++--
 src/librustc_mir/transform/erase_regions.rs |  2 +-
 src/librustc_trans/trans/mir/block.rs       |  2 +-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs
index 6ca39d3ba7a87..1b0dfc7322961 100644
--- a/src/librustc/mir/repr.rs
+++ b/src/librustc/mir/repr.rs
@@ -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
         }
diff --git a/src/librustc_mir/transform/erase_regions.rs b/src/librustc_mir/transform/erase_regions.rs
index 20a14cf415404..9679654d958e9 100644
--- a/src/librustc_mir/transform/erase_regions.rs
+++ b/src/librustc_mir/transform/erase_regions.rs
@@ -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);
diff --git a/src/librustc_trans/trans/mir/block.rs b/src/librustc_trans/trans/mir/block.rs
index aa0b3a25ebb0c..18a9aad0e915d 100644
--- a/src/librustc_trans/trans/mir/block.rs
+++ b/src/librustc_trans/trans/mir/block.rs
@@ -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) {