From 94b57f83048ed7ccb28b9ec059aae110e9f05166 Mon Sep 17 00:00:00 2001 From: Sebastian Hugentobler Date: Wed, 28 May 2025 09:21:59 +0200 Subject: [PATCH] fix resolving of vars --- lox/fibonacci.lox | 1 - src/environment.rs | 49 ++++++++++++++++++++++---------------- src/interpreter.rs | 52 ++++++++++++++++++++++------------------- src/lib.rs | 3 --- src/native_functions.rs | 6 ++++- src/resolver.rs | 8 +++---- 6 files changed, 66 insertions(+), 53 deletions(-) diff --git a/lox/fibonacci.lox b/lox/fibonacci.lox index 630e3b2..117d457 100644 --- a/lox/fibonacci.lox +++ b/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; diff --git a/src/environment.rs b/src/environment.rs index e8e4747..8966ee8 100644 --- a/src/environment.rs +++ b/src/environment.rs @@ -55,15 +55,24 @@ impl Environment { } } - /// Assign a value to a variable at a specific lexical distance (number of scopes away). - /// Used by the resolver to handle closures correctly. + /// Assign a value to a variable in an environment at a specific lexical distance. pub fn assign_at( - &mut self, + env: Rc>, 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(&self, distance: usize, token: &Token) -> Result { - if let Some(v) = self.ancestor(distance)?.values.get(token.lexeme.as_str()) { + pub fn get_at( + env: Rc>, + distance: usize, + token: &Token, + ) -> Result { + 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 { - // println!("{distance}: {self:?}"); - if distance == 0 { - return Ok(self.clone()); - } - - match &self.enclosing { - Some(enclosing) => enclosing.borrow().ancestor(distance - 1), - None => Err(EnvironmentError::InvalidDistance), - } - } } diff --git a/src/interpreter.rs b/src/interpreter.rs index dabd215..7adfef3 100644 --- a/src/interpreter.rs +++ b/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 { if let Some(distance) = self.locals.get(expression) { - let superclass = self.environment.borrow().get_at(*distance, keyword)?; - let object = self.environment.borrow().get_at( + let superclass = Environment::get_at(self.environment.clone(), *distance, keyword)?; + 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 { - let value = self.evaluate(expression)?; + fn assign(&mut self, assign_expr: &Expression) -> Result { + if let Expression::Assign { name, value } = assign_expr { + let evaluated_value = self.evaluate(value)?; - if let Some(distance) = self.locals.get(expression) { - self.environment - .borrow_mut() - .assign_at(*distance, name, value.clone()) - .map_err(InterpreterError::UndefinedVariable)?; + if let Some(distance) = self.locals.get(assign_expr) { + Environment::assign_at( + self.environment.clone(), + *distance, + name, + evaluated_value.clone(), + )?; + } else { + self.globals + .borrow_mut() + .assign(name, evaluated_value.clone())?; + } + Ok(evaluated_value) } else { - self.globals - .borrow_mut() - .assign(name, value.clone()) - .map_err(InterpreterError::UndefinedVariable)?; + // This should ideally not be reached if called correctly from `evaluate` + unreachable!("Interpreter::assign called with a non-Assign expression"); } - - Ok(value) } /// Convert the literal value into a Value. @@ -537,15 +543,13 @@ impl Interpreter { expression: &Expression, ) -> Result { if let Some(distance) = self.locals.get(expression) { - self.environment - .borrow() - .get_at(*distance, name) - .map_err(InterpreterError::UndefinedVariable) + Ok(Environment::get_at( + self.environment.clone(), + *distance, + name, + )?) } else { - self.globals - .borrow() - .get(name) - .map_err(InterpreterError::UndefinedVariable) + Ok(self.globals.borrow().get(name)?) } } diff --git a/src/lib.rs b/src/lib.rs index c072493..d27e546 100644 --- a/src/lib.rs +++ b/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(()) diff --git a/src/native_functions.rs b/src/native_functions.rs index f72b221..2e61e13 100644 --- a/src/native_functions.rs +++ b/src/native_functions.rs @@ -159,7 +159,11 @@ impl Callable for AsciiOut { 1 } - fn call(&self, interpreter: &mut Interpreter, args: Vec) -> Result { + fn call( + &self, + _interpreter: &mut Interpreter, + args: Vec, + ) -> Result { if args.len() != self.arity() { return Err(CallingError::ArgumentMismatch(self.arity(), args.len())); } diff --git a/src/resolver.rs b/src/resolver.rs index 5e97ac6..5c39cc9 100644 --- a/src/resolver.rs +++ b/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, + initializer: Option<&Expression>, ) -> Result<(), ResolverError> { self.declare(name)?; - if let Some(initializer) = initializer { - self.resolve_expression(&initializer)?; + if let Some(init_expr_ref) = initializer { + self.resolve_expression(init_expr_ref)?; } self.define(name);