diff --git a/docs/binder-refactoring-log.md b/docs/binder-refactoring-log.md index 42c8cc7..c350774 100644 --- a/docs/binder-refactoring-log.md +++ b/docs/binder-refactoring-log.md @@ -20,16 +20,16 @@ Implement strict **Block Scoping** and distinguish between **Statements** and ** ## Implementation Plan -### Phase 1: Binder Infrastructure (In Progress) -1. [ ] Update `CompilerScope` to support hierarchical nesting via a stack in `FunctionCompiler`. -2. [ ] Implement `push_scope` and `pop_scope` in `Binder`. -3. [ ] Refactor `resolve_variable` to traverse the scope stack (inner-to-outer). -4. [ ] Update `define_variable` to enforce "No Shadowing" at the current scope level. +### Phase 1: Binder Infrastructure (Completed) +1. [x] Update `CompilerScope` to support hierarchical nesting via a stack in `FunctionCompiler`. +2. [x] Implement `push_scope` and `pop_scope` in `Binder`. +3. [x] Refactor `resolve_variable` to traverse the scope stack (inner-to-outer). +4. [x] Update `define_variable` to enforce "No Shadowing" at the current scope level. -### Phase 2: Statement/Expression Validation -1. [ ] Introduce a mechanism to track "Context" (Expression vs. Statement) during binding. -2. [ ] Validate that `def` is only used in statement positions. -3. [ ] Enforce that the last node in a `BoundKind::Block` is an expression. +### Phase 2: Statement/Expression Validation (Completed) +1. [x] Introduce a mechanism to track "Context" (Expression vs. Statement) during binding. +2. [x] Validate that `def` is only used in statement positions. +3. [x] Enforce that the last node in a `BoundKind::Block` is an expression. ### Phase 3: VM Compatibility 1. [ ] Ensure the VM handles "Void" results from statements correctly. @@ -43,3 +43,10 @@ Implement strict **Block Scoping** and distinguish between **Statements** and ** - Analyzed `design-flaw.myc`. - Defined the "Statement" concept to solve the uninitialized variable problem. - Initialized this log. + +### Phase 1 & 2 Completed +- Introduced scope stack into `FunctionCompiler`. +- Introduced `ExprContext` (`Expression` / `Statement`) in the `Binder`. +- `def` now acts strictly as a statement. +- Variables defined inside nested scopes (e.g. inside `if` or `do`) do not leak into outer local scopes anymore. +- Outstanding behavior to discuss: `def` inside nested scopes at the *root* level still leaks into globals. diff --git a/src/ast/compiler/binder.rs b/src/ast/compiler/binder.rs index 8f7e961..3771c44 100644 --- a/src/ast/compiler/binder.rs +++ b/src/ast/compiler/binder.rs @@ -1,750 +1,787 @@ -use crate::ast::compiler::bound_nodes::{ - Address, BoundKind, BoundNode, DeclarationKind, GlobalIdx, LocalSlot, UpvalueIdx, -}; -use crate::ast::diagnostics::Diagnostics; -use crate::ast::nodes::{Node, Symbol, UntypedKind}; -use crate::ast::types::{Identity, StaticType}; -use std::cell::RefCell; -use std::collections::HashMap; -use std::rc::Rc; - -#[derive(Debug, Clone)] -struct LocalInfo { - slot: LocalSlot, - identity: Identity, - // Note: Binder doesn't strictly need the type anymore, - // but it might be useful for built-ins during resolution. - // For now we keep it as Any or Unknown. - _ty: StaticType, -} - -#[derive(Debug, Clone)] -struct CompilerScope { - locals: HashMap, -} - -impl CompilerScope { - fn new() -> Self { - Self { - locals: HashMap::new(), - } - } -} - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum ScopeKind { - Root, - Local, -} - -struct FunctionCompiler { - identity: Identity, - scopes: Vec, - slot_count: u32, - upvalues: Vec
, - kind: ScopeKind, -} - -impl FunctionCompiler { - fn new(kind: ScopeKind, identity: Identity) -> Self { - Self { - identity, - scopes: vec![CompilerScope::new()], - slot_count: 0, - upvalues: Vec::new(), - kind, - } - } - - fn push_scope(&mut self) { - self.scopes.push(CompilerScope::new()); - } - - fn pop_scope(&mut self) { - self.scopes.pop(); - } - - fn define_variable( - &mut self, - name: &Symbol, - identity: Identity, - globals: &Rc>>, - ) -> Result { - match self.kind { - ScopeKind::Root => { - let mut globals_map = globals.borrow_mut(); - if let Some((idx, existing_id)) = globals_map.get(name) { - if *existing_id != identity { - return Err(format!("Variable '{}' is already defined in global scope.", name.name)); - } - return Ok(Address::Global(*idx)); - } - let idx = GlobalIdx(globals_map.len() as u32); - globals_map.insert(name.clone(), (idx, identity)); - Ok(Address::Global(idx)) - } - ScopeKind::Local => { - let current_scope = self.scopes.last_mut().unwrap(); - if current_scope.locals.contains_key(name) { - return Err(format!( - "Variable '{}' is already defined in this scope level.", - name.name - )); - } - let slot = LocalSlot(self.slot_count); - current_scope.locals.insert( - name.clone(), - LocalInfo { - slot, - identity, - _ty: StaticType::Any, - }, - ); - self.slot_count += 1; - Ok(Address::Local(slot)) - } - } - } - - fn resolve_local(&self, sym: &Symbol) -> Option { - for scope in self.scopes.iter().rev() { - if let Some(info) = scope.locals.get(sym) { - return Some(info.clone()); - } - } - None - } - - fn add_upvalue(&mut self, addr: Address) -> UpvalueIdx { - if let Some(idx) = self.upvalues.iter().position(|&a| a == addr) { - return UpvalueIdx(idx as u32); - } - let idx = UpvalueIdx(self.upvalues.len() as u32); - self.upvalues.push(addr); - idx - } -} - -pub struct Binder { - functions: Vec, - // Globals mapping: Symbol -> (Index, DefinitionIdentity) - globals: Rc>>, - // Map of Declaration Identity -> List of Lambda Identities that capture it - capture_map: HashMap>, -} - -impl Binder { - pub fn new(globals: Rc>>) -> Self { - let mut binder = Self { - functions: Vec::new(), - globals, - capture_map: HashMap::new(), - }; - binder.functions.push(FunctionCompiler::new( - ScopeKind::Root, - crate::ast::types::NodeIdentity::new(crate::ast::types::SourceLocation { - line: 0, - col: 0, - }), - )); - binder - } - - pub fn bind_root( - globals: Rc>>, - node: &Node, - diagnostics: &mut Diagnostics, - ) -> Result<(BoundNode, HashMap>), String> { - let mut binder = Self::new(globals); - let bound = binder.bind(node, diagnostics); - - // Convert HashSet to sorted Vec - let final_captures = binder - .capture_map - .into_iter() - .map(|(k, v)| (k, v.into_iter().collect())) - .collect(); - - Ok((bound, final_captures)) - } - - fn declare_variable( - &mut self, - name: &Symbol, - identity: Identity, - _kind: crate::ast::compiler::bound_nodes::DeclarationKind, - diag: &mut Diagnostics, - ) -> Option
{ - let current_fn = self.functions.last_mut().unwrap(); - match current_fn.define_variable(name, identity, &self.globals) { - Ok(addr) => Some(addr), - Err(e) => { - diag.push_error(e, None); - None - } - } - } - - pub fn bind(&mut self, node: &Node, diag: &mut Diagnostics) -> BoundNode { - match &node.kind { - UntypedKind::Nop => self.make_node(node.identity.clone(), BoundKind::Nop), - UntypedKind::Constant(v) => { - self.make_node(node.identity.clone(), BoundKind::Constant(v.clone())) - } - - UntypedKind::Identifier(sym) => { - if let Some(addr) = self.resolve_variable(sym, diag, &node.identity) { - self.make_node( - node.identity.clone(), - BoundKind::Get { - addr, - name: sym.clone(), - }, - ) - } else { - self.make_node(node.identity.clone(), BoundKind::Error) - } - } - - UntypedKind::FieldAccessor(k) => { - self.make_node(node.identity.clone(), BoundKind::FieldAccessor(*k)) - } - - UntypedKind::If { - cond, - then_br, - else_br, - } => { - let cond = self.bind(cond, diag); - - self.functions.last_mut().unwrap().push_scope(); - let then_br = self.bind(then_br, diag); - self.functions.last_mut().unwrap().pop_scope(); - - let mut else_br_bound = None; - if let Some(e) = else_br { - self.functions.last_mut().unwrap().push_scope(); - else_br_bound = Some(Rc::new(self.bind(e, diag))); - self.functions.last_mut().unwrap().pop_scope(); - } - - self.make_node( - node.identity.clone(), - BoundKind::If { - cond: Rc::new(cond), - then_br: Rc::new(then_br), - else_br: else_br_bound, - }, - ) - } - - UntypedKind::Def { target, value } => { - // Special case: Single identifier (to support recursion) - if let UntypedKind::Identifier(ref name) = target.kind { - let addr_opt = self.declare_variable( - name, - node.identity.clone(), // Identity of the Def node - crate::ast::compiler::bound_nodes::DeclarationKind::Variable, - diag, - ); - let val_node = self.bind(value, diag); - - if let Some(addr) = addr_opt { - self.make_node( - node.identity.clone(), - BoundKind::Define { - name: name.clone(), - addr, - kind: crate::ast::compiler::bound_nodes::DeclarationKind::Variable, - value: Rc::new(val_node), - captured_by: Vec::new(), // Will be filled by post-pass - }, - ) - } else { - self.make_node(node.identity.clone(), BoundKind::Error) - } - } else { - // Complex Destructuring Pattern - // NOTE: Destructuring definitions are NOT recursive by default - // (the variables are only available AFTER the definition) - let val_node = self.bind(value, diag); - let target_node = self.bind_pattern(target, DeclarationKind::Variable, diag); - - self.make_node( - node.identity.clone(), - BoundKind::Destructure { - pattern: Rc::new(target_node), - value: Rc::new(val_node), - }, - ) - } - } - - UntypedKind::Assign { target, value } => { - let val_node = self.bind(value, diag); - - if let UntypedKind::Identifier(sym) = &target.kind { - if let Some(addr) = self.resolve_variable(sym, diag, &target.identity) { - self.make_node( - node.identity.clone(), - BoundKind::Set { - addr, - value: Rc::new(val_node), - }, - ) - } else { - self.make_node(node.identity.clone(), BoundKind::Error) - } - } else { - let target_node = self.bind_assign_pattern(target, diag); - self.make_node( - node.identity.clone(), - BoundKind::Destructure { - pattern: Rc::new(target_node), - value: Rc::new(val_node), - }, - ) - } - } - - UntypedKind::Pipe { inputs, lambda } => { - let mut bound_inputs = Vec::with_capacity(inputs.len()); - for input in inputs { - bound_inputs.push(Rc::new(self.bind(input, diag))); - } - let bound_lambda = Rc::new(self.bind(lambda.as_ref(), diag)); - self.make_node( - node.identity.clone(), - BoundKind::Pipe { - inputs: bound_inputs, - lambda: bound_lambda, - out_type: crate::ast::types::StaticType::Any, - }, - ) - } - - UntypedKind::Lambda { params, body } => { - let identity = node.identity.clone(); - self.functions - .push(FunctionCompiler::new(ScopeKind::Local, identity.clone())); - - // 1. Bind the parameter pattern/tuple - let params_bound = self.bind_pattern(params, DeclarationKind::Parameter, diag); - - // 2. Bind the body - let body_bound = self.bind(body, diag); - - let compiled_fn = self.functions.pop().unwrap(); - - // 3. Static optimization: count total parameters needed in flat argument list - fn count_params(node: &BoundNode) -> Option { - match &node.kind { - BoundKind::Define { - kind: DeclarationKind::Parameter, - .. - } => Some(1), - BoundKind::Tuple { elements } => { - let mut total = 0; - for e in elements { - total += count_params(e)?; - } - Some(total) - } - BoundKind::Nop => Some(0), - _ => None, - } - } - - let positional_count = count_params(¶ms_bound); - - self.make_node( - identity, - BoundKind::Lambda { - params: Rc::new(params_bound), - upvalues: compiled_fn.upvalues, - body: Rc::new(body_bound), - positional_count, - }, - ) - } - - UntypedKind::Call { callee, args } => { - let callee = self.bind(callee, diag); - let args = self.bind(args, diag); - - self.make_node( - node.identity.clone(), - BoundKind::Call { - callee: Rc::new(callee), - args: Rc::new(args), - }, - ) - } - - UntypedKind::Again { args } => { - if self.functions.len() <= 1 { - diag.push_error( - "'again' is only allowed inside a function or lambda.", - Some(node.identity.clone()), - ); - return self.make_node(node.identity.clone(), BoundKind::Error); - } - let args = self.bind(args, diag); - self.make_node( - node.identity.clone(), - BoundKind::Again { - args: Rc::new(args), - }, - ) - } - - UntypedKind::Block { exprs } => { - self.functions.last_mut().unwrap().push_scope(); - let mut bound_exprs = Vec::new(); - for expr in exprs { - bound_exprs.push(Rc::new(self.bind(expr, diag))); - } - self.functions.last_mut().unwrap().pop_scope(); - self.make_node( - node.identity.clone(), - BoundKind::Block { exprs: bound_exprs }, - ) - } - - UntypedKind::Tuple { elements } => { - let mut bound_elems = Vec::new(); - for e in elements { - bound_elems.push(Rc::new(self.bind(e, diag))); - } - self.make_node( - node.identity.clone(), - BoundKind::Tuple { - elements: bound_elems, - }, - ) - } - - UntypedKind::Record { fields } => { - let mut bound_values = Vec::new(); - let mut layout_fields = Vec::new(); - - for (k, v) in fields { - let key_node = self.bind(k, diag); - let val_node = self.bind(v, diag); - - if let BoundKind::Constant(crate::ast::types::Value::Keyword(kw)) = - key_node.kind - { - layout_fields.push((kw, crate::ast::types::StaticType::Any)); - } else { - diag.push_error( - format!( - "Record keys must be keywords, found at {:?}", - key_node.identity.location - ), - Some(key_node.identity.clone()), - ); - } - bound_values.push(Rc::new(val_node)); - } - - let layout = crate::ast::types::RecordLayout::get_or_create(layout_fields); - - self.make_node( - node.identity.clone(), - BoundKind::Record { - layout, - values: bound_values, - }, - ) - } - - UntypedKind::Expansion { call, expanded } => { - let bound_expanded = self.bind(expanded, diag); - self.make_node( - node.identity.clone(), - BoundKind::Expansion { - original_call: Rc::from(call.as_ref().clone()), - bound_expanded: Rc::new(bound_expanded), - }, - ) - } - - UntypedKind::Template(_) - | UntypedKind::Placeholder(_) - | UntypedKind::Splice(_) - | UntypedKind::MacroDecl { .. } => { - diag.push_error(format!("Macro construct {:?} found in Binder. Macros must be expanded before binding.", node.kind), Some(node.identity.clone())); - self.make_node(node.identity.clone(), BoundKind::Error) - } - - UntypedKind::Extension(_) => { - diag.push_error( - "Custom extensions not supported in Binder yet", - Some(node.identity.clone()), - ); - self.make_node(node.identity.clone(), BoundKind::Error) - } - UntypedKind::Error => crate::ast::compiler::bound_nodes::BoundNode { - identity: node.identity.clone(), - kind: crate::ast::compiler::bound_nodes::BoundKind::Error, - ty: (), - }, - } - } - - fn resolve_variable( - &mut self, - sym: &Symbol, - diag: &mut Diagnostics, - identity: &Identity, - ) -> Option
{ - let current_fn_idx = self.functions.len() - 1; - - // 1. Try local in current function - if let Some(info) = self.functions[current_fn_idx].resolve_local(sym) { - return Some(Address::Local(info.slot)); - } - - // 2. Try enclosing scopes (capture chain) - for i in (0..current_fn_idx).rev() { - if let Some(info) = self.functions[i].resolve_local(sym) { - let mut addr = Address::Local(info.slot); - - // Record the capture for each lambda level in between - for k in (i + 1)..=current_fn_idx { - let lambda_id = self.functions[k].identity.clone(); - self.capture_map - .entry(info.identity.clone()) - .or_default() - .insert(lambda_id); - addr = Address::Upvalue(self.functions[k].add_upvalue(addr)); - } - return Some(addr); - } - } - - // 3. Try Global - let globals = self.globals.borrow(); - if let Some((idx, _)) = globals.get(sym) { - return Some(Address::Global(*idx)); - } - - // 4. Global Fallback - if sym.context.is_some() { - let fallback_sym = Symbol { - name: sym.name.clone(), - context: None, - }; - if let Some((idx, _)) = globals.get(&fallback_sym) { - return Some(Address::Global(*idx)); - } - } - - diag.push_error( - format!("Undefined variable '{}'", sym.name), - Some(identity.clone()), - ); - None - } - - fn bind_pattern( - &mut self, - node: &Node, - kind: DeclarationKind, - diag: &mut Diagnostics, - ) -> BoundNode { - match &node.kind { - UntypedKind::Identifier(sym) => { - if let Some(addr) = self.declare_variable(sym, node.identity.clone(), kind, diag) { - self.make_node( - node.identity.clone(), - BoundKind::Define { - name: sym.clone(), - addr, - kind, - value: Rc::new(self.make_node(node.identity.clone(), BoundKind::Nop)), - captured_by: Vec::new(), // Filled by post-pass - }, - ) - } else { - self.make_node(node.identity.clone(), BoundKind::Error) - } - } - UntypedKind::Tuple { elements } => { - let mut bound_elems = Vec::new(); - for e in elements { - bound_elems.push(Rc::new(self.bind_pattern(e, kind, diag))); - } - self.make_node( - node.identity.clone(), - BoundKind::Tuple { - elements: bound_elems, - }, - ) - } - _ => { - diag.push_error( - format!("Invalid node in pattern: {:?}", node.kind), - Some(node.identity.clone()), - ); - self.make_node(node.identity.clone(), BoundKind::Error) - } - } - } - - fn bind_assign_pattern( - &mut self, - node: &Node, - diag: &mut Diagnostics, - ) -> BoundNode { - match &node.kind { - UntypedKind::Identifier(sym) => { - if let Some(addr) = self.resolve_variable(sym, diag, &node.identity) { - self.make_node( - node.identity.clone(), - BoundKind::Set { - addr, - value: Rc::new(self.make_node(node.identity.clone(), BoundKind::Nop)), - }, - ) - } else { - self.make_node(node.identity.clone(), BoundKind::Error) - } - } - UntypedKind::Tuple { elements } => { - let mut bound_elems = Vec::new(); - for e in elements { - bound_elems.push(Rc::new(self.bind_assign_pattern(e, diag))); - } - self.make_node( - node.identity.clone(), - BoundKind::Tuple { - elements: bound_elems, - }, - ) - } - _ => { - diag.push_error( - format!("Invalid node in assignment pattern: {:?}", node.kind), - Some(node.identity.clone()), - ); - self.make_node(node.identity.clone(), BoundKind::Error) - } - } - } - - fn make_node(&self, identity: Identity, kind: BoundKind<()>) -> BoundNode { - Node { - identity, - kind, - ty: (), - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::ast::diagnostics::Diagnostics; - use crate::ast::parser::Parser; - - #[test] - fn test_upvalue_capture_sets_is_boxed() { - // Wrap in a lambda to ensure 'x' is a local variable, not a global - let source = "(fn [] (do (def x 10) (def f (fn [] x)) x))"; - let mut parser = Parser::new(source); - let untyped = parser.parse_expression(); - - let globals = Rc::new(RefCell::new(HashMap::new())); - let mut diagnostics = Diagnostics::new(); - let (bound, captures) = Binder::bind_root(globals, &untyped, &mut diagnostics).unwrap(); - - // Structure: Lambda -> Block -> [ Define(x), Define(f), Get(x) ] - if let BoundKind::Lambda { body, .. } = &bound.kind { - if let BoundKind::Block { exprs } = &body.kind { - let x_decl = &exprs[0]; - if let BoundKind::Define { addr, .. } = &x_decl.kind { - assert!(matches!(addr, Address::Local(_))); - assert!( - captures.contains_key(&x_decl.identity), - "Variable 'x' should have capturers because it is used in lambda 'f'" - ); - } else { - panic!( - "First expression in block should be Define, got {:?}", - x_decl.kind - ); - } - } else { - panic!("Lambda body should be a Block, got {:?}", body.kind); - } - } else { - panic!("Root should be a Lambda, got {:?}", bound.kind); - } - } - - #[test] - fn test_no_capture_not_boxed() { - let source = "(fn [] (do (def x 10) x))"; - let mut parser = Parser::new(source); - let untyped = parser.parse_expression(); - - let globals = Rc::new(RefCell::new(HashMap::new())); - let mut diagnostics = Diagnostics::new(); - let (bound, captures) = Binder::bind_root(globals, &untyped, &mut diagnostics).unwrap(); - - if let BoundKind::Lambda { body, .. } = &bound.kind { - if let BoundKind::Block { exprs } = &body.kind { - let x_decl = &exprs[0]; - if let BoundKind::Define { addr, .. } = &x_decl.kind { - assert!(matches!(addr, Address::Local(_))); - assert!( - !captures.contains_key(&x_decl.identity), - "Variable 'x' should NOT have any capturers" - ); - } else { - panic!("First expression should be Define"); - } - } else { - panic!("Lambda body should be a Block"); - } - } else { - panic!("Root should be a Lambda"); - } - } - - #[test] - fn test_redefinition_error() { - let source = "(do (def x 1) (def x 2))"; - let mut parser = Parser::new(source); - let untyped = parser.parse_expression(); - - let globals = Rc::new(RefCell::new(HashMap::new())); - let mut diagnostics = Diagnostics::new(); - let _ = Binder::bind_root(globals, &untyped, &mut diagnostics); - - assert!(diagnostics.has_errors()); - assert!(diagnostics.items[0].message.contains("already defined")); - } - - #[test] - fn test_repro_global_redefinition() { - let globals = Rc::new(RefCell::new(HashMap::new())); - - // First run: defines 'x' - let source1 = "(def x 1)"; - let untyped1 = Parser::new(source1).parse_expression(); - let mut diagnostics = Diagnostics::new(); - assert!(Binder::bind_root(globals.clone(), &untyped1, &mut diagnostics).is_ok()); - - // Second run: attempts to redefine 'x' in the same global environment - let source2 = "(def x 2)"; - let untyped2 = Parser::new(source2).parse_expression(); - let mut diagnostics2 = Diagnostics::new(); - let _ = Binder::bind_root(globals.clone(), &untyped2, &mut diagnostics2); - - assert!(diagnostics2.has_errors()); - assert!(diagnostics2.items[0].message.contains("already defined")); - } -} +use crate::ast::compiler::bound_nodes::{ + Address, BoundKind, BoundNode, DeclarationKind, GlobalIdx, LocalSlot, UpvalueIdx, +}; +use crate::ast::diagnostics::Diagnostics; +use crate::ast::nodes::{Node, Symbol, UntypedKind}; +use crate::ast::types::{Identity, StaticType}; +use std::cell::RefCell; +use std::collections::HashMap; +use std::rc::Rc; + +#[derive(Debug, Clone)] +struct LocalInfo { + addr: Address, + identity: Identity, + // Note: Binder doesn't strictly need the type anymore, + // but it might be useful for built-ins during resolution. + // For now we keep it as Any or Unknown. + _ty: StaticType, +} + +#[derive(Debug, Clone)] +struct CompilerScope { + locals: HashMap, +} + +impl CompilerScope { + fn new() -> Self { + Self { + locals: HashMap::new(), + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ScopeKind { + Root, + Local, +} + +struct FunctionCompiler { + identity: Identity, + scopes: Vec, + slot_count: u32, + upvalues: Vec
, + kind: ScopeKind, +} + +impl FunctionCompiler { + fn new(kind: ScopeKind, identity: Identity) -> Self { + Self { + identity, + scopes: vec![CompilerScope::new()], + slot_count: 0, + upvalues: Vec::new(), + kind, + } + } + + fn push_scope(&mut self) { + self.scopes.push(CompilerScope::new()); + } + + fn pop_scope(&mut self) { + self.scopes.pop(); + } + + fn define_variable( + &mut self, + name: &Symbol, + identity: Identity, + globals: &Rc>>, + ) -> Result { + match self.kind { + ScopeKind::Root => { + let current_scope = self.scopes.last_mut().unwrap(); + if current_scope.locals.contains_key(name) { + return Err(format!( + "Variable '{}' is already defined in this scope level.", + name.name + )); + } + + let mut globals_map = globals.borrow_mut(); + let addr = if let Some((idx, existing_id)) = globals_map.get(name) { + if *existing_id != identity { + return Err(format!("Variable '{}' is already defined in global scope.", name.name)); + } + Address::Global(*idx) + } else { + let idx = GlobalIdx(globals_map.len() as u32); + globals_map.insert(name.clone(), (idx, identity.clone())); + Address::Global(idx) + }; + + current_scope.locals.insert( + name.clone(), + LocalInfo { + addr, + identity, + _ty: StaticType::Any, + }, + ); + Ok(addr) + } + ScopeKind::Local => { + let current_scope = self.scopes.last_mut().unwrap(); + if current_scope.locals.contains_key(name) { + return Err(format!( + "Variable '{}' is already defined in this scope level.", + name.name + )); + } + let slot = LocalSlot(self.slot_count); + current_scope.locals.insert( + name.clone(), + LocalInfo { + addr: Address::Local(slot), + identity, + _ty: StaticType::Any, + }, + ); + self.slot_count += 1; + Ok(Address::Local(slot)) + } + } + } + + fn resolve_local(&self, sym: &Symbol) -> Option { + for scope in self.scopes.iter().rev() { + if let Some(info) = scope.locals.get(sym) { + return Some(info.clone()); + } + } + None + } + + fn add_upvalue(&mut self, addr: Address) -> UpvalueIdx { + if let Some(idx) = self.upvalues.iter().position(|&a| a == addr) { + return UpvalueIdx(idx as u32); + } + let idx = UpvalueIdx(self.upvalues.len() as u32); + self.upvalues.push(addr); + idx + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExprContext { + Expression, + Statement, +} + +pub struct Binder { + functions: Vec, + // Globals mapping: Symbol -> (Index, DefinitionIdentity) + globals: Rc>>, + // Map of Declaration Identity -> List of Lambda Identities that capture it + capture_map: HashMap>, +} + +impl Binder { + pub fn new(globals: Rc>>) -> Self { + let mut binder = Self { + functions: Vec::new(), + globals, + capture_map: HashMap::new(), + }; + binder.functions.push(FunctionCompiler::new( + ScopeKind::Root, + crate::ast::types::NodeIdentity::new(crate::ast::types::SourceLocation { + line: 0, + col: 0, + }), + )); + binder + } + + pub fn bind_root( + globals: Rc>>, + node: &Node, + diagnostics: &mut Diagnostics, + ) -> Result<(BoundNode, HashMap>), String> { + let mut binder = Self::new(globals); + let bound = binder.bind(node, ExprContext::Expression, diagnostics); + + // Convert HashSet to sorted Vec + let final_captures = binder + .capture_map + .into_iter() + .map(|(k, v)| (k, v.into_iter().collect())) + .collect(); + + Ok((bound, final_captures)) + } + + fn declare_variable( + &mut self, + name: &Symbol, + identity: Identity, + _kind: crate::ast::compiler::bound_nodes::DeclarationKind, + diag: &mut Diagnostics, + ) -> Option
{ + let current_fn = self.functions.last_mut().unwrap(); + match current_fn.define_variable(name, identity, &self.globals) { + Ok(addr) => Some(addr), + Err(e) => { + diag.push_error(e, None); + None + } + } + } + + pub fn bind(&mut self, node: &Node, ctx: ExprContext, diag: &mut Diagnostics) -> BoundNode { + match &node.kind { + UntypedKind::Nop => self.make_node(node.identity.clone(), BoundKind::Nop), + UntypedKind::Constant(v) => { + self.make_node(node.identity.clone(), BoundKind::Constant(v.clone())) + } + + UntypedKind::Identifier(sym) => { + if let Some(addr) = self.resolve_variable(sym, diag, &node.identity) { + self.make_node( + node.identity.clone(), + BoundKind::Get { + addr, + name: sym.clone(), + }, + ) + } else { + self.make_node(node.identity.clone(), BoundKind::Error) + } + } + + UntypedKind::FieldAccessor(k) => { + self.make_node(node.identity.clone(), BoundKind::FieldAccessor(*k)) + } + + UntypedKind::If { + cond, + then_br, + else_br, + } => { + let cond = self.bind(cond, ExprContext::Expression, diag); + + self.functions.last_mut().unwrap().push_scope(); + let then_br = self.bind(then_br, ctx, diag); + self.functions.last_mut().unwrap().pop_scope(); + + let mut else_br_bound = None; + if let Some(e) = else_br { + self.functions.last_mut().unwrap().push_scope(); + else_br_bound = Some(Rc::new(self.bind(e, ctx, diag))); + self.functions.last_mut().unwrap().pop_scope(); + } + + self.make_node( + node.identity.clone(), + BoundKind::If { + cond: Rc::new(cond), + then_br: Rc::new(then_br), + else_br: else_br_bound, + }, + ) + } + + UntypedKind::Def { target, value } => { + if ctx == ExprContext::Expression { + diag.push_error( + "Statement 'def' cannot be used as an expression.", + Some(node.identity.clone()), + ); + } + // Special case: Single identifier (to support recursion) + if let UntypedKind::Identifier(ref name) = target.kind { + let addr_opt = self.declare_variable( + name, + node.identity.clone(), // Identity of the Def node + crate::ast::compiler::bound_nodes::DeclarationKind::Variable, + diag, + ); + let val_node = self.bind(value, ExprContext::Expression, diag); + + if let Some(addr) = addr_opt { + self.make_node( + node.identity.clone(), + BoundKind::Define { + name: name.clone(), + addr, + kind: crate::ast::compiler::bound_nodes::DeclarationKind::Variable, + value: Rc::new(val_node), + captured_by: Vec::new(), // Will be filled by post-pass + }, + ) + } else { + self.make_node(node.identity.clone(), BoundKind::Error) + } + } else { + // Complex Destructuring Pattern + // NOTE: Destructuring definitions are NOT recursive by default + // (the variables are only available AFTER the definition) + let val_node = self.bind(value, ExprContext::Expression, diag); + let target_node = self.bind_pattern(target, DeclarationKind::Variable, diag); + + self.make_node( + node.identity.clone(), + BoundKind::Destructure { + pattern: Rc::new(target_node), + value: Rc::new(val_node), + }, + ) + } + } + + UntypedKind::Assign { target, value } => { + let val_node = self.bind(value, ExprContext::Expression, diag); + + if let UntypedKind::Identifier(sym) = &target.kind { + if let Some(addr) = self.resolve_variable(sym, diag, &target.identity) { + self.make_node( + node.identity.clone(), + BoundKind::Set { + addr, + value: Rc::new(val_node), + }, + ) + } else { + self.make_node(node.identity.clone(), BoundKind::Error) + } + } else { + let target_node = self.bind_assign_pattern(target, diag); + self.make_node( + node.identity.clone(), + BoundKind::Destructure { + pattern: Rc::new(target_node), + value: Rc::new(val_node), + }, + ) + } + } + + UntypedKind::Pipe { inputs, lambda } => { + let mut bound_inputs = Vec::with_capacity(inputs.len()); + for input in inputs { + bound_inputs.push(Rc::new(self.bind(input, ExprContext::Expression, diag))); + } + let bound_lambda = Rc::new(self.bind(lambda.as_ref(), ExprContext::Expression, diag)); + self.make_node( + node.identity.clone(), + BoundKind::Pipe { + inputs: bound_inputs, + lambda: bound_lambda, + out_type: crate::ast::types::StaticType::Any, + }, + ) + } + + UntypedKind::Lambda { params, body } => { + let identity = node.identity.clone(); + self.functions + .push(FunctionCompiler::new(ScopeKind::Local, identity.clone())); + + // 1. Bind the parameter pattern/tuple + let params_bound = self.bind_pattern(params, DeclarationKind::Parameter, diag); + + // 2. Bind the body + let body_bound = self.bind(body, ExprContext::Expression, diag); + + let compiled_fn = self.functions.pop().unwrap(); + + // 3. Static optimization: count total parameters needed in flat argument list + fn count_params(node: &BoundNode) -> Option { + match &node.kind { + BoundKind::Define { + kind: DeclarationKind::Parameter, + .. + } => Some(1), + BoundKind::Tuple { elements } => { + let mut total = 0; + for e in elements { + total += count_params(e)?; + } + Some(total) + } + BoundKind::Nop => Some(0), + _ => None, + } + } + + let positional_count = count_params(¶ms_bound); + + self.make_node( + identity, + BoundKind::Lambda { + params: Rc::new(params_bound), + upvalues: compiled_fn.upvalues, + body: Rc::new(body_bound), + positional_count, + }, + ) + } + + UntypedKind::Call { callee, args } => { + let callee = self.bind(callee, ExprContext::Expression, diag); + let args = self.bind(args, ExprContext::Expression, diag); + + self.make_node( + node.identity.clone(), + BoundKind::Call { + callee: Rc::new(callee), + args: Rc::new(args), + }, + ) + } + + UntypedKind::Again { args } => { + if self.functions.len() <= 1 { + diag.push_error( + "'again' is only allowed inside a function or lambda.", + Some(node.identity.clone()), + ); + return self.make_node(node.identity.clone(), BoundKind::Error); + } + let args = self.bind(args, ExprContext::Expression, diag); + self.make_node( + node.identity.clone(), + BoundKind::Again { + args: Rc::new(args), + }, + ) + } + + UntypedKind::Block { exprs } => { + self.functions.last_mut().unwrap().push_scope(); + let mut bound_exprs = Vec::new(); + for (i, expr) in exprs.iter().enumerate() { + let expr_ctx = if i == exprs.len() - 1 { ctx } else { ExprContext::Statement }; + bound_exprs.push(Rc::new(self.bind(expr, expr_ctx, diag))); + } + self.functions.last_mut().unwrap().pop_scope(); + self.make_node( + node.identity.clone(), + BoundKind::Block { exprs: bound_exprs }, + ) + } + + UntypedKind::Tuple { elements } => { + let mut bound_elems = Vec::new(); + for e in elements { + bound_elems.push(Rc::new(self.bind(e, ExprContext::Expression, diag))); + } + self.make_node( + node.identity.clone(), + BoundKind::Tuple { + elements: bound_elems, + }, + ) + } + + UntypedKind::Record { fields } => { + let mut bound_values = Vec::new(); + let mut layout_fields = Vec::new(); + + for (k, v) in fields { + let key_node = self.bind(k, ExprContext::Expression, diag); + let val_node = self.bind(v, ExprContext::Expression, diag); + + if let BoundKind::Constant(crate::ast::types::Value::Keyword(kw)) = + key_node.kind + { + layout_fields.push((kw, crate::ast::types::StaticType::Any)); + } else { + diag.push_error( + format!( + "Record keys must be keywords, found at {:?}", + key_node.identity.location + ), + Some(key_node.identity.clone()), + ); + } + bound_values.push(Rc::new(val_node)); + } + + let layout = crate::ast::types::RecordLayout::get_or_create(layout_fields); + + self.make_node( + node.identity.clone(), + BoundKind::Record { + layout, + values: bound_values, + }, + ) + } + + UntypedKind::Expansion { call, expanded } => { + let bound_expanded = self.bind(expanded, ctx, diag); + self.make_node( + node.identity.clone(), + BoundKind::Expansion { + original_call: Rc::from(call.as_ref().clone()), + bound_expanded: Rc::new(bound_expanded), + }, + ) + } + + UntypedKind::Template(_) + | UntypedKind::Placeholder(_) + | UntypedKind::Splice(_) + | UntypedKind::MacroDecl { .. } => { + diag.push_error(format!("Macro construct {:?} found in Binder. Macros must be expanded before binding.", node.kind), Some(node.identity.clone())); + self.make_node(node.identity.clone(), BoundKind::Error) + } + + UntypedKind::Extension(_) => { + diag.push_error( + "Custom extensions not supported in Binder yet", + Some(node.identity.clone()), + ); + self.make_node(node.identity.clone(), BoundKind::Error) + } + UntypedKind::Error => crate::ast::compiler::bound_nodes::BoundNode { + identity: node.identity.clone(), + kind: crate::ast::compiler::bound_nodes::BoundKind::Error, + ty: (), + }, + } + } + + fn resolve_variable( + &mut self, + sym: &Symbol, + diag: &mut Diagnostics, + identity: &Identity, + ) -> Option
{ + let current_fn_idx = self.functions.len() - 1; + + // 1. Try local in current function + if let Some(info) = self.functions[current_fn_idx].resolve_local(sym) { + return Some(info.addr); + } + + // 2. Try enclosing scopes (capture chain) + for i in (0..current_fn_idx).rev() { + if let Some(info) = self.functions[i].resolve_local(sym) { + // If the resolved address is Global, we don't need to capture it as an upvalue + if let Address::Global(_) = info.addr { + return Some(info.addr); + } + + let mut addr = info.addr; + + // Record the capture for each lambda level in between + for k in (i + 1)..=current_fn_idx { + let lambda_id = self.functions[k].identity.clone(); + self.capture_map + .entry(info.identity.clone()) + .or_default() + .insert(lambda_id); + addr = Address::Upvalue(self.functions[k].add_upvalue(addr)); + } + return Some(addr); + } + } + + // 3. Try Global + let globals = self.globals.borrow(); + if let Some((idx, _)) = globals.get(sym) { + return Some(Address::Global(*idx)); + } + + // 4. Global Fallback + if sym.context.is_some() { + let fallback_sym = Symbol { + name: sym.name.clone(), + context: None, + }; + if let Some((idx, _)) = globals.get(&fallback_sym) { + return Some(Address::Global(*idx)); + } + } + + diag.push_error( + format!("Undefined variable '{}'", sym.name), + Some(identity.clone()), + ); + None + } + + fn bind_pattern( + &mut self, + node: &Node, + kind: DeclarationKind, + diag: &mut Diagnostics, + ) -> BoundNode { + match &node.kind { + UntypedKind::Identifier(sym) => { + if let Some(addr) = self.declare_variable(sym, node.identity.clone(), kind, diag) { + self.make_node( + node.identity.clone(), + BoundKind::Define { + name: sym.clone(), + addr, + kind, + value: Rc::new(self.make_node(node.identity.clone(), BoundKind::Nop)), + captured_by: Vec::new(), // Filled by post-pass + }, + ) + } else { + self.make_node(node.identity.clone(), BoundKind::Error) + } + } + UntypedKind::Tuple { elements } => { + let mut bound_elems = Vec::new(); + for e in elements { + bound_elems.push(Rc::new(self.bind_pattern(e, kind, diag))); + } + self.make_node( + node.identity.clone(), + BoundKind::Tuple { + elements: bound_elems, + }, + ) + } + _ => { + diag.push_error( + format!("Invalid node in pattern: {:?}", node.kind), + Some(node.identity.clone()), + ); + self.make_node(node.identity.clone(), BoundKind::Error) + } + } + } + + fn bind_assign_pattern( + &mut self, + node: &Node, + diag: &mut Diagnostics, + ) -> BoundNode { + match &node.kind { + UntypedKind::Identifier(sym) => { + if let Some(addr) = self.resolve_variable(sym, diag, &node.identity) { + self.make_node( + node.identity.clone(), + BoundKind::Set { + addr, + value: Rc::new(self.make_node(node.identity.clone(), BoundKind::Nop)), + }, + ) + } else { + self.make_node(node.identity.clone(), BoundKind::Error) + } + } + UntypedKind::Tuple { elements } => { + let mut bound_elems = Vec::new(); + for e in elements { + bound_elems.push(Rc::new(self.bind_assign_pattern(e, diag))); + } + self.make_node( + node.identity.clone(), + BoundKind::Tuple { + elements: bound_elems, + }, + ) + } + _ => { + diag.push_error( + format!("Invalid node in assignment pattern: {:?}", node.kind), + Some(node.identity.clone()), + ); + self.make_node(node.identity.clone(), BoundKind::Error) + } + } + } + + fn make_node(&self, identity: Identity, kind: BoundKind<()>) -> BoundNode { + Node { + identity, + kind, + ty: (), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ast::diagnostics::Diagnostics; + use crate::ast::parser::Parser; + + #[test] + fn test_upvalue_capture_sets_is_boxed() { + // Wrap in a lambda to ensure 'x' is a local variable, not a global + let source = "(fn [] (do (def x 10) (def f (fn [] x)) x))"; + let mut parser = Parser::new(source); + let untyped = parser.parse_expression(); + + let globals = Rc::new(RefCell::new(HashMap::new())); + let mut diagnostics = Diagnostics::new(); + let (bound, captures) = Binder::bind_root(globals, &untyped, &mut diagnostics).unwrap(); + + // Structure: Lambda -> Block -> [ Define(x), Define(f), Get(x) ] + if let BoundKind::Lambda { body, .. } = &bound.kind { + if let BoundKind::Block { exprs } = &body.kind { + let x_decl = &exprs[0]; + if let BoundKind::Define { addr, .. } = &x_decl.kind { + assert!(matches!(addr, Address::Local(_))); + assert!( + captures.contains_key(&x_decl.identity), + "Variable 'x' should have capturers because it is used in lambda 'f'" + ); + } else { + panic!( + "First expression in block should be Define, got {:?}", + x_decl.kind + ); + } + } else { + panic!("Lambda body should be a Block, got {:?}", body.kind); + } + } else { + panic!("Root should be a Lambda, got {:?}", bound.kind); + } + } + + #[test] + fn test_no_capture_not_boxed() { + let source = "(fn [] (do (def x 10) x))"; + let mut parser = Parser::new(source); + let untyped = parser.parse_expression(); + + let globals = Rc::new(RefCell::new(HashMap::new())); + let mut diagnostics = Diagnostics::new(); + let (bound, captures) = Binder::bind_root(globals, &untyped, &mut diagnostics).unwrap(); + + if let BoundKind::Lambda { body, .. } = &bound.kind { + if let BoundKind::Block { exprs } = &body.kind { + let x_decl = &exprs[0]; + if let BoundKind::Define { addr, .. } = &x_decl.kind { + assert!(matches!(addr, Address::Local(_))); + assert!( + !captures.contains_key(&x_decl.identity), + "Variable 'x' should NOT have any capturers" + ); + } else { + panic!("First expression should be Define"); + } + } else { + panic!("Lambda body should be a Block"); + } + } else { + panic!("Root should be a Lambda"); + } + } + + #[test] + fn test_redefinition_error() { + let source = "(do (def x 1) (def x 2))"; + let mut parser = Parser::new(source); + let untyped = parser.parse_expression(); + + let globals = Rc::new(RefCell::new(HashMap::new())); + let mut diagnostics = Diagnostics::new(); + let _ = Binder::bind_root(globals, &untyped, &mut diagnostics); + + assert!(diagnostics.has_errors()); + assert!(diagnostics.items[0].message.contains("already defined")); + } + + #[test] + fn test_repro_global_redefinition() { + let globals = Rc::new(RefCell::new(HashMap::new())); + + // First run: defines 'x' + let source1 = "(def x 1)"; + let untyped1 = Parser::new(source1).parse_expression(); + let mut diagnostics = Diagnostics::new(); + assert!(Binder::bind_root(globals.clone(), &untyped1, &mut diagnostics).is_ok()); + + // Second run: attempts to redefine 'x' in the same global environment + let source2 = "(def x 2)"; + let untyped2 = Parser::new(source2).parse_expression(); + let mut diagnostics2 = Diagnostics::new(); + let _ = Binder::bind_root(globals.clone(), &untyped2, &mut diagnostics2); + + assert!(diagnostics2.has_errors()); + assert!(diagnostics2.items[0].message.contains("already defined")); + } +}