From 9f15a00b98bf7751c63ee248201775467c549576 Mon Sep 17 00:00:00 2001 From: Giulio Agostini Date: Tue, 30 Jun 2026 15:05:34 +0200 Subject: [PATCH] Add static variable resolution with scope tracking - Add distance-based get_at on EnvironmentStack - Add distance-based assign_at on EnvironmentStack - Introduce ErrorSink to accumulate diagnostics across passes - Implement Resolver with a ScopeStack and per-node distance map - Extend interpreter to store locals distances for runtime lookup - Add tests for resolution behavior and error accumulation Add static variable resolution with scope tracking --- src/backend/environment.rs | 32 ++++ src/backend/interpreter.rs | 171 ++++++++++-------- src/common/lox_result.rs | 87 +++++++++ src/main.rs | 22 ++- src/middleend/mod.rs | 1 + src/middleend/scope_stack.rs | 146 +++++++++++++++ src/middleend/variable_resolution.rs | 254 ++++++++++++++++++++++----- tests/resolution.rs | 69 ++++++++ 8 files changed, 666 insertions(+), 116 deletions(-) create mode 100644 src/middleend/scope_stack.rs create mode 100644 tests/resolution.rs diff --git a/src/backend/environment.rs b/src/backend/environment.rs index f6ed87b..2ba5832 100755 --- a/src/backend/environment.rs +++ b/src/backend/environment.rs @@ -58,6 +58,38 @@ impl EnvironmentStack { Ok(value) } + /// Read a variable at a known scope `distance` from the innermost scope + /// (0 = innermost). Used with resolver-computed distances. + pub fn get_at(&self, distance: usize, name: &str) -> LoxResult { + let index = self.stack.len().checked_sub(distance + 1); + match index + .and_then(|i| self.stack.get(i)) + .and_then(|s| s.get(name)) + { + Some(value) => Ok(value.clone()), + None => runtime_error( + SourceSlice::synthetic(), + format!("Undefined variable '{}'", name), + ), + } + } + + /// Assign to a variable at a known scope `distance` from the innermost + /// scope (0 = innermost). + pub fn assign_at(&mut self, distance: usize, name: String, value: T) -> LoxResult { + let index = self.stack.len().checked_sub(distance + 1); + match index { + Some(i) if i < self.stack.len() => { + self.stack[i].insert(name, value.clone()); + Ok(value) + } + _ => runtime_error( + SourceSlice::synthetic(), + format!("Undefined variable '{}'", name), + ), + } + } + pub fn set(&mut self, name: String, value: T) -> LoxResult { let size = self.stack.len(); diff --git a/src/backend/interpreter.rs b/src/backend/interpreter.rs index bc6bb1a..e73f6c5 100755 --- a/src/backend/interpreter.rs +++ b/src/backend/interpreter.rs @@ -1,69 +1,26 @@ use crate::{ backend::environment::EnvironmentStack, common::{ - ast::{AstNode, AstNodeKind, Expr, Stmt}, + ast::{AstNode, Expr, NodeId, Stmt}, base_value::{BaseValue, NativeFunction, Number, Truthy}, lox_result::{runtime_error, LoxError, LoxResult}, }, frontend::{source_registry::SourceSlice, tokens::TokenType}, }; -use std::fmt::{Debug, Display}; +use std::collections::HashMap; pub struct Interpreter { enviorment: EnvironmentStack, + /// Per-reference scope distances from the resolver. Empty means "no + /// resolution pass was run", in which case lookups fall back to a dynamic + /// search of the scope chain. + locals: HashMap, } pub trait EvaluateInterpreter { fn evaluate(&mut self, stmt: T) -> LoxResult; } -impl EvaluateInterpreter> for Interpreter -where - Interpreter: EvaluateInterpreter, -{ - fn evaluate(&mut self, stmt: AstNode) -> LoxResult { - match self.evaluate(stmt.node.clone()) { - Ok(value) => Ok(value), - Err(err) => runtime_error(stmt.source_slice, err.get_message()), - } - } -} - -// Direct Expr evaluation to avoid infinite recursion -impl EvaluateInterpreter for Interpreter { - fn evaluate(&mut self, expr: Expr) -> LoxResult { - match expr { - Expr::Literal { value } => match value { - BaseValue::Function(mut func) => { - func.closure = Some(self.enviorment.clone()); - Ok(BaseValue::Function(func)) - } - _ => Ok(value), - }, - Expr::Identifier { name } => self.enviorment.get(&name), - Expr::Binary { - left, - operator, - right, - } => { - let left_val = self.evaluate(*left)?; - let right_val = self.evaluate(*right)?; - self.evaluate_binary(left_val, operator, right_val) - } - Expr::Unary { operator, operand } => { - let operand_val = self.evaluate(*operand)?; - self.evaluate_unary(operator, operand_val) - } - Expr::Grouping { expression } => self.evaluate(*expression), - Expr::Call { callee, arguments } => self.evaluate_call(callee, arguments), - Expr::Assign { name, value } => { - let value = self.evaluate(*value)?; - self.enviorment.set(name, value) - } - } - } -} - impl EvaluateInterpreter> for Interpreter { fn evaluate(&mut self, node: AstNode) -> LoxResult { let stmt = node.node; @@ -93,27 +50,93 @@ impl Interpreter { )) })), ); - Self { enviorment: env } + Self { + enviorment: env, + locals: HashMap::new(), + } } + + /// Install the resolver's per-reference scope distances. + pub fn set_locals(&mut self, locals: HashMap) { + self.locals = locals; + } + + /// Evaluate an expression node, threading its `NodeId` so resolved + /// variables and assignments can use their precomputed scope distance. + fn eval_expr(&mut self, node: &AstNode) -> LoxResult { + let result = match &node.node { + Expr::Literal { value } => match value { + BaseValue::Function(func) => { + let mut func = func.clone(); + func.closure = Some(self.enviorment.clone()); + Ok(BaseValue::Function(func)) + } + other => Ok(other.clone()), + }, + Expr::Identifier { name } => self.look_up_variable(name, node.id), + Expr::Binary { + left, + operator, + right, + } => { + let left_val = self.eval_expr(left)?; + let right_val = self.eval_expr(right)?; + self.evaluate_binary(left_val, operator.clone(), right_val) + } + Expr::Unary { operator, operand } => { + let operand_val = self.eval_expr(operand)?; + self.evaluate_unary(operator.clone(), operand_val) + } + Expr::Grouping { expression } => self.eval_expr(expression), + Expr::Call { callee, arguments } => self.evaluate_call(callee, arguments), + Expr::Assign { name, value } => { + let value = self.eval_expr(value)?; + self.assign_variable(name, node.id, value) + } + }; + // Give location-less runtime errors this node's span. + match result { + Err(LoxError::RuntimeError { + message, + source_slice, + }) if source_slice.is_synthetic() => runtime_error(node.source_slice.clone(), message), + other => other, + } + } + + /// Look up a variable: by resolved distance if known, else dynamically. + fn look_up_variable(&self, name: &str, id: NodeId) -> LoxResult { + match self.locals.get(&id) { + Some(&distance) => self.enviorment.get_at(distance, name), + None => self.enviorment.get(name), + } + } + + /// Assign a variable: at its resolved distance if known, else dynamically. + fn assign_variable( + &mut self, + name: &str, + id: NodeId, + value: BaseValue, + ) -> LoxResult { + match self.locals.get(&id) { + Some(&distance) => self.enviorment.assign_at(distance, name.to_string(), value), + None => self.enviorment.set(name.to_string(), value), + } + } + fn evaluate_call( &mut self, - callee: Box>, - arguments: Vec>, + callee: &AstNode, + arguments: &[AstNode], ) -> LoxResult { let source_slice = callee.source_slice.clone(); - // Estrai il nome della variabile se il callee è un identificatore - let function_name = match &callee.node { - Expr::Identifier { name } => Some(name.clone()), - _ => None, - }; - - let function = if let Some(name) = function_name { - // Se abbiamo un nome di variabile, ottieni la funzione dall'ambiente - self.enviorment.get(&name)? - } else { - // Altrimenti valuta l'espressione callee (per casi più complessi) - self.evaluate(*callee)? + // A bare identifier callee resolves through `locals`; anything else is + // evaluated as a general expression. + let function = match &callee.node { + Expr::Identifier { name } => self.look_up_variable(name, callee.id)?, + _ => self.eval_expr(callee)?, }; if !function.is_callable() { @@ -122,7 +145,7 @@ impl Interpreter { let evaluated_arguments = arguments .iter() - .map(|arg| self.evaluate(arg.clone())) + .map(|arg| self.eval_expr(arg)) .collect::, LoxError>>()?; match function { @@ -207,19 +230,19 @@ impl Interpreter { fn interpret_stmt_inner(&mut self, stmt: Stmt) -> LoxResult { match stmt { - Stmt::Expression { expression, .. } => self.evaluate(*expression), + Stmt::Expression { expression, .. } => self.eval_expr(&expression), Stmt::Print { expression, .. } => { - let value = self.evaluate(*expression)?; + let value = self.eval_expr(&expression)?; println!("{}", value); Ok(BaseValue::Nil) } Stmt::Block { statements, .. } => self.evaluate_block(*statements), - Stmt::Return { expression, .. } => self.evaluate(*expression), + Stmt::Return { expression, .. } => self.eval_expr(&expression), Stmt::VarDeclaration { name, initializer, .. } => { let value = match initializer { - Some(expr_node) => self.evaluate(*expr_node)?, + Some(expr_node) => self.eval_expr(&expr_node)?, None => BaseValue::Nil, }; self.enviorment.declare(name.clone(), value) @@ -236,7 +259,7 @@ impl Interpreter { condition, body, .. } => { let mut ret = BaseValue::Nil; - while self.evaluate(*condition.clone())?.is_truthy() { + while self.eval_expr(&condition)?.is_truthy() { match self.evaluate(*body.clone()) { Ok(val) => ret = val, Err(LoxError::Return { value, .. }) => { @@ -261,7 +284,7 @@ impl Interpreter { return runtime_error(source_slice, "Expected number literal"); } let mut ret = BaseValue::Nil; - while self.evaluate(*condition.clone())?.is_truthy() { + while self.eval_expr(&condition)?.is_truthy() { match self.evaluate(*body.clone()) { Ok(val) => ret = val, Err(LoxError::Return { value, .. }) => { @@ -284,12 +307,12 @@ impl Interpreter { elif_branch: Vec<(Box>, Box>)>, else_branch: Option>>, ) -> LoxResult { - let condition = self.evaluate(*condition)?; + let condition = self.eval_expr(&condition)?; match condition { BaseValue::Boolean(true) => self.evaluate(*then_branch), BaseValue::Boolean(false) => { for (elif_condition, elif_then_branch) in elif_branch { - let condition = self.evaluate(*elif_condition)?; + let condition = self.eval_expr(&elif_condition)?; match condition { BaseValue::Boolean(true) => { return self.evaluate(*elif_then_branch); @@ -327,7 +350,7 @@ impl Interpreter { let node = statement.node.clone(); match node { Stmt::Return { expression, .. } => { - let value = self.evaluate(*expression)?; + let value = self.eval_expr(&expression)?; result = Err(LoxError::Return { source_slice: statement.source_slice.clone(), diff --git a/src/common/lox_result.rs b/src/common/lox_result.rs index 63327b4..602d7c3 100755 --- a/src/common/lox_result.rs +++ b/src/common/lox_result.rs @@ -479,3 +479,90 @@ pub fn io_error(message: impl Into) -> LoxResult { message: message.into(), }) } + +/// Accumulates diagnostics during a pass instead of bailing on the first one. +/// +/// Static analyses (e.g. the resolver) `report` problems and keep traversing so +/// the user sees every error at once, then surface them at the end. +#[derive(Debug, Default)] +pub struct ErrorSink { + errors: Vec, +} + +impl ErrorSink { + pub fn new() -> Self { + Self::default() + } + + /// Record a diagnostic and keep going. + pub fn report(&mut self, error: LoxError) { + self.errors.push(error); + } + + pub fn has_errors(&self) -> bool { + !self.errors.is_empty() + } + + pub fn errors(&self) -> &[LoxError] { + &self.errors + } + + /// Merge another sink's errors into this one (e.g. from a separate pass). + pub fn extend(&mut self, other: ErrorSink) { + self.errors.extend(other.errors); + } + + pub fn into_errors(self) -> Vec { + self.errors + } + + /// Bridge to the single-error [`LoxResult`] API: `Ok` if empty, otherwise + /// the first reported error. + pub fn into_result(self) -> LoxResult<()> { + match self.errors.into_iter().next() { + Some(error) => Err(error), + None => Ok(()), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::frontend::source_registry::SourceSlice; + + fn parse_err(message: &str) -> LoxError { + LoxError::ParseError { + source_slice: SourceSlice::synthetic(), + message: message.to_string(), + } + } + + #[test] + fn empty_sink_is_ok() { + let sink = ErrorSink::new(); + assert!(!sink.has_errors()); + assert!(sink.into_result().is_ok()); + } + + #[test] + fn accumulates_multiple_errors() { + let mut sink = ErrorSink::new(); + sink.report(parse_err("first")); + sink.report(parse_err("second")); + assert!(sink.has_errors()); + assert_eq!(sink.errors().len(), 2); + assert!(sink.into_result().is_err()); + } + + #[test] + fn extend_merges_sinks() { + let mut a = ErrorSink::new(); + a.report(parse_err("a")); + let mut b = ErrorSink::new(); + b.report(parse_err("b1")); + b.report(parse_err("b2")); + a.extend(b); + assert_eq!(a.into_errors().len(), 3); + } +} diff --git a/src/main.rs b/src/main.rs index f79d8f8..e60e051 100755 --- a/src/main.rs +++ b/src/main.rs @@ -15,6 +15,7 @@ use crate::{ parser::Parser, source_registry::{SourceId, SourceRegistry}, }, + middleend::variable_resolution::Resolver, }; use std::env; use std::fs; @@ -185,7 +186,11 @@ impl LoxInterpreter { // Funzione per eseguire l'AST fn execute_ast(&self, ast: Vec>, debug: bool) -> LoxResult { + // Static resolution pass: compute variable scope distances and report + // any resolution errors before executing. + let locals = self.resolve(&ast)?; let mut interpreter = Interpreter::new(); + interpreter.set_locals(locals); let mut result = None; for (index, stmt) in ast.iter().enumerate() { @@ -204,6 +209,19 @@ impl LoxInterpreter { } } + // Static variable resolution; prints and returns the first error, if any. + fn resolve( + &self, + ast: &[AstNode], + ) -> LoxResult> { + Resolver::resolve_program(ast).map_err(|errors| { + for err in &errors { + err.print_with_context(&self.source_registry); + } + errors.into_iter().next().unwrap() + }) + } + fn process_source( &self, source_id: SourceId, @@ -245,8 +263,10 @@ impl LoxInterpreter { return Ok(format_ast(&ast)); } - // Stage 3: Interpretation + // Stage 3: Resolution + Interpretation + let locals = self.resolve(&ast)?; let mut interpreter = Interpreter::new(); + interpreter.set_locals(locals); let mut result = None; for (index, stmt) in ast.iter().enumerate() { diff --git a/src/middleend/mod.rs b/src/middleend/mod.rs index 05d6ea6..b361e3d 100755 --- a/src/middleend/mod.rs +++ b/src/middleend/mod.rs @@ -1,2 +1,3 @@ +pub mod scope_stack; pub mod variable_resolution; pub mod visit_ast; diff --git a/src/middleend/scope_stack.rs b/src/middleend/scope_stack.rs new file mode 100644 index 0000000..c19689a --- /dev/null +++ b/src/middleend/scope_stack.rs @@ -0,0 +1,146 @@ +//! A reusable lexical-scope stack for static analyses. +//! +//! Unlike [`EnvironmentStack`](crate::backend::environment::EnvironmentStack), +//! which stores runtime *values*, this stores per-binding *analysis state* +//! (for the resolver, a `bool` meaning "defined yet?"). It is generic over that +//! state so other passes (a type checker, an unused-variable lint, ...) can +//! reuse the same machinery. +//! +//! It starts **empty**: the global scope is intentionally untracked, so +//! [`ScopeStack::resolve`] returning `None` means "not local — assume global". + +use std::collections::HashMap; + +#[derive(Debug)] +pub struct ScopeStack { + scopes: Vec>, +} + +impl Default for ScopeStack { + fn default() -> Self { + Self::new() + } +} + +impl ScopeStack { + pub fn new() -> Self { + ScopeStack { scopes: Vec::new() } + } + + pub fn begin_scope(&mut self) { + self.scopes.push(HashMap::new()); + } + + pub fn end_scope(&mut self) { + self.scopes.pop(); + } + + pub fn is_empty(&self) -> bool { + self.scopes.is_empty() + } + + pub fn depth(&self) -> usize { + self.scopes.len() + } + + /// Insert `name` with `state` into the innermost scope (no-op at global). + pub fn declare(&mut self, name: impl Into, state: T) { + if let Some(scope) = self.scopes.last_mut() { + scope.insert(name.into(), state); + } + } + + /// Update an existing binding in the innermost scope (no-op if absent). + pub fn set_local(&mut self, name: &str, state: T) { + if let Some(scope) = self.scopes.last_mut() { + if let Some(slot) = scope.get_mut(name) { + *slot = state; + } + } + } + + /// Look up `name` in the innermost scope only. + pub fn get_local(&self, name: &str) -> Option<&T> { + self.scopes.last().and_then(|scope| scope.get(name)) + } + + /// Whether the innermost scope already declares `name`. + pub fn declared_in_current(&self, name: &str) -> bool { + self.scopes + .last() + .map_or(false, |scope| scope.contains_key(name)) + } + + /// Distance (in scopes) from the innermost scope to the one declaring + /// `name`, or `None` if it isn't in any scope (i.e. global). + pub fn resolve(&self, name: &str) -> Option { + self.scopes + .iter() + .rev() + .enumerate() + .find_map(|(distance, scope)| scope.contains_key(name).then_some(distance)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn declare_and_get_local() { + let mut scopes: ScopeStack = ScopeStack::new(); + scopes.begin_scope(); + scopes.declare("a", false); + assert_eq!(scopes.get_local("a"), Some(&false)); + scopes.set_local("a", true); + assert_eq!(scopes.get_local("a"), Some(&true)); + assert_eq!(scopes.get_local("missing"), None); + } + + #[test] + fn resolve_returns_distance_from_innermost() { + let mut scopes: ScopeStack = ScopeStack::new(); + scopes.begin_scope(); + scopes.declare("a", true); + scopes.begin_scope(); + scopes.declare("b", true); + assert_eq!(scopes.resolve("b"), Some(0)); + assert_eq!(scopes.resolve("a"), Some(1)); + assert_eq!(scopes.resolve("missing"), None); + } + + #[test] + fn shadowing_and_end_scope() { + let mut scopes: ScopeStack = ScopeStack::new(); + scopes.begin_scope(); + scopes.declare("a", true); + scopes.begin_scope(); + scopes.declare("a", false); + assert_eq!(scopes.get_local("a"), Some(&false)); + assert_eq!(scopes.resolve("a"), Some(0)); + scopes.end_scope(); + assert_eq!(scopes.get_local("a"), Some(&true)); + assert_eq!(scopes.resolve("a"), Some(0)); + } + + #[test] + fn declared_in_current_checks_only_innermost() { + let mut scopes: ScopeStack = ScopeStack::new(); + scopes.begin_scope(); + scopes.declare("a", true); + scopes.begin_scope(); + assert!(!scopes.declared_in_current("a")); + scopes.declare("a", true); + assert!(scopes.declared_in_current("a")); + } + + #[test] + fn global_scope_is_untracked() { + let mut scopes: ScopeStack = ScopeStack::new(); + // No scope pushed: declarations are no-ops and nothing resolves locally. + scopes.declare("a", true); + assert!(scopes.is_empty()); + assert_eq!(scopes.resolve("a"), None); + assert_eq!(scopes.get_local("a"), None); + } +} diff --git a/src/middleend/variable_resolution.rs b/src/middleend/variable_resolution.rs index 32467b5..c2be7e5 100755 --- a/src/middleend/variable_resolution.rs +++ b/src/middleend/variable_resolution.rs @@ -1,95 +1,267 @@ +//! Static variable resolution (Crafting Interpreters, chapter 11). +//! +//! Walks the AST with the shared [`Visitor`] traversal, tracking lexical scopes +//! in a [`ScopeStack`], and records for every variable *reference* how many +//! scopes up its declaration lives (`locals: NodeId -> distance`). It also +//! reports the resolution errors from chapter 11: +//! +//! * reading a local variable in its own initializer (`var a = a;`), +//! * declaring two variables with the same name in one local scope, +//! * `return` outside of any function. +//! +//! Diagnostics accumulate in an [`ErrorSink`] so a single pass surfaces them +//! all. The produced `locals` map is keyed by [`NodeId`] (stable identity), +//! ready for the interpreter to consume via distance-based lookup. + use std::collections::HashMap; use crate::{ - backend::environment::EnvironmentStack, common::{ ast::{AstNode, Expr, NodeId, Stmt}, - lox_result::{LoxError, LoxResult}, + base_value::{BaseValue, LoxFunction}, + lox_result::{ErrorSink, LoxError, LoxResult}, + }, + middleend::{ + scope_stack::ScopeStack, + visit_ast::{walk_expr, walk_function, walk_stmt, Visitor}, }, - frontend::source_registry::SourceSlice, - middleend::visit_ast::{walk_expr, walk_stmt, Visitor}, }; -struct Resolver { - scopes: EnvironmentStack, +/// Tracks whether resolution is currently inside a function body, so a +/// top-level `return` can be reported. +#[derive(Clone, Copy, PartialEq)] +enum FunctionType { + None, + Function, +} + +pub struct Resolver { + scopes: ScopeStack, locals: HashMap, + errors: ErrorSink, + current_function: FunctionType, } impl Resolver { pub fn new() -> Self { Resolver { - scopes: EnvironmentStack::new(), + scopes: ScopeStack::new(), locals: HashMap::new(), + errors: ErrorSink::new(), + current_function: FunctionType::None, } } - fn declare(&mut self, name: &String) { + /// Resolve a whole program, returning the per-reference scope distances or + /// the accumulated resolution errors. + pub fn resolve_program( + statements: &[AstNode], + ) -> Result, Vec> { + let mut resolver = Resolver::new(); + for statement in statements { + // Visiting only fails for fatal/internal errors, which this pass + // never produces; user-facing diagnostics go to `errors`. + let _ = resolver.visit_stmt(statement); + } + if resolver.errors.has_errors() { + Err(resolver.errors.into_errors()) + } else { + Ok(resolver.locals) + } + } + + fn declare(&mut self, name: &str, slice: &crate::frontend::source_registry::SourceSlice) { + if self.scopes.is_empty() { + return; // global scope is untracked + } + if self.scopes.declared_in_current(name) { + self.errors.report(LoxError::ParseError { + source_slice: slice.clone(), + message: format!("Already a variable named '{}' in this scope.", name), + }); + } + self.scopes.declare(name.to_string(), false); + } + + fn define(&mut self, name: &str) { if self.scopes.is_empty() { return; } - let scope = self.scopes.peek(); - let _ = self.scopes.set(name.clone(), false); + self.scopes.set_local(name, true); } - fn define(&mut self, name: &String) { - if self.scopes.is_empty() { - return; + fn resolve_local(&mut self, id: NodeId, name: &str) { + if let Some(distance) = self.scopes.resolve(name) { + self.locals.insert(id, distance); } - let _ = self.scopes.set(name.clone(), true); + // Not found locally: assume global, record nothing. } +} - fn resolve_local(&mut self, id: NodeId, name: &String) { - let depth = self.scopes.depth(); - for i in (0..depth).rev() { - if self.scopes.scope_contains(i, name) { - // Distance = how many scopes up from the innermost the binding lives. - self.locals.insert(id, depth - 1 - i); - return; - } - } - // Not found in any tracked scope: assume global, record nothing. +impl Default for Resolver { + fn default() -> Self { + Self::new() } } impl Visitor for Resolver { fn visit_stmt(&mut self, stmt: &AstNode) -> LoxResult<()> { match &stmt.node { - Stmt::VarDeclaration { name, .. } => { - self.declare(name); - // `walk_stmt` resolves the initializer (if present). - walk_stmt(self, stmt)?; - self.define(name); + Stmt::VarDeclaration { + name, initializer, .. + } => { + // A function declaration is a var bound to a function literal. + // Define its name *before* resolving the body so it can recurse; + // a plain variable is defined *after* its initializer so that + // `var a = a;` is caught. + let is_function = matches!( + initializer.as_deref().map(|node| &node.node), + Some(Expr::Literal { + value: BaseValue::Function(_) + }) + ); + self.declare(name, &stmt.source_slice); + if is_function { + self.define(name); + walk_stmt(self, stmt)?; + } else { + walk_stmt(self, stmt)?; // resolves the initializer, if any + self.define(name); + } Ok(()) } - // NOTE: assignment is now an `Expr::Assign`, not a statement. - // Add an `Expr::Assign` arm to `visit_expr` to resolve assignments. Stmt::Block { .. } => { - self.scopes.push_new_scope(); + self.scopes.begin_scope(); walk_stmt(self, stmt)?; - self.scopes.pop_scope(); + self.scopes.end_scope(); Ok(()) } - // Expression, Print, Return, If, While, For: default traversal. + Stmt::Return { .. } => { + if self.current_function == FunctionType::None { + self.errors.report(LoxError::ParseError { + source_slice: stmt.source_slice.clone(), + message: "Can't return from top-level code.".to_string(), + }); + } + walk_stmt(self, stmt) // resolve the returned expression + } + // Expression, Print, If, While, For: default traversal. _ => walk_stmt(self, stmt), } } fn visit_expr(&mut self, expr: &AstNode) -> LoxResult<()> { match &expr.node { - Expr::Identifier { name, .. } => { - if !self.scopes.is_empty() && self.scopes.get(name).is_ok() { - return Err(LoxError::ParseError { - source_slice: SourceSlice::synthetic(), - message: "Cant read local varialbe in it own initializer".to_string(), + Expr::Identifier { name } => { + if self.scopes.get_local(name) == Some(&false) { + self.errors.report(LoxError::ParseError { + source_slice: expr.source_slice.clone(), + message: "Can't read local variable in its own initializer.".to_string(), }); } - walk_expr(self, expr) + self.resolve_local(expr.id, name); + Ok(()) } Expr::Assign { name, .. } => { + walk_expr(self, expr)?; // resolve the assigned value first self.resolve_local(expr.id, name); - walk_expr(self, expr) + Ok(()) } _ => walk_expr(self, expr), } } + + fn visit_function(&mut self, function: &LoxFunction) -> LoxResult<()> { + let enclosing = std::mem::replace(&mut self.current_function, FunctionType::Function); + self.scopes.begin_scope(); + // Parameters carry no source slice of their own; use the body's. + let param_slice = function.body.source_slice.clone(); + for (param, _ty) in &function.parameters { + self.declare(param, ¶m_slice); + self.define(param); + } + walk_function(self, function)?; // resolves guard + body + self.scopes.end_scope(); + self.current_function = enclosing; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::frontend::lexer::Lexer; + use crate::frontend::parser::Parser; + + fn parse(src: &str) -> Vec> { + let tokens = Lexer::new(src.to_string(), 0) + .scans_tokens() + .expect("source should lex"); + Parser::new(tokens).parse().expect("source should parse") + } + + fn resolve(src: &str) -> Result, Vec> { + Resolver::resolve_program(&parse(src)) + } + + #[test] + fn global_variables_are_not_resolved() { + // Top-level (global) scope is untracked, so nothing is recorded. + let locals = resolve("x := 1; print x;").expect("should resolve"); + assert!(locals.is_empty()); + } + + #[test] + fn local_read_resolves_to_distance_zero() { + let locals = resolve("do x := 1; print x; end").expect("should resolve"); + assert_eq!(locals.len(), 1); + assert_eq!(*locals.values().next().unwrap(), 0); + } + + #[test] + fn nested_scope_resolves_to_outer_distance() { + let src = "do x := 1; do print x; end end"; + let locals = resolve(src).expect("should resolve"); + assert_eq!(locals.len(), 1); + // `x` is read one scope above where it is read from. + assert_eq!(*locals.values().next().unwrap(), 1); + } + + #[test] + fn reading_a_variable_in_its_own_initializer_is_an_error() { + let errors = resolve("do x := x; end").expect_err("should fail"); + assert!(errors + .iter() + .any(|e| e.get_message().contains("its own initializer"))); + } + + #[test] + fn duplicate_declaration_in_same_scope_is_an_error() { + let errors = resolve("do x := 1; x := 2; end").expect_err("should fail"); + assert!(errors + .iter() + .any(|e| e.get_message().contains("Already a variable"))); + } + + #[test] + fn return_at_top_level_is_an_error() { + let errors = resolve("return 1;").expect_err("should fail"); + assert!(errors.iter().any(|e| e.get_message().contains("top-level"))); + } + + #[test] + fn return_inside_a_function_is_allowed() { + let locals = resolve("f :: fn (n) do return n; end").expect("should resolve"); + // `n` resolves from the body block up to the parameter scope. + assert_eq!(locals.len(), 1); + assert_eq!(*locals.values().next().unwrap(), 1); + } + + #[test] + fn assignment_target_is_resolved() { + let locals = resolve("do x := 1; x = 2; end").expect("should resolve"); + // Both the assignment target and... only the target is a reference here. + assert!(locals.values().all(|&d| d == 0)); + assert!(!locals.is_empty()); + } } diff --git a/tests/resolution.rs b/tests/resolution.rs new file mode 100644 index 0000000..e099415 --- /dev/null +++ b/tests/resolution.rs @@ -0,0 +1,69 @@ +//! End-to-end pipeline tests: lexer -> parser -> resolver -> interpreter. +//! +//! These verify chapter-11 resolution wired into execution: a variable's +//! resolved scope distance is used for lookup (`get_at`), and resolution errors +//! abort before running. + +use rlox::backend::interpreter::{EvaluateInterpreter, Interpreter}; +use rlox::common::base_value::BaseValue; +use rlox::frontend::lexer::Lexer; +use rlox::frontend::parser::Parser; +use rlox::middleend::variable_resolution::Resolver; + +/// Run the whole pipeline, returning the value of the last statement. +fn run(src: &str) -> Result { + let tokens = Lexer::new(src.to_string(), 0) + .scans_tokens() + .map_err(|e| e.to_string())?; + let ast = Parser::new(tokens).parse().map_err(|e| e.to_string())?; + let locals = Resolver::resolve_program(&ast) + .map_err(|errors| errors.into_iter().next().unwrap().to_string())?; + let mut interpreter = Interpreter::new(); + interpreter.set_locals(locals); + let mut last = BaseValue::Nil; + for stmt in ast { + last = interpreter.evaluate(stmt).map_err(|e| e.to_string())?; + } + Ok(last) +} + +#[test] +fn block_local_variables_evaluate() { + assert_eq!(run("do x := 10; x + 5; end").unwrap().to_string(), "15"); +} + +#[test] +fn resolved_nested_closure_reads_captured_local() { + // `count` is read from inside a nested function, two scopes up; the + // resolver records distance 2 and the interpreter uses `get_at(2, ..)`. + let src = "\ +make :: fn () do + count := 10; + get :: fn () do + return count; + end + return get; +end +g := make(); +g(); +"; + assert_eq!(run(src).unwrap().to_string(), "10"); +} + +#[test] +fn use_before_initializer_is_rejected() { + let err = run("do x := x; end").expect_err("should be a resolution error"); + assert!(err.contains("its own initializer")); +} + +#[test] +fn top_level_return_is_rejected() { + let err = run("return 1;").expect_err("should be a resolution error"); + assert!(err.contains("top-level")); +} + +#[test] +fn duplicate_declaration_in_block_is_rejected() { + let err = run("do x := 1; x := 2; end").expect_err("should be a resolution error"); + assert!(err.contains("Already a variable")); +}