From 38a3414e8831536915aefd34355b69078491833c Mon Sep 17 00:00:00 2001
From: Alex Bethel <abethel@cs.nmt.edu>
Date: Sat, 27 Nov 2021 11:02:41 -0600
Subject: [PATCH] Slightly better ExecEnv::assign

A little more idiomatic, avoid some borrow checker clumsiness, add
comments.
---
 ablescript/src/interpret.rs | 37 ++++++++++++++++++++++---------------
 ablescript/src/variables.rs |  6 ++++++
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/ablescript/src/interpret.rs b/ablescript/src/interpret.rs
index e1c3af51..5c9cae6c 100644
--- a/ablescript/src/interpret.rs
+++ b/ablescript/src/interpret.rs
@@ -11,7 +11,7 @@ use std::{
     cell::RefCell,
     collections::{HashMap, VecDeque},
     io::{stdin, stdout, Read, Write},
-    mem::swap,
+    mem::take,
     ops::Range,
     process::exit,
     rc::Rc,
@@ -292,28 +292,35 @@ impl ExecEnv {
                 for index in indices {
                     let index = self.eval_expr(index)?;
 
-                    let next_cell;
-                    match &mut *cell.borrow_mut() {
+                    let next_cell = match &mut *cell.borrow_mut() {
                         Value::Cart(c) => {
+                            // cell is a cart, so we can do simple
+                            // indexing.
                             if let Some(x) = c.get(&index) {
-                                next_cell = Rc::clone(x);
+                                // cell[index] exists, get a shared
+                                // reference to it.
+                                Rc::clone(x)
                             } else {
-                                next_cell = Rc::new(RefCell::new(Value::Cart(Default::default())));
+                                // cell[index] does not exist, so we
+                                // insert an empty cart by default
+                                // instead.
+                                let next_cell =
+                                    Rc::new(RefCell::new(Value::Cart(Default::default())));
                                 c.insert(index, Rc::clone(&next_cell));
+                                next_cell
                             }
                         }
                         x => {
-                            // Annoying borrow checker dance
-                            // to move *x.
-                            let mut tmp = Value::Nul;
-                            swap(&mut tmp, x);
-
-                            let mut c = tmp.into_cart();
-                            next_cell = Rc::new(RefCell::new(Value::Cart(Default::default())));
-                            c.insert(index, Rc::clone(&next_cell));
-                            *x = Value::Cart(c);
+                            // cell is not a cart; `take` it, convert
+                            // it into a cart, and write the result
+                            // back into it.
+                            let mut cart = take(x).into_cart();
+                            let next_cell = Rc::new(RefCell::new(Value::Cart(Default::default())));
+                            cart.insert(index, Rc::clone(&next_cell));
+                            *x = Value::Cart(cart);
+                            next_cell
                         }
-                    }
+                    };
                     cell = next_cell;
                 }
                 cell.replace(value);
diff --git a/ablescript/src/variables.rs b/ablescript/src/variables.rs
index fb663a51..99f79557 100644
--- a/ablescript/src/variables.rs
+++ b/ablescript/src/variables.rs
@@ -60,6 +60,12 @@ pub enum Value {
     Cart(Cart),
 }
 
+impl Default for Value {
+    fn default() -> Self {
+        Self::Nul
+    }
+}
+
 impl Hash for Value {
     fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
         discriminant(self).hash(state);