fix resolving of vars

lox/fibonacci.lox

@@ -1,7 +1,6 @@
 var a = 0;
 var temp;
 
-var b = 1;
 for(var b = 1; a < 1000; b = temp + b) {
     print a;
     temp = a;

src/environment.rs

@@ -55,15 +55,24 @@ impl Environment {
         }
     }
 
-    /// Assign a value to a variable at a specific lexical distance (number of scopes away).
+    /// Assign a value to a variable in an environment at a specific lexical distance.
-    /// Used by the resolver to handle closures correctly.
     pub fn assign_at(
-        &mut self,
+        env: Rc<RefCell<Environment>>,
         distance: usize,
         token: &Token,
         value: Value,
     ) -> Result<(), EnvironmentError> {
-        self.ancestor(distance)?
+        let mut current_env = env;
+        for _ in 0..distance {
+            let enclosing = current_env
+                .borrow()
+                .enclosing
+                .clone()
+                .ok_or(EnvironmentError::InvalidDistance)?;
+            current_env = enclosing;
+        }
+        current_env
+            .borrow_mut()
             .values
             .insert(token.lexeme.clone(), value);
         Ok(())
@@ -85,9 +94,22 @@ impl Environment {
     }
 
     /// Get a variable's value from an environment at a specific lexical distance.
-    /// Used by the resolver to handle closures correctly.
+    pub fn get_at(
-    pub fn get_at(&self, distance: usize, token: &Token) -> Result<Value, EnvironmentError> {
+        env: Rc<RefCell<Environment>>,
-        if let Some(v) = self.ancestor(distance)?.values.get(token.lexeme.as_str()) {
+        distance: usize,
+        token: &Token,
+    ) -> Result<Value, EnvironmentError> {
+        let mut current_env_rc = env;
+        for _ in 0..distance {
+            let enclosing = current_env_rc
+                .borrow()
+                .enclosing
+                .clone()
+                .ok_or(EnvironmentError::InvalidDistance)?;
+            current_env_rc = enclosing;
+        }
+        let borrowed_env = current_env_rc.borrow();
+        if let Some(v) = borrowed_env.values.get(token.lexeme.as_str()) {
             Ok(v.clone())
         } else {
             Err(EnvironmentError::UndefinedVariable(
@@ -96,17 +118,4 @@ impl Environment {
             ))
         }
     }
-
-    /// Find an environment at a specific lexical distance from the current one.
-    fn ancestor(&self, distance: usize) -> Result<Environment, EnvironmentError> {
-        // println!("{distance}: {self:?}");
-        if distance == 0 {
-            return Ok(self.clone());
-        }
-
-        match &self.enclosing {
-            Some(enclosing) => enclosing.borrow().ancestor(distance - 1),
-            None => Err(EnvironmentError::InvalidDistance),
-        }
-    }
 }

src/interpreter.rs

@@ -242,7 +242,7 @@ impl Interpreter {
                 right,
             } => self.binary(left, operator, right),
             Expression::Variable { name } => self.var_expression(name, expression),
-            Expression::Assign { name, value } => self.assign(name, value),
+            Expression::Assign { .. } => self.assign(expression),
             Expression::Logical {
                 left,
                 operator,
@@ -323,8 +323,9 @@ impl Interpreter {
         method: &Token,
     ) -> Result<Value, InterpreterError> {
         if let Some(distance) = self.locals.get(expression) {
-            let superclass = self.environment.borrow().get_at(*distance, keyword)?;
+            let superclass = Environment::get_at(self.environment.clone(), *distance, keyword)?;
-            let object = self.environment.borrow().get_at(
+            let object = Environment::get_at(
+                self.environment.clone(),
                 *distance - 1,
                 &Token {
                     token_type: TokenType::This,
@@ -449,22 +450,27 @@ impl Interpreter {
     }
 
     /// Assign the value of an expression to a variable.
-    fn assign(&mut self, name: &Token, expression: &Expression) -> Result<Value, InterpreterError> {
+    fn assign(&mut self, assign_expr: &Expression) -> Result<Value, InterpreterError> {
-        let value = self.evaluate(expression)?;
+        if let Expression::Assign { name, value } = assign_expr {
+            let evaluated_value = self.evaluate(value)?;
 
-        if let Some(distance) = self.locals.get(expression) {
+            if let Some(distance) = self.locals.get(assign_expr) {
-            self.environment
+                Environment::assign_at(
-                .borrow_mut()
+                    self.environment.clone(),
-                .assign_at(*distance, name, value.clone())
+                    *distance,
-                .map_err(InterpreterError::UndefinedVariable)?;
+                    name,
+                    evaluated_value.clone(),
+                )?;
+            } else {
+                self.globals
+                    .borrow_mut()
+                    .assign(name, evaluated_value.clone())?;
+            }
+            Ok(evaluated_value)
         } else {
-            self.globals
+            // This should ideally not be reached if called correctly from `evaluate`
-                .borrow_mut()
+            unreachable!("Interpreter::assign called with a non-Assign expression");
-                .assign(name, value.clone())
-                .map_err(InterpreterError::UndefinedVariable)?;
         }
-
-        Ok(value)
     }
 
     /// Convert the literal value into a Value.
@@ -537,15 +543,13 @@ impl Interpreter {
         expression: &Expression,
     ) -> Result<Value, InterpreterError> {
         if let Some(distance) = self.locals.get(expression) {
-            self.environment
+            Ok(Environment::get_at(
-                .borrow()
+                self.environment.clone(),
-                .get_at(*distance, name)
+                *distance,
-                .map_err(InterpreterError::UndefinedVariable)
+                name,
+            )?)
         } else {
-            self.globals
+            Ok(self.globals.borrow().get(name)?)
-                .borrow()
-                .get(name)
-                .map_err(InterpreterError::UndefinedVariable)
         }
     }
 

src/lib.rs

@@ -94,9 +94,6 @@ fn run_rox(input: &str, interpreter: &mut Interpreter) -> Result<(), RoxError> {
     debug!("AST:\n{}", crate::ast_printer::print(&ast));
 
     resolver.resolve(&ast)?;
-    println!("globals: {:?}", interpreter.globals);
-    println!("environment: {:?}", interpreter.environment);
-    println!("locals: {:?}", interpreter.locals);
     interpreter.run(ast)?;
 
     Ok(())

src/native_functions.rs

@@ -159,7 +159,11 @@ impl Callable for AsciiOut {
         1
     }
 
-    fn call(&self, interpreter: &mut Interpreter, args: Vec<Value>) -> Result<Value, CallingError> {
+    fn call(
+        &self,
+        _interpreter: &mut Interpreter,
+        args: Vec<Value>,
+    ) -> Result<Value, CallingError> {
         if args.len() != self.arity() {
             return Err(CallingError::ArgumentMismatch(self.arity(), args.len()));
         }

src/resolver.rs

@@ -71,7 +71,7 @@ impl<'a> Resolver<'a> {
             Statement::Block(statements) => self.resolve_block(statements),
             Statement::Print(expression) => self.resolve_expression(expression),
             Statement::Expression(expression) => self.resolve_expression(expression),
-            Statement::Var { name, initializer } => self.resolve_var(name, *initializer.clone()),
+            Statement::Var { name, initializer } => self.resolve_var(name, initializer.as_ref().as_ref()),
             Statement::If {
                 condition,
                 then_branch,
@@ -262,12 +262,12 @@ impl<'a> Resolver<'a> {
     fn resolve_var(
         &mut self,
         name: &Token,
-        initializer: Option<Expression>,
+        initializer: Option<&Expression>,
     ) -> Result<(), ResolverError> {
         self.declare(name)?;
 
-        if let Some(initializer) = initializer {
+        if let Some(init_expr_ref) = initializer {
-            self.resolve_expression(&initializer)?;
+            self.resolve_expression(init_expr_ref)?;
         }
 
         self.define(name);