Change destination accessor to return references

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.

diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs
index 6ca39d3ba7a8758f7481adee2297fc9b7c3c3db6..1b0dfc73229613a56babad1cd099123edd233fe2 100644 (file)
--- a/src/librustc/mir/repr.rs
+++ b/src/librustc/mir/repr.rs
@@ -314,10 +314,19 @@ pub fn successors_mut(&mut self) -> &mut [BasicBlock] {
         }
     }
 
-    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 20a14cf415404feab75ba62affa757bd34727e40..9679654d958e9a24e202443a272f5a52e762a9f8 100644 (file)
--- a/src/librustc_mir/transform/erase_regions.rs
+++ b/src/librustc_mir/transform/erase_regions.rs
@@ -94,7 +94,7 @@ fn erase_regions_terminator(&mut self,
                 *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 aa0b3a25ebb0cabc8f3603255922c8c2bdd484f1..18a9aad0e915d1a885ea49c439ae64183f4b4e94 100644 (file)
--- a/src/librustc_trans/trans/mir/block.rs
+++ b/src/librustc_trans/trans/mir/block.rs
@@ -100,7 +100,7 @@ pub fn trans_block(&mut self, bb: mir::BasicBlock) {
                 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) {