Recursion finally works

Switched from Box to Rc (Yeah, I peeked at solution but I understood Rc
and RefCell just now) to allow multiple references to env data

Signed-off-by: teo3300 <matteo.rogora@live.it>

src/env.rs

@@ -1,5 +1,7 @@
 use crate::types::MalType::*;
 use crate::types::{MalMap, MalRet, MalType};
+use std::cell::RefCell;
+use std::rc::Rc;
 
 // This is the first time I implement a macro, and I'm copying it
 // so I will comment this a LOT
@@ -9,7 +11,7 @@ macro_rules! env_init {
         {
             // the macro prevent the macro from disrupting the external code
             // this is the block of code that will substitute the macro
-            Env::new($outer)
+            env_new($outer)
             // returns an empty map
         }
     };
@@ -20,9 +22,9 @@ macro_rules! env_init {
         // recognizable structure
         {
             // create an empty map
-            let mut map = env_init!($outer);
+            let map = env_init!($outer);
             $( // Do this for all elements of the arguments list
-                map.set($key, &$val);
+                env_set(&map, $key, &$val);
             )*
             // return the new map
             map
@@ -31,36 +33,37 @@ macro_rules! env_init {
 }
 
 #[derive(Debug, Clone)]
-pub struct Env {
-    data: MalMap,
-    outer: Option<Box<Env>>,
+pub struct EnvType {
+    data: RefCell<MalMap>,
+    outer: Option<Env>,
 }
 
-impl Env {
-    pub fn new(outer: Option<Box<Env>>) -> Self {
-        Env {
-            data: MalMap::new(),
-            outer,
-        }
-    }
+pub type Env = Rc<EnvType>;
+// Following rust implementation, using shorthand to always pas Reference count
 
-    pub fn set(&mut self, sym: &str, val: &MalType) {
-        self.data.insert(sym.to_string(), val.clone());
-    }
+pub fn env_new(outer: Option<Env>) -> Env {
+    Rc::new(EnvType {
+        data: RefCell::new(MalMap::new()),
+        outer,
+    })
+}
 
-    pub fn get(&self, sym: &String) -> MalRet {
-        match self.data.get(sym) {
-            Some(val) => Ok(val.clone()),
-            None => match &self.outer {
-                Some(outer) => outer.get(sym),
-                None => Err(format!("symbol {:?} not defined", sym)),
-            },
-        }
+pub fn env_set(env: &Env, sym: &str, val: &MalType) {
+    env.data.borrow_mut().insert(sym.to_string(), val.clone());
+}
+
+pub fn env_get(env: &Env, sym: &String) -> MalRet {
+    match env.data.borrow().get(sym) {
+        Some(val) => Ok(val.clone()),
+        None => match env.outer.clone() {
+            Some(outer) => env_get(&outer, sym),
+            None => Err(format!("symbol {:?} not defined", sym)),
+        },
     }
 }
 
-pub fn env_binds(outer: &Env, binds: &MalType, exprs: &[MalType]) -> Result<Env, String> {
-    let mut env = Env::new(Some(Box::new(outer.clone())));
+pub fn env_binds(outer: Env, binds: &MalType, exprs: &[MalType]) -> Result<Env, String> {
+    let env = env_new(Some(outer));
     match binds {
         List(binds) => {
             if binds.len() != exprs.len() {
@@ -68,7 +71,7 @@ pub fn env_binds(outer: &Env, binds: &MalType, exprs: &[MalType]) -> Result<Env,
             } // TODO: May be possible to leave this be and not set additional elements at all
             for (bind, expr) in binds.iter().zip(exprs.iter()) {
                 match bind {
-                    Sym(sym) => env.set(sym, expr),
+                    Sym(sym) => env_set(&env, sym, expr),
                     _ => {
                         return Err(format!(
                             "Initializing environment: {:?} is not a symbol",

src/eval.rs

@@ -1,5 +1,5 @@
-use crate::env::env_binds;
 use crate::env::Env;
+use crate::env::{env_binds, env_get, env_new, env_set};
 use crate::types::car_cdr;
 use crate::types::MalType::*;
 use crate::types::{MalArgs, MalMap, MalRet, MalType};
@@ -13,8 +13,10 @@ fn call_func(func: &MalType, args: &[MalType]) -> MalRet {
             ast,
             env,
         } => {
-            let mut inner_env = env_binds(env, params, args)?;
-            match eval(ast, &mut inner_env)? {
+            let inner_env = env_binds(env.clone(), params, args)?;
+            // It's fine to clone the environment here
+            // since this is when the function is actually called
+            match eval(ast, inner_env)? {
                 List(list) => Ok(list.last().unwrap_or(&Nil).clone()),
                 _ => Err("This should not happen".to_string()),
             }
@@ -35,14 +37,21 @@ fn eval_func(list: &MalType) -> MalRet {
     }
 }
 
+// When evaluating an expression it's possible
+// (actually the only option I'm aware until now)
+// to clone the environment "env.clone()", performances aside
+//
+// It's not possible, however, to clone the outer when defining
+// a new environment that will be used later (such as when using fn*)
+
 /// def! special form:
 ///     Evaluate the second expression and assign it to the first symbol
-fn def_bang_form(list: &[MalType], env: &mut Env) -> MalRet {
+fn def_bang_form(list: &[MalType], env: &Env) -> MalRet {
     match list.len() {
         2 => match &list[0] {
             Sym(sym) => {
-                let cdr = eval(&list[1], env)?;
-                env.set(sym.as_str(), &cdr);
+                let cdr = eval(&list[1], env.clone())?;
+                env_set(&env, sym.as_str(), &cdr);
                 Ok(cdr)
             }
             _ => Err(format!(
@@ -57,22 +66,22 @@ fn def_bang_form(list: &[MalType], env: &mut Env) -> MalRet {
 /// let* special form:
 ///     Create a temporary inner environment, assigning pair of elements in
 ///     the first list and returning the evaluation of the second expression
-fn let_star_form(list: &[MalType], env: &Env) -> MalRet {
+fn let_star_form(list: &[MalType], env: Env) -> MalRet {
     // Create the inner environment
-    let mut inner_env = Env::new(Some(Box::new(env.clone())));
+    let inner_env = env_new(Some(env.clone()));
     // change the inner environment
     let (car, cdr) = car_cdr(list);
     match car {
         List(list) if list.len() % 2 == 0 => {
             // TODO: Find a way to avoid index looping that is ugly
             for i in (0..list.len()).step_by(2) {
-                def_bang_form(&list[i..i + 2], &mut inner_env)?;
+                def_bang_form(&list[i..i + 2], &inner_env)?;
             }
             if cdr.is_empty() {
                 // TODO: check if it exists a better way to do this
                 Ok(Nil)
             } else {
-                eval(&cdr[0], &mut inner_env)
+                eval(&cdr[0], inner_env)
             }
         }
         _ => Err("First argument of let* must be a list of pair definitions".to_string()),
@@ -82,7 +91,7 @@ fn let_star_form(list: &[MalType], env: &Env) -> MalRet {
 /// do special form:
 ///     Evaluate all the elements in a list using eval_ast and return the
 ///     result of the last evaluation
-fn do_form(list: &[MalType], env: &mut Env) -> MalRet {
+fn do_form(list: &[MalType], env: Env) -> MalRet {
     if list.is_empty() {
         return Err("do form: provide a list as argument".to_string());
     }
@@ -92,12 +101,12 @@ fn do_form(list: &[MalType], env: &mut Env) -> MalRet {
     }
 }
 
-fn if_form(list: &[MalType], env: &mut Env) -> MalRet {
+fn if_form(list: &[MalType], env: Env) -> MalRet {
     if !(2..=3).contains(&list.len()) {
         return Err("if form: number of arguments".to_string());
     }
     let (cond, branches) = car_cdr(list);
-    match eval(cond, env)? {
+    match eval(cond, env.clone())? {
         Nil | Bool(false) => match branches.len() {
             1 => Ok(Nil),
             _ => eval(&branches[1], env),
@@ -106,7 +115,7 @@ fn if_form(list: &[MalType], env: &mut Env) -> MalRet {
     }
 }
 
-fn fn_star_form(list: &[MalType], env: &Env) -> MalRet {
+fn fn_star_form(list: &[MalType], env: Env) -> MalRet {
     if list.is_empty() {
         return Err("fn* form: specify lambda arguments".to_string());
     }
@@ -115,16 +124,16 @@ fn fn_star_form(list: &[MalType], env: &Env) -> MalRet {
         eval: eval_ast,
         params: Box::new(binds.clone()),
         ast: Box::new(List(exprs.to_vec())),
-        env: env.clone(),
+        env: env,
     })
 }
 
 /// Intermediate function to discern special forms from defined symbols
-fn apply(list: &MalArgs, env: &mut Env) -> MalRet {
+fn apply(list: &MalArgs, env: Env) -> MalRet {
     let (car, cdr) = car_cdr(list);
     match car {
-        Sym(sym) if sym == "def!" => def_bang_form(cdr, env),
-        Sym(sym) if sym == "let*" => let_star_form(cdr, env),
+        Sym(sym) if sym == "def!" => def_bang_form(cdr, &env), // Set for env
+        Sym(sym) if sym == "let*" => let_star_form(cdr, env),  // Clone the env
         Sym(sym) if sym == "do" => do_form(cdr, env),
         Sym(sym) if sym == "if" => if_form(cdr, env),
         Sym(sym) if sym == "fn*" => fn_star_form(cdr, env),
@@ -136,7 +145,7 @@ fn apply(list: &MalArgs, env: &mut Env) -> MalRet {
 /// Switch ast evaluation depending on it being a list or not and return the
 /// result of the evaluation, this function calls "eval_ast" to recursively
 /// evaluate asts
-pub fn eval(ast: &MalType, env: &mut Env) -> MalRet {
+pub fn eval(ast: &MalType, env: Env) -> MalRet {
     match &ast {
         List(list) if list.is_empty() => Ok(ast.clone()),
         List(list) if !list.is_empty() => apply(list, env),
@@ -144,28 +153,28 @@ pub fn eval(ast: &MalType, env: &mut Env) -> MalRet {
     }
 }
 
-/// Separetely evaluate all elements in a collection (list or vector)
-fn eval_collection(list: &MalArgs, env: &mut Env) -> Result<MalArgs, String> {
+/// Separately evaluate all elements in a collection (list or vector)
+fn eval_collection(list: &MalArgs, env: Env) -> Result<MalArgs, String> {
     let mut ret = MalArgs::new();
     for el in list {
-        ret.push(eval(el, env)?);
+        ret.push(eval(el, env.clone())?);
     }
     Ok(ret)
 }
 
 /// Evaluate the values of a map
-fn eval_map(map: &MalMap, env: &mut Env) -> MalRet {
+fn eval_map(map: &MalMap, env: Env) -> MalRet {
     let mut ret = MalMap::new();
     for (k, v) in map {
-        ret.insert(k.to_string(), eval(v, env)?);
+        ret.insert(k.to_string(), eval(v, env.clone())?);
     }
     Ok(Map(ret))
 }
 
 /// Eval the provided ast
-fn eval_ast(ast: &MalType, env: &mut Env) -> MalRet {
+fn eval_ast(ast: &MalType, env: Env) -> MalRet {
     match ast {
-        Sym(sym) => env.get(sym),
+        Sym(sym) => env_get(&env, sym),
         List(list) => Ok(List(eval_collection(list, env)?)),
         Vector(vec) => Ok(Vector(eval_collection(vec, env)?)),
         Map(map) => eval_map(map, env),

src/main.rs

@@ -8,12 +8,12 @@ mod reader;
 mod types;
 use env::env_init;
 
-mod step3_env;
-use step3_env::rep;
+mod step4_if_fn_do;
+use step4_if_fn_do::rep;
 
 fn main() {
     let mut num = 0;
-    let mut reply_env = env_init();
+    let reply_env = env_init();
 
     loop {
         let mut input = String::new();
@@ -30,7 +30,7 @@ fn main() {
 
             if input != "\n" {
                 // Perform rep on whole available input
-                match rep(&input, &mut reply_env) {
+                match rep(&input, &reply_env) {
                     Ok(output) => println!("[{}]> {}", num, output),
                     Err(err) => {
                         if line != "\n" {

src/step4_if_fn_do.rs

@@ -18,7 +18,7 @@ fn READ(input: &str) -> MalRet {
 
 #[allow(non_snake_case)]
 /// Evaluate the generated ast
-fn EVAL(ast: MalType, env: &mut Env) -> MalRet {
+fn EVAL(ast: MalType, env: Env) -> MalRet {
     eval(&ast, env).map_err(|err| format!("@ EVAL: {}", err))
 }
 
@@ -28,8 +28,8 @@ fn PRINT(output: MalType) -> String {
     pr_str(&output, true)
 }
 
-pub fn rep(input: &str, env: &mut Env) -> Result<String, String> {
+pub fn rep(input: &str, env: &Env) -> Result<String, String> {
     let ast = READ(input)?;
-    let out = EVAL(ast, env)?;
+    let out = EVAL(ast, env.clone())?;
     Ok(PRINT(out))
 }

src/types.rs

@@ -9,7 +9,7 @@ pub enum MalType {
     Map(MalMap),
     Fun(fn(&[MalType]) -> MalRet), // Used for base functions, implemented using the underlying language (rust)
     MalFun {
-        eval: fn(ast: &MalType, env: &mut Env) -> MalRet,
+        eval: fn(ast: &MalType, env: Env) -> MalRet,
         params: Box<MalType>,
         ast: Box<MalType>,
         env: Env,