From cdc37bb142ca2322e6f3ffc25fbc8a0273e09a37 Mon Sep 17 00:00:00 2001
From: JT <jonathan.d.turner@gmail.com>
Date: Fri, 30 Jul 2021 20:06:48 +1200
Subject: [PATCH] fix eval bug

---
 src/eval.rs    | 60 ++++++++++++++++++++++++++++++--------------------
 src/flatten.rs | 11 ---------
 src/main.rs    | 12 ++++------
 3 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/src/eval.rs b/src/eval.rs
index 7ce02c33f..6e5e0a7cc 100644
--- a/src/eval.rs
+++ b/src/eval.rs
@@ -77,11 +77,24 @@ pub struct StackFrame {
     pub parent: Option<Stack>,
 }
 
-pub type Stack = Rc<RefCell<StackFrame>>;
+#[derive(Clone)]
+pub struct Stack(Rc<RefCell<StackFrame>>);
 
-impl StackFrame {
-    pub fn get_var(this: Stack, var_id: VarId) -> Result<Value, ShellError> {
-        let this = this.borrow();
+impl Default for Stack {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl Stack {
+    pub fn new() -> Stack {
+        Stack(Rc::new(RefCell::new(StackFrame {
+            vars: HashMap::new(),
+            parent: None,
+        })))
+    }
+    pub fn get_var(&self, var_id: VarId) -> Result<Value, ShellError> {
+        let this = self.0.borrow();
         match this.vars.get(&var_id) {
             Some(v) => Ok(v.clone()),
             _ => {
@@ -91,25 +104,25 @@ impl StackFrame {
         }
     }
 
-    pub fn add_var(this: Stack, var_id: VarId, value: Value) {
-        let mut this = this.borrow_mut();
+    pub fn add_var(&self, var_id: VarId, value: Value) {
+        let mut this = self.0.borrow_mut();
         this.vars.insert(var_id, value);
     }
 
-    pub fn enter_scope(this: Stack) -> Stack {
-        Rc::new(RefCell::new(StackFrame {
+    pub fn enter_scope(self) -> Stack {
+        Stack(Rc::new(RefCell::new(StackFrame {
             vars: HashMap::new(),
-            parent: Some(this),
-        }))
+            parent: Some(self),
+        })))
     }
 
     pub fn print_stack(&self) {
         println!("===frame===");
-        for (var, val) in &self.vars {
+        for (var, val) in &self.0.borrow().vars {
             println!("{}: {:?}", var, val);
         }
-        if let Some(parent) = &self.parent {
-            parent.borrow().print_stack()
+        if let Some(parent) = &self.0.borrow().parent {
+            parent.print_stack()
         }
     }
 }
@@ -131,6 +144,7 @@ pub fn eval_operator(
 fn eval_call(state: &State, stack: Stack, call: &Call) -> Result<Value, ShellError> {
     let decl = state.parser_state.get_decl(call.decl_id);
     if let Some(block_id) = decl.body {
+        let stack = stack.enter_scope();
         for (arg, param) in call
             .positional
             .iter()
@@ -141,10 +155,9 @@ fn eval_call(state: &State, stack: Stack, call: &Call) -> Result<Value, ShellErr
                 .var_id
                 .expect("internal error: all custom parameters must have var_ids");
 
-            StackFrame::add_var(stack.clone(), var_id, result);
+            stack.add_var(var_id, result);
         }
         let block = state.parser_state.get_block(block_id);
-        let stack = StackFrame::enter_scope(stack);
         eval_block(state, stack, block)
     } else if decl.signature.name == "let" {
         let var_id = call.positional[0]
@@ -159,7 +172,7 @@ fn eval_call(state: &State, stack: Stack, call: &Call) -> Result<Value, ShellErr
 
         //println!("Adding: {:?} to {}", rhs, var_id);
 
-        StackFrame::add_var(stack, var_id, rhs);
+        stack.add_var(var_id, rhs);
         Ok(Value::Unknown)
     } else if decl.signature.name == "if" {
         let cond = &call.positional[0];
@@ -173,14 +186,13 @@ fn eval_call(state: &State, stack: Stack, call: &Call) -> Result<Value, ShellErr
             Value::Bool { val, .. } => {
                 if val {
                     let block = state.parser_state.get_block(then_block);
-                    let stack = StackFrame::enter_scope(stack);
+                    let stack = stack.enter_scope();
                     eval_block(state, stack, block)
                 } else if let Some(else_case) = else_case {
-                    println!("{:?}", else_case);
                     if let Some(else_expr) = else_case.as_keyword() {
                         if let Some(block_id) = else_expr.as_block() {
                             let block = state.parser_state.get_block(block_id);
-                            let stack = StackFrame::enter_scope(stack);
+                            let stack = stack.enter_scope();
                             eval_block(state, stack, block)
                         } else {
                             eval_expression(state, stack, else_expr)
@@ -212,7 +224,7 @@ fn eval_call(state: &State, stack: Stack, call: &Call) -> Result<Value, ShellErr
             .expect("internal error: expected block");
         let block = state.parser_state.get_block(block);
 
-        let stack = StackFrame::enter_scope(stack);
+        let stack = stack.enter_scope();
         let start_time = Instant::now();
         eval_block(state, stack, block)?;
         let end_time = Instant::now();
@@ -233,7 +245,7 @@ fn eval_call(state: &State, stack: Stack, call: &Call) -> Result<Value, ShellErr
             .expect("internal error: expected block");
         let block = state.parser_state.get_block(block);
 
-        let stack = StackFrame::enter_scope(stack);
+        let stack = stack.enter_scope();
 
         let mut x = Value::Int {
             val: 0,
@@ -244,7 +256,7 @@ fn eval_call(state: &State, stack: Stack, call: &Call) -> Result<Value, ShellErr
             if x == end_val {
                 break;
             } else {
-                StackFrame::add_var(stack.clone(), var_id, x.clone());
+                stack.add_var(var_id, x.clone());
                 eval_block(state, stack.clone(), block)?;
             }
             if let Value::Int { ref mut val, .. } = x {
@@ -271,7 +283,7 @@ pub fn eval_expression(
             val: *i,
             span: expr.span,
         }),
-        Expr::Var(var_id) => StackFrame::get_var(stack, *var_id),
+        Expr::Var(var_id) => stack.get_var(*var_id),
         Expr::Call(call) => eval_call(state, stack, call),
         Expr::ExternalCall(_, _) => Err(ShellError::Unsupported(expr.span)),
         Expr::Operator(_) => Ok(Value::Unknown),
@@ -289,7 +301,7 @@ pub fn eval_expression(
         Expr::Subexpression(block_id) => {
             let block = state.parser_state.get_block(*block_id);
 
-            let stack = StackFrame::enter_scope(stack);
+            let stack = stack.enter_scope();
             eval_block(state, stack, block)
         }
         Expr::Block(block_id) => Ok(Value::Block(*block_id)),
diff --git a/src/flatten.rs b/src/flatten.rs
index e06e54204..fcafd54b3 100644
--- a/src/flatten.rs
+++ b/src/flatten.rs
@@ -43,20 +43,9 @@ impl<'a> ParserWorkingSet<'a> {
             Expr::Block(block_id) => self.flatten_block(self.get_block(*block_id)),
             Expr::Call(call) => {
                 let mut output = vec![(call.head, FlatShape::InternalCall)];
-                let mut last_span = call.head.end;
                 for positional in &call.positional {
-                    last_span = positional.span.end;
                     output.extend(self.flatten_expression(positional));
                 }
-                if last_span < expr.span.end {
-                    output.push((
-                        Span {
-                            start: last_span,
-                            end: expr.span.end,
-                        },
-                        FlatShape::InternalCall,
-                    ));
-                }
                 output
             }
             Expr::ExternalCall(..) => {
diff --git a/src/main.rs b/src/main.rs
index 53dcdc0ed..11ba59ecd 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,8 +1,7 @@
-use std::{cell::RefCell, collections::HashMap, rc::Rc};
+use std::{cell::RefCell, rc::Rc};
 
 use engine_q::{
-    eval_block, NuHighlighter, ParserState, ParserWorkingSet, Signature, StackFrame, State,
-    SyntaxShape,
+    eval_block, NuHighlighter, ParserState, ParserWorkingSet, Signature, Stack, State, SyntaxShape,
 };
 
 fn main() -> std::io::Result<()> {
@@ -152,10 +151,7 @@ fn main() -> std::io::Result<()> {
 
         let prompt = DefaultPrompt::new(1);
         let mut current_line = 1;
-        let stack = Rc::new(RefCell::new(StackFrame {
-            vars: HashMap::new(),
-            parent: None,
-        }));
+        let stack = Stack::new();
 
         loop {
             let input = line_editor.read_line(&prompt)?;
@@ -173,7 +169,7 @@ fn main() -> std::io::Result<()> {
                         parser_state.borrow().print_blocks();
                         continue;
                     } else if s.trim() == "stack" {
-                        stack.borrow().print_stack();
+                        stack.print_stack();
                     }
                     // println!("input: '{}'", s);