From f5c7d7cd0df3df74ef5ec5c209afd83562e19f16 Mon Sep 17 00:00:00 2001
From: Stephen Chung <Stephen.Chung@intexact.com>
Date: Sun, 26 Apr 2020 19:37:32 +0800
Subject: [PATCH] Reduce memory footprint of Target.

---
 src/engine.rs |  9 +++++----
 src/parser.rs | 30 +++++++++++++++++-------------
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/engine.rs b/src/engine.rs
index a0d568ca..82f3e3d4 100644
--- a/src/engine.rs
+++ b/src/engine.rs
@@ -83,7 +83,7 @@ enum Target<'a> {
     /// The target is a variable stored in the current `Scope`.
     Scope(&'a RefCell<Dynamic>),
     /// The target is a temporary `Dynamic` value (i.e. the mutation can cause no side effects).
-    Value(Dynamic),
+    Value(Box<Dynamic>),
     /// The target is a character inside a String.
     StringChar(Box<(&'a mut Dynamic, usize, Dynamic)>),
 }
@@ -94,7 +94,7 @@ impl Target<'_> {
         match self {
             Target::Ref(r) => r.clone(),
             Target::Scope(r) => r.borrow().clone(),
-            Target::Value(v) => v,
+            Target::Value(v) => *v,
             Target::StringChar(s) => s.2,
         }
     }
@@ -144,7 +144,7 @@ impl<'a> From<&'a mut Dynamic> for Target<'a> {
 }
 impl<T: Into<Dynamic>> From<T> for Target<'_> {
     fn from(value: T) -> Self {
-        Self::Value(value.into())
+        Self::Value(Box::new(value.into()))
     }
 }
 
@@ -724,7 +724,7 @@ impl Engine {
         let obj = match target {
             Target::Scope(_) => scope_base.as_mut().unwrap().deref_mut(),
             Target::Ref(r) => r,
-            Target::Value(ref mut r) => r,
+            Target::Value(ref mut r) => r.as_mut(),
             Target::StringChar(ref mut x) => &mut x.2,
         };
 
@@ -777,6 +777,7 @@ impl Engine {
                         .collect::<Vec<_>>();
                     let def_val = def_val.as_ref();
                     // A function call is assumed to have side effects, so the value is changed
+                    // TODO - Remove assumption of side effects by checking whether the first parameter is &mut
                     self.exec_fn_call(fn_lib, fn_name, &mut args, def_val, *pos, 0).map(|v| (v, true))
                 }
                 // {xxx:map}.id
diff --git a/src/parser.rs b/src/parser.rs
index a9f79250..36cf4fd0 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -505,6 +505,7 @@ impl Expr {
         }
     }
 
+    /// Convert a `Variable` into a `Property`.  All other variants are untouched.
     pub(crate) fn into_property(self) -> Self {
         match self {
             Self::Variable(id, pos) => Self::Property(id, pos),
@@ -626,9 +627,10 @@ fn parse_call_expr<'a, S: Into<Cow<'static, str>> + Display>(
     }
 }
 
-/// Parse an indexing expression.
-fn parse_index_expr<'a>(
-    lhs: Box<Expr>,
+/// Parse an indexing chain.
+/// Indexing binds to the right, so this call parses all possible levels of indexing following in the input.
+fn parse_index_chain<'a>(
+    lhs: Expr,
     input: &mut Peekable<TokenIterator<'a>>,
     pos: Position,
     allow_stmt_expr: bool,
@@ -645,7 +647,7 @@ fn parse_index_expr<'a>(
             ))
             .into_err(*pos))
         }
-        Expr::IntegerConstant(_, pos) => match *lhs {
+        Expr::IntegerConstant(_, pos) => match lhs {
             Expr::Array(_, _) | Expr::StringConstant(_, _) => (),
 
             Expr::Map(_, _) => {
@@ -674,7 +676,7 @@ fn parse_index_expr<'a>(
         },
 
         // lhs[string]
-        Expr::StringConstant(_, pos) => match *lhs {
+        Expr::StringConstant(_, pos) => match lhs {
             Expr::Map(_, _) => (),
 
             Expr::Array(_, _) | Expr::StringConstant(_, _) => {
@@ -742,15 +744,18 @@ fn parse_index_expr<'a>(
         (Token::RightBracket, _) => {
             eat_token(input, Token::RightBracket);
 
-            let idx_expr = Box::new(idx_expr);
-
+            // Any more indexing following?
             match input.peek().unwrap() {
+                // If another indexing level, right-bind it
                 (Token::LeftBracket, _) => {
                     let follow_pos = eat_token(input, Token::LeftBracket);
-                    let follow = parse_index_expr(idx_expr, input, follow_pos, allow_stmt_expr)?;
-                    Ok(Expr::Index(lhs, Box::new(follow), pos))
+                    // Recursively parse the indexing chain, right-binding each
+                    let follow = parse_index_chain(idx_expr, input, follow_pos, allow_stmt_expr)?;
+                    // Indexing binds to right
+                    Ok(Expr::Index(Box::new(lhs), Box::new(follow), pos))
                 }
-                _ => Ok(Expr::Index(lhs, idx_expr, pos)),
+                // Otherwise terminate the indexing chain
+                _ => Ok(Expr::Index(Box::new(lhs), Box::new(idx_expr), pos)),
             }
         }
         (Token::LexError(err), pos) => return Err(PERR::BadInput(err.to_string()).into_err(*pos)),
@@ -943,9 +948,7 @@ fn parse_primary<'a>(
                 parse_call_expr(id, input, pos, allow_stmt_expr)?
             }
             // Indexing
-            (expr, Token::LeftBracket) => {
-                parse_index_expr(Box::new(expr), input, pos, allow_stmt_expr)?
-            }
+            (expr, Token::LeftBracket) => parse_index_chain(expr, input, pos, allow_stmt_expr)?,
             // Unknown postfix operator
             (expr, token) => panic!("unknown postfix operator {:?} for {:?}", token, expr),
         }
@@ -1068,6 +1071,7 @@ fn parse_op_assignment_stmt<'a>(
 fn make_dot_expr(lhs: Expr, rhs: Expr, op_pos: Position, is_index: bool) -> Expr {
     match (lhs, rhs) {
         // idx_lhs[idx_rhs].rhs
+        // Attach dot chain to the bottom level of indexing chain
         (Expr::Index(idx_lhs, idx_rhs, idx_pos), rhs) => Expr::Index(
             idx_lhs,
             Box::new(make_dot_expr(*idx_rhs, rhs, op_pos, true)),