fix: closure circular reference fix

src/bytecode/src/environment.rs

@@ -1,4 +1,4 @@
-use std::{cell::RefCell, collections::HashMap, fmt::Debug, rc::Rc};
+use std::{cell::RefCell, collections::HashMap, fmt::Debug, rc::Rc, rc::Weak};
 
 use serde::{Deserialize, Deserializer, Serialize};
 
@@ -51,7 +51,7 @@ impl Environment {
 /// Environment should NOT be serialized. It is only used for runtime state.
 /// This trait is pseudo-implemented so that we can add it to the operant stack.
 /// Note we cannot implement Serialize for Rc<RefCell<Environment>> because it is not defined in this crate.
-impl Serialize for W<Rc<RefCell<Environment>>> {
+impl Serialize for W<Weak<RefCell<Environment>>> {
     fn serialize<S: serde::Serializer>(&self, _serializer: S) -> Result<S::Ok, S::Error> {
         panic!("Environment should not be serialized");
     }
@@ -60,30 +60,37 @@ impl Serialize for W<Rc<RefCell<Environment>>> {
 /// Environment should NOT be deserialized. It is only used for runtime state.
 /// This trait is pseudo-implemented so that we can add it to the operant stack.
 /// Note we cannot implement Deserialize for Rc<RefCell<Environment>> because it is not defined in this crate.
-impl<'de> Deserialize<'de> for W<Rc<RefCell<Environment>>> {
+impl<'de> Deserialize<'de> for W<Weak<RefCell<Environment>>> {
     fn deserialize<D: Deserializer<'de>>(_deserializer: D) -> Result<Self, D::Error> {
         panic!("Environment should not be deserialized");
     }
 }
 
 /// Implement Clone trait to satisfy the requirements of Value enum.
-impl Clone for W<Rc<RefCell<Environment>>> {
+impl Clone for W<Weak<RefCell<Environment>>> {
     fn clone(&self) -> Self {
         W(self.0.clone())
     }
 }
 
 /// Implement PartialEq trait to satisfy the requirements of Value enum.
-impl PartialEq for W<Rc<RefCell<Environment>>> {
+impl PartialEq for W<Weak<RefCell<Environment>>> {
     fn eq(&self, other: &Self) -> bool {
-        self.0 == other.0
+        match (self.0.upgrade(), other.0.upgrade()) {
+            (Some(self_rc), Some(other_rc)) => Rc::ptr_eq(&self_rc, &other_rc),
+            (None, None) => true, // Both point to dropped values, considered equal
+            _ => false,           // One is dropped and the other isn't, not equal
+        }
     }
 }
 
 /// Implement Debug trait to satisfy the requirements of Value enum.
-impl Debug for W<Rc<RefCell<Environment>>> {
+impl Debug for W<Weak<RefCell<Environment>>> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.0.borrow().fmt(f)
+        match self.0.upgrade() {
+            Some(env) => env.borrow().fmt(f),
+            None => write!(f, "<Environment dropped>"),
+        }
     }
 }
 

src/bytecode/src/value.rs

@@ -1,4 +1,4 @@
-use std::{cell::RefCell, rc::Rc};
+use std::{cell::RefCell, rc::Weak};
 
 use serde::{Deserialize, Serialize};
 
@@ -17,7 +17,7 @@ pub enum Value {
         sym: Symbol,
         prms: Vec<Symbol>,
         addr: usize,
-        env: W<Rc<RefCell<Environment>>>,
+        env: W<Weak<RefCell<Environment>>>,
     },
 }
 

vm/ignite/src/error.rs

@@ -20,6 +20,9 @@ pub enum VmError {
     #[error("runtime stack underflow")]
     RuntimeStackUnderflow,
 
+    #[error("environment dropped")]
+    EnvironmentDropped,
+
     #[error("pc out of bounds: {0}")]
     PcOutOfBounds(usize),
 

vm/ignite/src/micro_code/call.rs

@@ -50,6 +50,11 @@ pub fn call(rt: &mut Runtime, arity: usize) -> Result<()> {
         .into());
     };
 
+    let env = match env.0.upgrade() {
+        Some(env) => env,
+        None => return Err(VmError::EnvironmentDropped.into()),
+    };
+
     if prms.len() != arity {
         return Err(VmError::ArityParamsMismatch {
             arity: prms.len(),
@@ -60,7 +65,7 @@ pub fn call(rt: &mut Runtime, arity: usize) -> Result<()> {
 
     let frame = StackFrame {
         frame_type: FrameType::CallFrame,
-        env: Rc::clone(&env.0),
+        env: Rc::clone(&env),
         address: Some(rt.pc),
     };
 
@@ -80,14 +85,15 @@ mod tests {
     fn test_call() {
         let mut rt = Runtime::new(vec![ByteCode::CALL(0), ByteCode::DONE]);
         let result = call(&mut rt, 0);
+        let env = Environment::new_wrapped();
 
         assert!(result.is_err());
 
         rt.operand_stack.push(Value::Closure {
             sym: "Closure".to_string(),
             prms: vec![],
             addr: 123,
-            env: W(Environment::new_wrapped()),
+            env: W(Rc::downgrade(&env)),
         });
 
         let result = call(&mut rt, 0);

vm/ignite/src/micro_code/ldf.rs

@@ -19,11 +19,12 @@ use crate::Runtime;
 ///
 /// Infallible.
 pub fn ldf(rt: &mut Runtime, addr: usize, prms: Vec<Symbol>) -> Result<()> {
+    let env = Rc::downgrade(&Rc::clone(&rt.env));
     let closure = Value::Closure {
         sym: "Closure".to_string(),
         prms,
         addr,
-        env: W(Rc::clone(&rt.env)),
+        env: W(env),
     };
 
     rt.operand_stack.push(closure);
@@ -40,13 +41,16 @@ mod tests {
         ldf(&mut rt, 0, vec!["x".to_string()]).unwrap();
 
         let closure = rt.operand_stack.pop().unwrap();
+        let env1 = Rc::downgrade(&Rc::clone(&rt.env));
+        let env2 = Rc::downgrade(&Rc::clone(&rt.env));
+
         assert_eq!(
             &closure,
             &Value::Closure {
                 sym: "Closure".to_string(),
                 prms: vec!["x".to_string()],
                 addr: 0,
-                env: W(Rc::clone(&rt.env)),
+                env: W(env1),
             }
         );
 
@@ -56,7 +60,7 @@ mod tests {
                 sym: "Closure".to_string(),
                 prms: vec!["y".to_string()],
                 addr: 0,
-                env: W(Rc::clone(&rt.env)),
+                env: W(env2),
             }
         )
     }