Refactor Binder to use new scope structure

This commit refactors the Binder implementation to better align with the
updated scope structure.
Key changes include:

- Introducing `ExprContext` to distinguish between statement and
  expression contexts.
- Modifying `define_variable` to handle both global and local scope
  definitions more robustly.
- Updating `resolve_variable` to correctly handle upvalue captures,
  especially for global addresses.
- Adjusting `bind` and related functions to use the new `ExprContext`.
- Updating the refactoring log to reflect completed phases.

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)
+### Phase 1: Binder Infrastructure (Completed)
-1. [ ] Update `CompilerScope` to support hierarchical nesting via a stack in `FunctionCompiler`.
+1. [x] Update `CompilerScope` to support hierarchical nesting via a stack in `FunctionCompiler`.
-2. [ ] Implement `push_scope` and `pop_scope` in `Binder`.
+2. [x] Implement `push_scope` and `pop_scope` in `Binder`.
-3. [ ] Refactor `resolve_variable` to traverse the scope stack (inner-to-outer).
+3. [x] Refactor `resolve_variable` to traverse the scope stack (inner-to-outer).
-4. [ ] Update `define_variable` to enforce "No Shadowing" at the current scope level.
+4. [x] Update `define_variable` to enforce "No Shadowing" at the current scope level.
 
-### Phase 2: Statement/Expression Validation
+### Phase 2: Statement/Expression Validation (Completed)
-1. [ ] Introduce a mechanism to track "Context" (Expression vs. Statement) during binding.
+1. [x] Introduce a mechanism to track "Context" (Expression vs. Statement) during binding.
-2. [ ] Validate that `def` is only used in statement positions.
+2. [x] Validate that `def` is only used in statement positions.
-3. [ ] Enforce that the last node in a `BoundKind::Block` is an expression.
+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.

src/ast/compiler/binder.rs

@@ -10,7 +10,7 @@ use std::rc::Rc;
 
 #[derive(Debug, Clone)]
 struct LocalInfo {
-    slot: LocalSlot,
+    addr: Address,
     identity: Identity,
     // Note: Binder doesn't strictly need the type anymore,
     // but it might be useful for built-ins during resolution.
@@ -72,16 +72,35 @@ impl FunctionCompiler {
     ) -> Result<Address, String> {
         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();
-                if let Some((idx, existing_id)) = globals_map.get(name) {
+                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));
                     }
-                    return Ok(Address::Global(*idx));
+                    Address::Global(*idx)
-                }
+                } else {
-                let idx = GlobalIdx(globals_map.len() as u32);
+                    let idx = GlobalIdx(globals_map.len() as u32);
-                globals_map.insert(name.clone(), (idx, identity));
+                    globals_map.insert(name.clone(), (idx, identity.clone()));
-                Ok(Address::Global(idx))
+                    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();
@@ -95,7 +114,7 @@ impl FunctionCompiler {
                 current_scope.locals.insert(
                     name.clone(),
                     LocalInfo {
-                        slot,
+                        addr: Address::Local(slot),
                         identity,
                         _ty: StaticType::Any,
                     },
@@ -125,6 +144,12 @@ impl FunctionCompiler {
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ExprContext {
+    Expression,
+    Statement,
+}
+
 pub struct Binder {
     functions: Vec<FunctionCompiler>,
     // Globals mapping: Symbol -> (Index, DefinitionIdentity)
@@ -156,7 +181,7 @@ impl Binder {
         diagnostics: &mut Diagnostics,
     ) -> Result<(BoundNode, HashMap<Identity, Vec<Identity>>), String> {
         let mut binder = Self::new(globals);
-        let bound = binder.bind(node, diagnostics);
+        let bound = binder.bind(node, ExprContext::Expression, diagnostics);
 
         // Convert HashSet to sorted Vec
         let final_captures = binder
@@ -185,7 +210,7 @@ impl Binder {
         }
     }
 
-    pub fn bind(&mut self, node: &Node<UntypedKind>, diag: &mut Diagnostics) -> BoundNode {
+    pub fn bind(&mut self, node: &Node<UntypedKind>, ctx: ExprContext, diag: &mut Diagnostics) -> BoundNode {
         match &node.kind {
             UntypedKind::Nop => self.make_node(node.identity.clone(), BoundKind::Nop),
             UntypedKind::Constant(v) => {
@@ -215,16 +240,16 @@ impl Binder {
                 then_br,
                 else_br,
             } => {
-                let cond = self.bind(cond, diag);
+                let cond = self.bind(cond, ExprContext::Expression, diag);
 
                 self.functions.last_mut().unwrap().push_scope();
-                let then_br = self.bind(then_br, diag);
+                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, diag)));
+                    else_br_bound = Some(Rc::new(self.bind(e, ctx, diag)));
                     self.functions.last_mut().unwrap().pop_scope();
                 }
 
@@ -239,6 +264,12 @@ impl Binder {
             }
 
             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(
@@ -247,7 +278,7 @@ impl Binder {
                         crate::ast::compiler::bound_nodes::DeclarationKind::Variable,
                         diag,
                     );
-                    let val_node = self.bind(value, diag);
+                    let val_node = self.bind(value, ExprContext::Expression, diag);
 
                     if let Some(addr) = addr_opt {
                         self.make_node(
@@ -267,7 +298,7 @@ impl Binder {
                     // 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 val_node = self.bind(value, ExprContext::Expression, diag);
                     let target_node = self.bind_pattern(target, DeclarationKind::Variable, diag);
 
                     self.make_node(
@@ -281,7 +312,7 @@ impl Binder {
             }
 
             UntypedKind::Assign { target, value } => {
-                let val_node = self.bind(value, diag);
+                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) {
@@ -310,9 +341,9 @@ impl Binder {
             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)));
+                    bound_inputs.push(Rc::new(self.bind(input, ExprContext::Expression, diag)));
                 }
-                let bound_lambda = Rc::new(self.bind(lambda.as_ref(), diag));
+                let bound_lambda = Rc::new(self.bind(lambda.as_ref(), ExprContext::Expression, diag));
                 self.make_node(
                     node.identity.clone(),
                     BoundKind::Pipe {
@@ -332,7 +363,7 @@ impl Binder {
                 let params_bound = self.bind_pattern(params, DeclarationKind::Parameter, diag);
 
                 // 2. Bind the body
-                let body_bound = self.bind(body, diag);
+                let body_bound = self.bind(body, ExprContext::Expression, diag);
 
                 let compiled_fn = self.functions.pop().unwrap();
 
@@ -369,8 +400,8 @@ impl Binder {
             }
 
             UntypedKind::Call { callee, args } => {
-                let callee = self.bind(callee, diag);
+                let callee = self.bind(callee, ExprContext::Expression, diag);
-                let args = self.bind(args, diag);
+                let args = self.bind(args, ExprContext::Expression, diag);
 
                 self.make_node(
                     node.identity.clone(),
@@ -389,7 +420,7 @@ impl Binder {
                     );
                     return self.make_node(node.identity.clone(), BoundKind::Error);
                 }
-                let args = self.bind(args, diag);
+                let args = self.bind(args, ExprContext::Expression, diag);
                 self.make_node(
                     node.identity.clone(),
                     BoundKind::Again {
@@ -401,8 +432,9 @@ impl Binder {
             UntypedKind::Block { exprs } => {
                 self.functions.last_mut().unwrap().push_scope();
                 let mut bound_exprs = Vec::new();
-                for expr in exprs {
+                for (i, expr) in exprs.iter().enumerate() {
-                    bound_exprs.push(Rc::new(self.bind(expr, diag)));
+                    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(
@@ -414,7 +446,7 @@ impl Binder {
             UntypedKind::Tuple { elements } => {
                 let mut bound_elems = Vec::new();
                 for e in elements {
-                    bound_elems.push(Rc::new(self.bind(e, diag)));
+                    bound_elems.push(Rc::new(self.bind(e, ExprContext::Expression, diag)));
                 }
                 self.make_node(
                     node.identity.clone(),
@@ -429,8 +461,8 @@ impl Binder {
                 let mut layout_fields = Vec::new();
 
                 for (k, v) in fields {
-                    let key_node = self.bind(k, diag);
+                    let key_node = self.bind(k, ExprContext::Expression, diag);
-                    let val_node = self.bind(v, diag);
+                    let val_node = self.bind(v, ExprContext::Expression, diag);
 
                     if let BoundKind::Constant(crate::ast::types::Value::Keyword(kw)) =
                         key_node.kind
@@ -460,7 +492,7 @@ impl Binder {
             }
 
             UntypedKind::Expansion { call, expanded } => {
-                let bound_expanded = self.bind(expanded, diag);
+                let bound_expanded = self.bind(expanded, ctx, diag);
                 self.make_node(
                     node.identity.clone(),
                     BoundKind::Expansion {
@@ -503,13 +535,18 @@ impl Binder {
 
         // 1. Try local in current function
         if let Some(info) = self.functions[current_fn_idx].resolve_local(sym) {
-            return Some(Address::Local(info.slot));
+            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) {
-                let mut addr = Address::Local(info.slot);
+                // 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 {