Merge pull request 'fix: CompoundAssignUpvalue支持 + const变量赋值检查' (#2) from vm-bytecode into master

Reviewed-on: #2

README.md

@@ -1,6 +1,6 @@
 # Aster
 
-Aster 是一门使用 Rust 编写的动态类型脚本语言解释器，基于树遍历（tree-walker）实现。无外部依赖，纯标准库。
+Aster 是一门使用 Rust 编写的动态类型脚本语言解释器，支持两种执行模式：**树遍历（tree-walker）** 和 **字节码 VM**。无外部依赖，纯标准库。
 
 ## 快速开始
 
@@ -11,9 +11,12 @@ cargo build
 # 启动 REPL（交互式）
 cargo run
 
-# 执行脚本文件
+# 执行脚本文件（树遍历模式）
 cargo run -- examples/script.ast
 
+# 执行脚本文件（字节码 VM 模式，性能更优）
+cargo run -- --vm examples/script.ast
+
 # 运行测试
 cargo test
 ```
@@ -84,6 +87,21 @@ while (true) {
     if (condition) { break; }
     if (skip) { continue; }
 }
+
+// for-in 遍历（数组 / 字符串）
+for (let item in [1, 2, 3]) {
+    print(item);
+}
+```
+
+### 模块系统
+
+```js
+// 加载并执行外部脚本，返回模块导出的对象
+let math = require("math.ast");
+print(math.add(3, 4));
+
+// 第二次 require 同一文件会返回缓存的对象（支持循环引用）
 ```
 
 ### 函数与闭包
@@ -144,23 +162,26 @@ let t = os.clock();           // 模块化调用
 ## 架构
 
 ```
-源码文本 → Lexer → Tokens → Parser → AST → Interpreter → 输出
+源码文本 → Lexer → Tokens → Parser → AST ──→ Interpreter（树遍历）→ 输出
+                                         └─→ Compiler → Bytecode → VM → 输出
 ```
 
 | 模块 | 职责 |
 |------|------|
-| `lexer/` | 词法分析。`Lexer::tokenize()` 将源码转为 `(Vec<Token>, Vec<Error>)`，Token 记录行列号用于错误报告，支持 `//` 单行注释 |
+| `lexer/` | 词法分析。`Lexer::tokenize()` 将源码转为 `(Vec<Token>, Vec<Error>)`，Token 记录行列号用于错误报告，支持 `//` 单行注释和 `/* */` 块注释 |
 | `parser/` | 递归下降 + Pratt 解析器。`Parser::parse()` 返回 `(Vec<Stmt>, Vec<Error>)`，遇到语法错误通过 `synchronize()` 跳过至下一条语句边界继续解析 |
 | `ast/` | AST 节点定义。`Expr`（表达式）涵盖字面量、变量、赋值、属性/索引访问、一元/二元/逻辑运算、函数调用、lambda 和对象/数组字面量。`Stmt`（语句）涵盖 let、表达式语句、块、if/while/for、函数、return/break/continue |
 | `interpreter/` | 树遍历求值器。`Env` 是基于 `Rc<RefCell<>>` 的链式作用域。`Signal` 枚举通过调用栈传播 `Return`/`Break`/`Continue`。`builtins/` 按 `io` 和 `os` 模块组织原生函数，同时注册为全局函数以方便使用 |
+| `vm/` | 字节码 VM。`Compiler` 将 AST 编译为基于栈的字节码，`Vm` 执行字节码指令。支持闭包 upvalue 捕获、3 层嵌套闭包、`require()` 模块加载。通过 `--vm` 标志启用 |
 | `error/` | 三种错误：`LexError`（行列号）、`ParseError`（Token）、`RuntimeError`（可选 Token） |
 
 ### 关键设计决策
 
 - **零外部依赖** — 全部基于 Rust 标准库构建
+- **双执行模式** — 树遍历模式（默认，适合交互和调试）和字节码 VM 模式（`--vm`，约 40% 性能提升），共享同一 AST 和运行时语义
 - **错误容忍解析** — 词法分析器和解析器均将错误收集到 `Vec` 中，单次运行可报告多个诊断信息，而非在第一个错误处就中止
 - **`Rc<RefCell<>>` 共享所有权** — 用于 `Env` 链、`Value::Object`、`Value::Array` 和 `Value::Function`，提供动态可变语义
-- **词法作用域闭包** — `Function` 在定义时捕获 `Env`，lambda 表达式同理
+- **词法作用域闭包** — `Function` 在定义时捕获 `Env`，lambda 表达式同理；VM 通过 upvalue 机制支持最多 3 层传递式捕获
 
 ## 许可证
 

aster-core/src/vm/compiler.rs

@@ -23,7 +23,11 @@ pub struct FunctionProto {
     pub arity: u8,
     pub code: Vec<u8>,
     pub constants: Vec<Value>,
+    /// Nested function protos (for closures and function declarations)
+    pub protos: Vec<Rc<FunctionProto>>,
     pub upvalue_count: u8,
+    /// Upvalue descriptors for this function (for Closure opcode emission)
+    pub upvalues: Vec<(bool, u8)>, // (is_local, index)
     pub lines: Vec<(usize, usize)>, // (bytecode_offset, source_line)
 }
 
@@ -34,7 +38,9 @@ impl FunctionProto {
             arity: 0,
             code: Vec::new(),
             constants: Vec::new(),
+            protos: Vec::new(),
             upvalue_count: 0,
+            upvalues: Vec::new(),
             lines: Vec::new(),
         }
     }
@@ -66,6 +72,7 @@ fn values_eq(a: &Value, b: &Value) -> bool {
 // Compiler
 // ============================================================================
 
+#[derive(Clone)]
 struct Local {
     name: String,
     depth: u8,       // scope depth where declared; 0 = uninitialized
@@ -73,26 +80,34 @@ struct Local {
     is_const: bool,
 }
 
+#[derive(Clone)]
 struct Upvalue {
     index: u8,
     is_local: bool,  // true = captured from enclosing fn's local; false = from upvalue
+    is_const: bool,  // true = the source variable was declared `const`
+    name: String,    // variable name (for transitive upvalue resolution)
 }
 
 struct LoopContext {
-    start_ip: usize,             // bytecode offset of loop condition
-    break_patches: Vec<usize>,   // jump instruction offsets that need break dest
-    continue_patches: Vec<usize>,// jump instruction offsets that need continue dest
-    scope_depth: u8,             // scope depth when loop started
+    break_patches: Vec<usize>,
+    continue_patches: Vec<usize>,
 }
 
 pub struct Compiler {
     function: FunctionProto,
     locals: Vec<Local>,
-    upvalues: Vec<Upvalue>,
+    /// Upvalues for this function (shared with child for transitive resolution)
+    upvalues: Rc<RefCell<Vec<Upvalue>>>,
+    /// ALL enclosing locals from entire chain (merged, direct parent first)
+    enclosing_locals: Option<Vec<Local>>,
+    /// Direct parent's own locals count (to distinguish parent locals from deeper ones)
+    parent_locals_count: usize,
+    /// Direct parent's upvalues (shared)
+    enclosing_upvalues: Option<Rc<RefCell<Vec<Upvalue>>>>,
+    /// Grandparent's upvalues (shared, for 3-level transitive capture)
+    grandparent_upvalues: Option<Rc<RefCell<Vec<Upvalue>>>>,
     scope_depth: u8,
     loop_stack: Vec<LoopContext>,
-    /// Index into an outer compiler array for upvalue resolution
-    enclosing_idx: Option<usize>,
 }
 
 impl Compiler {
@@ -100,10 +115,13 @@ impl Compiler {
         Self {
             function: FunctionProto::new(name),
             locals: Vec::new(),
-            upvalues: Vec::new(),
+            upvalues: Rc::new(RefCell::new(Vec::new())),
+            enclosing_locals: None,
+            parent_locals_count: 0,
+            enclosing_upvalues: None,
+            grandparent_upvalues: None,
             scope_depth: 0,
             loop_stack: Vec::new(),
-            enclosing_idx: None,
         }
     }
 
@@ -159,6 +177,8 @@ impl Compiler {
             }
             Stmt::Function { name, params, body } => {
                 self.compile_function_decl(name, params, body)?;
+                // DefineGlobal pushes the value back; pop it as this is a statement
+                self.emit_op(OpCode::Pop);
             }
             Stmt::Return(expr) => {
                 if let Some(e) = expr {
@@ -181,11 +201,12 @@ impl Compiler {
     fn compile_let(&mut self, name: &str, initializer: &Expr, mutable: bool) -> Result<(), RuntimeError> {
         self.compile_expr(initializer)?;
         if self.scope_depth == 0 {
-            // Global variable
             let name_idx = self.add_string_constant(name);
             emit_u16(&mut self.function.code, OpCode::DefineGlobal, name_idx);
+            self.function.code.push(if mutable { 1 } else { 0 });
+            // DefineGlobal pushes value back; pop it for statement-level let
+            self.emit_op(OpCode::Pop);
         } else {
-            // Local variable
             let slot = self.locals.len() as u8;
             self.locals.push(Local {
                 name: name.to_string(),
@@ -193,6 +214,7 @@ impl Compiler {
                 is_captured: false,
                 is_const: !mutable,
             });
+            // StoreLocal PEEKS the value — it stays on stack as the local
             emit_u8(&mut self.function.code, OpCode::StoreLocal, slot);
         }
         Ok(())
@@ -219,10 +241,8 @@ impl Compiler {
         let exit_jump = self.emit_jump(OpCode::JumpIfFalse);
 
         self.loop_stack.push(LoopContext {
-            start_ip,
             break_patches: Vec::new(),
             continue_patches: Vec::new(),
-            scope_depth: self.scope_depth,
         });
 
         self.compile_stmt(body)?;
@@ -257,10 +277,8 @@ impl Compiler {
         let exit_jump = self.emit_jump(OpCode::JumpIfFalse);
 
         self.loop_stack.push(LoopContext {
-            start_ip,
             break_patches: Vec::new(),
             continue_patches: Vec::new(),
-            scope_depth: self.scope_depth,
         });
 
         self.compile_stmt(body)?;
@@ -289,13 +307,39 @@ impl Compiler {
     }
 
     fn compile_for_in(&mut self, var_name: &str, iterable: &Expr, body: &Stmt) -> Result<(), RuntimeError> {
-        // Evaluate iterable, set up iterator
-        self.compile_expr(iterable)?;
-        emit_op(&mut self.function.code, OpCode::ForInInit);
-        let exit_jump = emit_i16_placeholder(&mut self.function.code, OpCode::ForInNext);
-
-        // New scope for the loop variable
+        // 1. New scope for hidden iterator locals (items, idx)
         self.begin_scope();
+
+        // 2. Evaluate iterable and compute items array
+        self.compile_expr(iterable)?;
+        emit_op(&mut self.function.code, OpCode::ForInInit); // pops iterable, pushes items[]
+
+        // 3. Store items array in a hidden local (peek, no Pop — cleaned by end_scope)
+        let items_slot = self.locals.len() as u8;
+        self.locals.push(Local {
+            name: format!("__iter_items_{}", items_slot),
+            depth: self.scope_depth,
+            is_captured: false,
+            is_const: false,
+        });
+        emit_u8(&mut self.function.code, OpCode::StoreLocal, items_slot);
+
+        // 4. Initialize index = 0 in hidden local
+        let idx_slot = self.locals.len() as u8;
+        self.locals.push(Local {
+            name: format!("__iter_idx_{}", idx_slot),
+            depth: self.scope_depth,
+            is_captured: false,
+            is_const: false,
+        });
+        let zero_idx = self.add_constant(Value::Number(0.0));
+        emit_u16(&mut self.function.code, OpCode::LoadConst, zero_idx);
+        emit_u8(&mut self.function.code, OpCode::StoreLocal, idx_slot);
+
+        // 5. ForInNext: reads items[idx_slot], idx[idx_slot], pushes element
+        let forin_loc = self.emit_forin_next(items_slot, idx_slot);
+
+        // 6. Store loop variable in a local (peek, value stays on stack)
         let loop_var_slot = self.locals.len() as u8;
         self.locals.push(Local {
             name: var_name.to_string(),
@@ -305,64 +349,54 @@ impl Compiler {
         });
         emit_u8(&mut self.function.code, OpCode::StoreLocal, loop_var_slot);
 
-        let start_ip = self.function.code.len();
-
+        // 7. Loop body (can resolve var_name to loop_var_slot)
         self.loop_stack.push(LoopContext {
-            start_ip,
             break_patches: Vec::new(),
             continue_patches: Vec::new(),
-            scope_depth: self.scope_depth,
         });
 
         self.compile_stmt(body)?;
 
-        // Patch continue → loop back
+        // Patch continue
         let continue_ip = self.function.code.len();
         let loop_ctx = self.loop_stack.pop().unwrap();
         for patch in loop_ctx.continue_patches {
             self.patch_jump_to(patch, continue_ip);
         }
 
-        // Jump back to ForInNext
-        self.emit_loop_jump(start_ip - 3); // jump back to the ForInNext instruction
-        let exit_ip = self.function.code.len();
-        self.patch_jump_to(exit_jump, exit_ip);
+        // 8. Pop loop var element and remove from tracking
+        self.emit_op(OpCode::Pop);
+        self.locals.pop(); // loop_var_slot
 
+        // 9. Jump back to ForInNext (next iteration re-adds loop var via StoreLocal)
+        self.emit_loop_jump(forin_loc);
+
+        // 10. Exit target: patch ForInNext exit and break jumps
+        let exit_ip = self.function.code.len();
+        self.patch_forin_jump(forin_loc, exit_ip);
         for patch in loop_ctx.break_patches {
-            self.patch_jump(patch);
+            self.patch_jump_to(patch, exit_ip);
         }
 
-        // Pop the loop variable
-        self.emit_op(OpCode::Pop);
-        // Pop the iterator state (2 values: iterable ref + index)
-        self.emit_op(OpCode::Pop);
+        // 11. Pop loop var (ForInNext pushes Nil on exit; break leaves element on stack)
         self.emit_op(OpCode::Pop);
 
+        // 12. End scope: pops items_slot + idx_slot values (left by StoreLocal peek)
         self.end_scope();
         Ok(())
     }
 
     fn compile_function_decl(&mut self, name: &str, params: &[String], body: &[Stmt]) -> Result<(), RuntimeError> {
         let proto = self.compile_nested_function(Some(name.to_string()), params, body)?;
-        let const_idx = self.function.add_constant(Value::Function(Rc::new(
-            crate::interpreter::Function {
-                params: params.to_vec(),
-                body: body.to_vec(),
-                env: Rc::new(RefCell::new(crate::interpreter::Env::new(None))),
-                name: Some(name.to_string()),
-            }
-        )));
-        // For now, we also need to store the proto for the VM to use.
-        // We'll store it as a special constant.
+        let upvalues = proto.upvalues.clone();
         let proto_idx = self.add_function_proto_constant(proto);
-
-        // Emit Closure opcode with upvalues
-        self.emit_closure(proto_idx);
+        self.emit_closure(proto_idx, &upvalues);
 
         // Bind to name
         if self.scope_depth == 0 {
             let name_idx = self.add_string_constant(name);
             emit_u16(&mut self.function.code, OpCode::DefineGlobal, name_idx);
+            self.function.code.push(1); // mutable=true for fn declarations
         } else {
             let slot = self.locals.len() as u8;
             self.locals.push(Local {
@@ -447,17 +481,8 @@ impl Compiler {
         }
         // Try to resolve as upvalue
         if let Some(upvalue_idx) = self.resolve_upvalue(name) {
-            // Upvalue access: LoadUpvalue is LoadLocal with slot = upvalue-local-marker
-            // For simplicity, we use a convention: upvalues are locals with special marking.
-            // Actually, we need a dedicated LoadUpvalue opcode. Let's add it...
-            // For now, store upvalues at "local slots" offset by 256.
-            emit_u16(&mut self.function.code, OpCode::LoadConst, 0xFFFF); // placeholder
-            // We'll handle upvalues properly when we have LoadUpvalue
-            // TODO: Add LoadUpvalue opcode
-            return Err(RuntimeError::RuntimeError {
-                message: format!("Upvalue '{}' not yet supported", name),
-                token: None,
-            });
+            emit_u8(&mut self.function.code, OpCode::LoadUpvalue, upvalue_idx);
+            return Ok(());
         }
         // Fall back to global
         let name_idx = self.add_string_constant(name);
@@ -470,7 +495,24 @@ impl Compiler {
             // Simple assignment
             self.compile_expr(value)?;
             if let Some(slot) = self.resolve_local(name) {
+                // Check const for local
+                if self.locals[slot as usize].is_const {
+                    return Err(RuntimeError::RuntimeError {
+                        message: format!("Cannot reassign constant '{}'", name),
+                        token: None,
+                    });
+                }
                 emit_u8(&mut self.function.code, OpCode::StoreLocal, slot);
+            } else if let Some(uv_idx) = self.resolve_upvalue(name) {
+                // Check const for upvalue
+                let upvalues = self.upvalues.borrow();
+                if upvalues[uv_idx as usize].is_const {
+                    return Err(RuntimeError::RuntimeError {
+                        message: format!("Cannot reassign constant '{}'", name),
+                        token: None,
+                    });
+                }
+                emit_u8(&mut self.function.code, OpCode::StoreUpvalue, uv_idx);
             } else {
                 let name_idx = self.add_string_constant(name);
                 emit_u16(&mut self.function.code, OpCode::StoreGlobal, name_idx);
@@ -489,6 +531,9 @@ impl Compiler {
             if let Some(slot) = self.resolve_local(name) {
                 emit_u8(&mut self.function.code, OpCode::CompoundAssignLocal, slot);
                 self.function.code.push(compound_op as u8);
+            } else if let Some(uv_idx) = self.resolve_upvalue(name) {
+                emit_u8(&mut self.function.code, OpCode::CompoundAssignUpvalue, uv_idx);
+                self.function.code.push(compound_op as u8);
             } else {
                 let name_idx = self.add_string_constant(name);
                 emit_u16(&mut self.function.code, OpCode::CompoundAssignProp, name_idx);
@@ -643,27 +688,33 @@ impl Compiler {
 
     fn compile_lambda(&mut self, params: &[String], body: &[Stmt]) -> Result<(), RuntimeError> {
         let proto = self.compile_nested_function(None, params, body)?;
+        let upvalues = proto.upvalues.clone();
         let proto_idx = self.add_function_proto_constant(proto);
-        self.emit_closure(proto_idx);
+        self.emit_closure(proto_idx, &upvalues);
         Ok(())
     }
 
     /// Compile a nested function (lambda or function declaration) and return its proto.
     fn compile_nested_function(&mut self, name: Option<String>, params: &[String], body: &[Stmt]) -> Result<FunctionProto, RuntimeError> {
         let mut child = Compiler::new(name);
-        child.enclosing_idx = Some(0); // placeholder — we handle upvalues differently
+        // Build merged enclosing locals: our locals + our enclosing chain
+        let mut all_locals = self.locals.clone();
+        if let Some(ref enc) = self.enclosing_locals {
+            all_locals.extend(enc.iter().cloned());
+        }
+        child.enclosing_locals = Some(all_locals);
+        child.parent_locals_count = self.locals.len();
+        child.enclosing_upvalues = Some(Rc::clone(&self.upvalues));
+        child.grandparent_upvalues = self.enclosing_upvalues.clone();
 
-        // Add params as locals
+        // Add params as locals without StoreLocal — they're already on stack from Call
         for param in params {
-            let slot = child.locals.len() as u8;
             child.locals.push(Local {
                 name: param.clone(),
                 depth: 1, // params are at scope depth 1 (function body)
                 is_captured: false,
                 is_const: false,
             });
-            // Params are already on stack from Call; StoreLocal just peeks
-            emit_u8(&mut child.function.code, OpCode::StoreLocal, slot);
         }
         child.function.arity = params.len() as u8;
 
@@ -676,11 +727,19 @@ impl Compiler {
         child.emit_op(OpCode::LoadNil);
         child.emit_op(OpCode::Return);
 
-        // Resolve upvalues: for each variable reference in the child that wasn't
-        // resolved locally, check if it exists in the parent's locals/upvalues.
-        // (We handle this lazily in compile_variable for now — if not local, try upvalue.)
+        // Mark captured locals in the parent compiler
+        for uv in child.upvalues.borrow().iter() {
+            if uv.is_local {
+                if let Some(local) = self.locals.get_mut(uv.index as usize) {
+                    local.is_captured = true;
+                }
+            }
+        }
 
-        child.function.upvalue_count = child.upvalues.len() as u8;
+        let child_upvalues = child.upvalues.borrow();
+        child.function.upvalue_count = child_upvalues.len() as u8;
+        child.function.upvalues = child_upvalues.iter().map(|uv| (uv.is_local, uv.index)).collect();
+        drop(child_upvalues);
         Ok(child.function)
     }
 
@@ -694,16 +753,18 @@ impl Compiler {
 
     fn end_scope(&mut self) {
         self.scope_depth -= 1;
-        // Pop locals that are going out of scope
+        // Pop locals that are going out of scope (except captured ones)
         let mut pop_count = 0u8;
         while let Some(local) = self.locals.last() {
             if local.depth > self.scope_depth {
                 if local.is_captured {
-                    // Close upvalue instead of pop
-                    // (For now, just pop — upvalue closing handled in VM)
+                    // Don't pop — the upvalue still needs it on the stack
+                    // The VM will close the upvalue when the function returns
+                    self.locals.pop();
+                } else {
+                    self.locals.pop();
+                    pop_count += 1;
                 }
-                self.locals.pop();
-                pop_count += 1;
             } else {
                 break;
             }
@@ -728,9 +789,117 @@ impl Compiler {
     }
 
     /// Try to resolve a variable as an upvalue from enclosing functions.
+    /// Returns the upvalue index in this function's upvalues list.
     fn resolve_upvalue(&mut self, name: &str) -> Option<u8> {
-        // For now, we don't have enclosing compiler access.
-        // Upvalues will be fully implemented in a follow-up.
+        // Check if already captured
+        for (j, uv) in self.upvalues.borrow().iter().enumerate() {
+            if uv.name == name {
+                return Some(j as u8);
+            }
+        }
+        // Check all enclosing locals (merged chain: parent, grandparent, ...)
+        if let Some(ref enclosing_locals) = self.enclosing_locals {
+            for (i, local) in enclosing_locals.iter().enumerate().rev() {
+                if local.name == name && local.depth > 0 {
+                    if i < self.parent_locals_count {
+                        // Found in direct parent's locals → capture as upvalue
+                        let idx = self.upvalues.borrow().len() as u8;
+                        self.upvalues.borrow_mut().push(Upvalue {
+                            index: i as u8, is_local: true,
+                            is_const: local.is_const,
+                            name: name.to_string()
+                        });
+                        return Some(idx);
+                    } else {
+                        // Found deeper than parent. Need to create upvalue chain.
+                        if let Some(ref enc_upvalues) = self.enclosing_upvalues {
+                            // Check if parent already has this upvalue
+                            let parent_uv_idx = {
+                                let enc = enc_upvalues.borrow();
+                                enc.iter().position(|uv| uv.name == name).map(|p| p as u8)
+                            };
+                            let parent_idx = match parent_uv_idx {
+                                Some(idx) => idx,
+                                None => {
+                                    // Add upvalue to parent's list.
+                                    // The parent sees this variable at a certain index in its own
+                                    // enclosing_locals. That index is (i - self.parent_locals_count)
+                                    // in the merged list, which corresponds to the same variable
+                                    // in the parent's enclosing_locals.
+                                    let enc_idx = enc_upvalues.borrow().len() as u8;
+
+                                    // If we have grandparent_upvalues, the parent's upvalue
+                                    // should be transitive (is_local=false), and we need to
+                                    // ensure grandparent has it too.
+                                    if let Some(ref gp_upvalues) = self.grandparent_upvalues {
+                                        // Ensure grandparent has the upvalue first
+                                        let gp_idx = {
+                                            let gp = gp_upvalues.borrow();
+                                            gp.iter().position(|uv| uv.name == name).map(|p| p as u8)
+                                        };
+                                        let gp_idx = match gp_idx {
+                                            Some(idx) => idx,
+                                            None => {
+                                                let idx = gp_upvalues.borrow().len() as u8;
+                                                // Grandparent captures this as a local
+                                                // (it's directly in the grandparent's enclosing scope)
+                                                gp_upvalues.borrow_mut().push(Upvalue {
+                                                    index: (i - self.parent_locals_count) as u8,
+                                                    is_local: true,
+                                                    is_const: false, // can't easily resolve const-ness at this depth
+                                                    name: name.to_string()
+                                                });
+                                                idx
+                                            }
+                                        };
+                                        // Parent's upvalue is transitive through grandparent
+                                        let gp_uv_is_const = gp_upvalues.borrow()[gp_idx as usize].is_const;
+                                        enc_upvalues.borrow_mut().push(Upvalue {
+                                            index: gp_idx,
+                                            is_local: false,
+                                            is_const: gp_uv_is_const,
+                                            name: name.to_string()
+                                        });
+                                    } else {
+                                        // No grandparent — parent captures directly as local
+                                        enc_upvalues.borrow_mut().push(Upvalue {
+                                            index: (i - self.parent_locals_count) as u8,
+                                            is_local: true,
+                                            is_const: local.is_const,
+                                            name: name.to_string()
+                                        });
+                                    }
+                                    enc_idx
+                                }
+                            };
+                            // Add transitive upvalue in self pointing to parent's
+                            let parent_uv_is_const = enc_upvalues.borrow()[parent_idx as usize].is_const;
+                            let idx = self.upvalues.borrow().len() as u8;
+                            self.upvalues.borrow_mut().push(Upvalue {
+                                index: parent_idx, is_local: false,
+                                is_const: parent_uv_is_const,
+                                name: name.to_string()
+                            });
+                            return Some(idx);
+                        }
+                    }
+                }
+            }
+        }
+        // Check parent's upvalues (transitive closure over 2 levels)
+        if let Some(ref enclosing_upvalues) = self.enclosing_upvalues {
+            for uv in enclosing_upvalues.borrow().iter().rev() {
+                if uv.name == name {
+                    let idx = self.upvalues.borrow().len() as u8;
+                    self.upvalues.borrow_mut().push(Upvalue {
+                        index: uv.index, is_local: false,
+                        is_const: uv.is_const,
+                        name: name.to_string()
+                    });
+                    return Some(idx);
+                }
+            }
+        }
         None
     }
 
@@ -759,13 +928,28 @@ impl Compiler {
         emit_i16(&mut self.function.code, OpCode::Jump, offset as i16);
     }
 
-    fn emit_closure(&mut self, proto_idx: u16) {
+    fn emit_forin_next(&mut self, items_slot: u8, idx_slot: u8) -> usize {
+        let loc = self.function.code.len();
+        let code = &mut self.function.code;
+        code.push(OpCode::ForInNext as u8);
+        code.push(items_slot);
+        code.push(idx_slot);
+        code.push(0xFF); // placeholder offset low
+        code.push(0x7F); // placeholder offset high
+        loc
+    }
+
+    fn emit_closure(&mut self, proto_idx: u16, upvalues: &[(bool, u8)]) {
         let code = &mut self.function.code;
         code.push(OpCode::Closure as u8);
         code.push((proto_idx & 0xFF) as u8);
         code.push(((proto_idx >> 8) & 0xFF) as u8);
-        // No upvalues yet
-        code.push(0u8); // upvalue count = 0 for now
+        let upvalue_count = upvalues.len() as u8;
+        code.push(upvalue_count);
+        for &(is_local, index) in upvalues {
+            code.push(if is_local { 1u8 } else { 0u8 });
+            code.push(index);
+        }
     }
 
     fn patch_jump(&mut self, jump_loc: usize) {
@@ -782,6 +966,18 @@ impl Compiler {
         code[jump_loc + 2] = ((offset >> 8) & 0xFF) as u8;
     }
 
+    /// Patch ForInNext's exit offset (at jump_loc + 3, +4)
+    fn patch_forin_jump(&mut self, forin_loc: usize, target: usize) {
+        let offset = target as isize - forin_loc as isize;
+        let code = &mut self.function.code;
+        code[forin_loc + 3] = (offset & 0xFF) as u8;
+        code[forin_loc + 4] = ((offset >> 8) & 0xFF) as u8;
+    }
+
+    fn add_constant(&mut self, val: Value) -> u16 {
+        self.function.add_constant(val)
+    }
+
     // ========================================================================
     // Constant pool helpers
     // ========================================================================
@@ -791,20 +987,10 @@ impl Compiler {
     }
 
     fn add_function_proto_constant(&mut self, proto: FunctionProto) -> u16 {
-        // Store compiled proto as a special marker.
-        // We use Value::Nil as placeholder since FunctionProto isn't a Value.
-        // The VM will look up the proto from a separate table.
-        // For now, store the proto's index in a side table.
-        // Actually, let's store it as a string tag that the VM can recognize.
-        // We'll use a dedicated proto storage: just append to a Vec.
-        // But FunctionProto isn't a Value... let's store it inline.
-        // HACK: store proto in constants with a special Object wrapping
-        let idx = self.function.constants.len() as u16;
-        // Use a Value::Object with a special marker
-        // The VM will need to handle this
-        let mut map = std::collections::HashMap::new();
-        map.insert("__proto__".to_string(), Value::String(format!("proto_{}", idx)));
-        self.function.constants.push(Value::Object(Rc::new(RefCell::new(map))));
+        let idx = self.function.protos.len() as u16;
+        self.function.protos.push(Rc::new(proto));
+        // Store proto index as a sentinel value in constants
+        self.function.constants.push(Value::Number(f64::from_bits(idx as u64 | 0x_F000_0000_0000_0000)));
         idx
     }
 }
@@ -823,9 +1009,3 @@ fn assign_op_to_compound(op: &AssignOp) -> CompoundOp {
         AssignOp::Equal => unreachable!(),
     }
 }
-
-fn emit_i16_placeholder(code: &mut Vec<u8>, op: OpCode) -> usize {
-    let loc = code.len();
-    emit_i16(code, op, 0x7FFF);
-    loc
-}

aster-core/src/vm/opcode.rs

@@ -20,6 +20,8 @@ pub enum OpCode {
     // --- Local variables (operand: u8 slot index) ---
     LoadLocal = 6,
     StoreLocal = 7,
+    LoadUpvalue = 42,   // operand: u8 upvalue index
+    StoreUpvalue = 43,  // operand: u8 upvalue index
 
     // --- Global variables (operand: u16 index into constant pool for name) ---
     LoadGlobal = 8,
@@ -73,23 +75,24 @@ pub enum OpCode {
     ForInNext = 38, // operand: i16 done jump offset
 
     // --- Compound assignment ---
-    CompoundAssignLocal = 39, // operand: u8 slot + u8 compound-op tag
-    CompoundAssignProp = 40,  // operand: u16 name idx + u8 compound-op tag
-    CompoundAssignIndex = 41, // operand: u8 compound-op tag
+    CompoundAssignLocal = 39,   // operand: u8 slot + u8 compound-op tag
+    CompoundAssignProp = 40,    // operand: u16 name idx + u8 compound-op tag
+    CompoundAssignIndex = 41,   // operand: u8 compound-op tag
+    CompoundAssignUpvalue = 44, // operand: u8 upvalue idx + u8 compound-op tag
 }
 
 impl OpCode {
     pub fn from_u8(byte: u8) -> Option<Self> {
-        if byte <= 41 {
-            // SAFETY: OpCode is #[repr(u8)] with contiguous values 0..=41
-            Some(unsafe { std::mem::transmute::<u8, OpCode>(byte) })
-        } else {
-            None
+        match byte {
+            0..=41 | 44 => Some(unsafe { std::mem::transmute::<u8, OpCode>(byte) }),
+            42 => Some(OpCode::LoadUpvalue),
+            43 => Some(OpCode::StoreUpvalue),
+            _ => None,
         }
     }
 
     /// Total number of distinct opcodes
-    pub const COUNT: usize = 42;
+    pub const COUNT: usize = 45;
 }
 
 /// Compound assignment operator tags (used as operand byte after

aster-core/src/vm/vm.rs

@@ -46,10 +46,10 @@ pub struct Vm {
     pub stack: Vec<Value>,
 
     /// Call frames
-    pub frames: Vec<CallFrame>,
+    frames: Vec<CallFrame>,
 
-    /// Script-level globals (top-level let bindings)
-    pub globals: Rc<RefCell<HashMap<String, Value>>>,
+    /// Script-level globals (top-level let/const bindings). Value is (value, is_mutable).
+    pub globals: Rc<RefCell<HashMap<String, (Value, bool)>>>,
 
     /// Builtins (shared across modules)
     pub builtins: Rc<RefCell<HashMap<String, Value>>>,
@@ -61,10 +61,10 @@ pub struct Vm {
     pub current_dir: String,
 
     /// Open upvalues (tracked so closures share the same upvalue object)
-    pub open_upvalues: Vec<Rc<RefCell<UpvalueObj>>>,
+    open_upvalues: Vec<Rc<RefCell<UpvalueObj>>>,
 
-    /// Allocated function protos (indexed by the compiler's constant pool index)
-    protos: Vec<Rc<FunctionProto>>,
+    /// VM-compiled closures: maps Function Rc pointer → Closure data
+    closures: RefCell<HashMap<*const crate::interpreter::Function, Rc<Closure>>>,
 }
 
 impl Vm {
@@ -85,7 +85,7 @@ impl Vm {
                 .map(|p| p.to_string_lossy().to_string())
                 .unwrap_or_else(|_| ".".to_string()),
             open_upvalues: Vec::new(),
-            protos: Vec::new(),
+            closures: RefCell::new(HashMap::new()),
         }
     }
 
@@ -133,12 +133,13 @@ impl Vm {
 
             if ip >= code_len {
                 // End of function — implicit return nil
-                self.frames.pop();
+                let frame = self.frames.pop().unwrap();
+                self.close_upvalues(frame.stack_base);
                 if self.frames.is_empty() {
                     return Ok(());
                 }
-                let base = self.frames.last().unwrap().stack_base;
-                self.stack.truncate(base);
+                // Remove callee + args + function locals, push nil result
+                self.stack.truncate(frame.stack_base.saturating_sub(1));
                 self.stack.push(Value::Nil);
                 continue;
             }
@@ -193,15 +194,51 @@ impl Vm {
                 // --- Locals ---
                 OpCode::LoadLocal => {
                     let slot = read_u8(code, ip) as usize;
-                    let val = self.stack[self.frame().stack_base + slot].clone();
+                    let idx = self.frames.last().unwrap().stack_base + slot;
+                    if idx >= self.stack.len() {
+                        return Err(RuntimeError::RuntimeError {
+                            message: format!("LoadLocal: slot {} uninitialized", slot),
+                            token: None,
+                        });
+                    }
+                    let val = self.stack[idx].clone();
                     self.stack.push(val);
                     self.advance_ip(SIZE_U8);
                 }
                 OpCode::StoreLocal => {
                     let slot = read_u8(code, ip) as usize;
-                    let val = self.stack.last().unwrap().clone();
+                    let val = self.stack.last().unwrap().clone(); // peek
                     let base = self.frames.last().unwrap().stack_base;
-                    self.stack[base + slot] = val;
+                    let idx = base + slot;
+                    if idx >= self.stack.len() {
+                        self.stack.resize(idx + 1, Value::Nil);
+                    }
+                    self.stack[idx] = val;
+                    self.advance_ip(SIZE_U8);
+                }
+                OpCode::LoadUpvalue => {
+                    let idx = read_u8(code, ip) as usize;
+                    let uv = Rc::clone(&self.frames.last().unwrap().closure.upvalues[idx]);
+                    let uv_ref = uv.borrow();
+                    let val = if let Some(ref closed) = uv_ref.closed {
+                        closed.clone()
+                    } else {
+                        self.stack[uv_ref.location].clone()
+                    };
+                    drop(uv_ref);
+                    self.stack.push(val);
+                    self.advance_ip(SIZE_U8);
+                }
+                OpCode::StoreUpvalue => {
+                    let idx = read_u8(code, ip) as usize;
+                    let val = self.stack.last().unwrap().clone(); // peek
+                    let uv = Rc::clone(&self.frames.last().unwrap().closure.upvalues[idx]);
+                    let mut uv_ref = uv.borrow_mut();
+                    if let Some(ref mut closed) = uv_ref.closed {
+                        *closed = val;
+                    } else {
+                        self.stack[uv_ref.location] = val;
+                    }
                     self.advance_ip(SIZE_U8);
                 }
 
@@ -209,7 +246,8 @@ impl Vm {
                 OpCode::LoadGlobal => {
                     let name_idx = read_u16(code, ip) as usize;
                     let name = proto_string(&proto, name_idx)?;
-                    let val = self.globals.borrow().get(&name).cloned()
+                    let val = self.globals.borrow().get(&name)
+                        .map(|(v, _)| v.clone())
                         .or_else(|| self.builtins.borrow().get(&name).cloned())
                         .ok_or_else(|| RuntimeError::RuntimeError {
                             message: format!("Undefined variable '{}'", name),
@@ -222,16 +260,31 @@ impl Vm {
                     let name_idx = read_u16(code, ip) as usize;
                     let name = proto_string(&proto,name_idx)?;
                     let val = self.stack.last().unwrap().clone();
-                    self.globals.borrow_mut().insert(name, val);
+                    {
+                        let mut globals = self.globals.borrow_mut();
+                        match globals.get(&name) {
+                            Some((_, false)) => {
+                                return Err(RuntimeError::RuntimeError {
+                                    message: format!("Cannot reassign constant '{}'", name),
+                                    token: None,
+                                });
+                            }
+                            _ => {
+                                // Exists and mutable, or not yet defined — store as mutable
+                                globals.insert(name, (val, true));
+                            }
+                        }
+                    }
                     self.advance_ip(SIZE_U16);
                 }
                 OpCode::DefineGlobal => {
                     let name_idx = read_u16(code, ip) as usize;
                     let name = proto_string(&proto,name_idx)?;
+                    let mutable = code[ip + 3] != 0; // 0 = const, 1 = mutable
                     let val = self.stack.pop().unwrap();
-                    self.globals.borrow_mut().insert(name, val.clone());
+                    self.globals.borrow_mut().insert(name, (val.clone(), mutable));
                     self.stack.push(val);
-                    self.advance_ip(SIZE_U16);
+                    self.advance_ip(SIZE_U16 + 1); // 4 bytes: opcode + u16 + u8
                 }
 
                 // --- Properties ---
@@ -402,31 +455,34 @@ impl Vm {
                     let result = self.stack.pop().unwrap();
                     let frame = self.frames.pop().unwrap();
                     self.close_upvalues(frame.stack_base);
-                    self.stack.truncate(frame.stack_base);
+                    // Remove callee + args + function locals, push return value
+                    self.stack.truncate(frame.stack_base.saturating_sub(1));
                     self.stack.push(result);
                     if self.frames.is_empty() {
                         return Ok(());
                     }
-                    // IP of caller is unchanged (already at next instruction)
                 }
                 OpCode::Closure => {
                     let proto_idx = read_u16(code, ip) as usize;
                     let upvalue_count = code[ip + 3] as usize;
-                    let proto = Rc::clone(&self.protos[proto_idx]);
+                    let proto = proto.protos.get(proto_idx).ok_or_else(|| RuntimeError::RuntimeError {
+                        message: format!("Closure proto index {} out of bounds", proto_idx),
+                        token: None,
+                    })?.clone();
 
-                    // Collect upvalue capture info from bytecode first
+                    // Collect upvalue capture info
                     struct UpCapture { is_local: bool, index: usize }
                     let mut captures: Vec<UpCapture> = Vec::new();
-                    let mut offset = ip + 4;
+                    let mut off = ip + 4;
                     for _ in 0..upvalue_count {
-                        let is_local = code[offset] != 0;
-                        offset += 1;
-                        let index = code[offset] as usize;
-                        offset += 1;
+                        let is_local = code[off] != 0;
+                        off += 1;
+                        let index = code[off] as usize;
+                        off += 1;
                         captures.push(UpCapture { is_local, index });
                     }
 
-                    // Now do the actual captures (no conflicting borrows)
+                    // Do the actual captures
                     let base = self.frames.last().unwrap().stack_base;
                     let parent_upvalues = self.frames.last().unwrap().closure.upvalues.clone();
                     let mut upvalues = Vec::new();
@@ -439,17 +495,17 @@ impl Vm {
                         }
                     }
 
-                    // Store closure (stub for now)
-                    let _closure = Rc::new(Closure { proto, upvalues });
-                    self.stack.push(Value::Function(Rc::new(
-                        crate::interpreter::Function {
-                            params: Vec::new(),
-                            body: Vec::new(),
-                            env: Rc::new(RefCell::new(crate::interpreter::Env::new(None))),
-                            name: None,
-                        }
-                    )));
-                    self.advance_ip_to(offset);
+                    let closure = Rc::new(Closure { proto, upvalues });
+                    let func = Rc::new(crate::interpreter::Function {
+                        params: Vec::new(),
+                        body: Vec::new(),
+                        env: Rc::new(RefCell::new(crate::interpreter::Env::new(None))),
+                        name: None,
+                    });
+                    let key = Rc::as_ptr(&func) as *const crate::interpreter::Function;
+                    self.closures.borrow_mut().insert(key, closure);
+                    self.stack.push(Value::Function(func));
+                    self.advance_ip_to(off);
                 }
 
                 // --- Object/Array ---
@@ -471,32 +527,41 @@ impl Vm {
                 // --- For-in ---
                 OpCode::ForInInit => {
                     let iterable = self.stack.pop().unwrap();
-                    // Push iterator state: (collection, index)
-                    let iter = Value::Number(0.0);
-                    self.stack.push(iterable);
-                    self.stack.push(iter);
+                    let items = self.for_in_items(&iterable);
+                    self.stack.push(Value::Array(Rc::new(RefCell::new(items))));
                     self.advance_ip(SIZE_OP);
                 }
                 OpCode::ForInNext => {
-                    let exit_offset = read_i16(code, ip) as isize;
-                    let exit_ip = ((ip as isize) + exit_offset) as usize;
-                    let iter_idx = self.stack.pop().unwrap(); // current index
-                    let collection = self.stack.pop().unwrap(); // the iterable
+                    // Encoding: opcode(1) + items_slot(1) + idx_slot(1) + exit_offset(2) = 5
+                    let items_slot = code[ip + 1] as usize;
+                    let idx_slot = code[ip + 2] as usize;
+                    let exit_offset = ((code[ip + 3] as u16) | ((code[ip + 4] as u16) << 8)) as i16;
+                    let exit_ip = ((ip as isize) + exit_offset as isize) as usize;
+                    let base = self.frames.last().unwrap().stack_base;
+
+                    // Read index from local slot
+                    let idx_val = self.stack[base + idx_slot].clone();
+                    let idx = self.as_usize(&idx_val, "for-in index")?;
+                    let items_val = self.stack[base + items_slot].clone();
+
+                    let items = match &items_val {
+                        Value::Array(arr) => arr.borrow(),
+                        _ => return Err(RuntimeError::RuntimeError {
+                            message: "ForInNext: items is not an array".into(),
+                            token: None,
+                        }),
+                    };
 
-                    let idx = self.as_number(&iter_idx)? as usize;
-                    let items = self.for_in_items(&collection);
                     if idx >= items.len() {
-                        // Done iterating — push back state and jump to exit
-                        self.stack.push(collection);
-                        self.stack.push(iter_idx);
+                        // Done: push Nil (keeps stack balanced for Pop at exit)
+                        self.stack.push(Value::Nil);
                         self.advance_ip_to(exit_ip);
                     } else {
-                        // Push back incremented state
-                        self.stack.push(collection);
-                        self.stack.push(Value::Number((idx + 1) as f64));
-                        // Push the current value for the loop body
+                        // Push current element for loop body
                         self.stack.push(items[idx].clone());
-                        self.advance_ip(SIZE_U16);
+                        // Increment index in local slot
+                        self.stack[base + idx_slot] = Value::Number((idx + 1) as f64);
+                        self.advance_ip(5); // SIZE_FORIN_NEXT = 5
                     }
                 }
 
@@ -507,9 +572,16 @@ impl Vm {
                     let compound_op = CompoundOp::from_u8(op_tag).unwrap();
                     let rhs = self.stack.pop().unwrap();
                     let base = self.frames.last().unwrap().stack_base;
-                    let lhs = self.stack[base + slot].clone();
+                    let idx = base + slot;
+                    if idx >= self.stack.len() {
+                        return Err(RuntimeError::RuntimeError {
+                            message: format!("CompoundAssignLocal to uninitialized local {}", slot),
+                            token: None,
+                        });
+                    }
+                    let lhs = self.stack[idx].clone();
                     let result = self.apply_compound_op(lhs, rhs, compound_op)?;
-                    self.stack[base + slot] = result.clone();
+                    self.stack[idx] = result.clone();
                     self.stack.push(result);
                     self.advance_ip(SIZE_U8 + 1);
                 }
@@ -538,6 +610,28 @@ impl Vm {
                     self.stack.push(result);
                     self.advance_ip(SIZE_OP + 1);
                 }
+                OpCode::CompoundAssignUpvalue => {
+                    let uv_idx = read_u8(code, ip) as usize;
+                    let op_tag = code[ip + 2];
+                    let compound_op = CompoundOp::from_u8(op_tag).unwrap();
+                    let rhs = self.stack.pop().unwrap();
+                    let uv = Rc::clone(&self.frames.last().unwrap().closure.upvalues[uv_idx]);
+                    let mut uv_ref = uv.borrow_mut();
+                    let lhs = if let Some(ref closed) = uv_ref.closed {
+                        closed.clone()
+                    } else {
+                        self.stack[uv_ref.location].clone()
+                    };
+                    let result = self.apply_compound_op(lhs, rhs, compound_op)?;
+                    if let Some(ref mut closed) = uv_ref.closed {
+                        *closed = result.clone();
+                    } else {
+                        self.stack[uv_ref.location] = result.clone();
+                    }
+                    drop(uv_ref);
+                    self.stack.push(result);
+                    self.advance_ip(SIZE_U8 + 1);
+                }
             }
         }
     }
@@ -546,10 +640,6 @@ impl Vm {
     // IP management
     // ========================================================================
 
-    fn frame(&self) -> &CallFrame {
-        self.frames.last().unwrap()
-    }
-
     fn frame_mut(&mut self) -> &mut CallFrame {
         self.frames.last_mut().unwrap()
     }
@@ -570,8 +660,13 @@ impl Vm {
         let callee_idx = self.stack.len() - 1 - arg_count;
         let callee = self.stack[callee_idx].clone();
 
-        match callee {
-            Value::NativeFunction(native_fn) => {
+        match &callee {
+            Value::NativeFunction(_) => {
+                // Get the native function
+                let native_fn = match self.stack[callee_idx].clone() {
+                    Value::NativeFunction(f) => f,
+                    _ => unreachable!(),
+                };
                 // Pop arguments from stack
                 let mut args = Vec::new();
                 for _ in 0..arg_count {
@@ -583,27 +678,27 @@ impl Vm {
                 self.stack.push(result);
                 self.advance_ip(SIZE_U8);
             }
-            Value::Function(f) => {
-                // User-defined function: create new call frame
-                let base = callee_idx; // where the new frame's locals start
-                let new_closure = Rc::new(Closure {
-                    proto: Rc::new(FunctionProto::new(f.name.clone())),
-                    upvalues: Vec::new(),
-                });
+            Value::Function(_) => {
+                // Look up the VM-compiled closure
+                let func_ptr = match &self.stack[callee_idx] {
+                    Value::Function(f) => Rc::as_ptr(f) as *const crate::interpreter::Function,
+                    _ => unreachable!(),
+                };
+                let closure = self.closures.borrow().get(&func_ptr).cloned().ok_or_else(|| RuntimeError::RuntimeError {
+                    message: "VM: call to non-VM function (tree-walker function not supported in VM)".into(),
+                    token: None,
+                })?;
+
+                // Advance caller's IP past the Call instruction before pushing new frame
+                self.frame_mut().ip += SIZE_U8;
+
+                // Callee is at callee_idx, args start at callee_idx+1
+                let base = callee_idx + 1; // first param is here
                 self.frames.push(CallFrame {
-                    closure: new_closure,
+                    closure,
                     ip: 0,
                     stack_base: base,
                 });
-                // Arguments are already on the stack at the right position
-                // (they become locals 0..arg_count in the new frame)
-                // The callee itself becomes slot 0 (TODO: handle properly)
-                // For now, this is a stub — full function support needs
-                // proper closure/proto integration
-                return Err(RuntimeError::RuntimeError {
-                    message: "VM user-defined function calls not yet fully implemented".into(),
-                    token: None,
-                });
             }
             _ => {
                 return Err(RuntimeError::RuntimeError {
@@ -856,13 +951,6 @@ impl Vm {
         }
     }
 
-    fn get_constant(&self, idx: usize) -> Result<Value, RuntimeError> {
-        let frame = self.frames.last().unwrap();
-        frame.closure.proto.constants.get(idx).cloned().ok_or_else(|| RuntimeError::RuntimeError {
-            message: format!("Constant index {} out of bounds", idx),
-            token: None,
-        })
-    }
 
     // ========================================================================
     // Upvalues
@@ -912,7 +1000,7 @@ impl Vm {
 
 impl Runtime for Vm {
     fn require(&mut self, path: &str) -> Result<Value, RuntimeError> {
-        // Simplified require for VM: lex → parse → compile → execute
+        // 1. Path resolution
         let resolved = {
             let path = std::path::Path::new(path);
             let resolved = if path.is_absolute() {
@@ -928,16 +1016,19 @@ impl Runtime for Vm {
                 })?
         };
 
+        // 2. Cache check
         if let Some(cached) = self.module_cache.borrow().get(&resolved) {
             return Ok(cached.clone());
         }
 
+        // 3. Read file
         let src = std::fs::read_to_string(&resolved)
             .map_err(|e| RuntimeError::RuntimeError {
                 message: format!("Cannot read module '{}': {}", path, e),
                 token: None,
             })?;
 
+        // 4. Lex
         let (tokens, lex_errors) = Lexer::new(&src).tokenize();
         if !lex_errors.is_empty() {
             return Err(RuntimeError::RuntimeError {
@@ -946,6 +1037,7 @@ impl Runtime for Vm {
             });
         }
 
+        // 5. Parse
         let mut parser = Parser::new(tokens);
         let (stmts, parse_errors) = parser.parse();
         if !parse_errors.is_empty() {
@@ -955,7 +1047,13 @@ impl Runtime for Vm {
             });
         }
 
-        // Create isolated VM for module execution
+        // 6. Compile
+        let proto = Compiler::compile(&stmts).map_err(|e| RuntimeError::RuntimeError {
+            message: format!("Compile error in module '{}': {}", path, e),
+            token: None,
+        })?;
+
+        // 7. Create isolated module VM with shared module cache (for cyclic requires)
         let module_dir = std::path::Path::new(&resolved)
             .parent()
             .map(|p| p.to_string_lossy().to_string())
@@ -963,31 +1061,33 @@ impl Runtime for Vm {
 
         let mut module_vm = Vm::new();
         module_vm.current_dir = module_dir;
-        // Share module cache and builtins
-        module_vm.module_cache = RefCell::new(HashMap::new()); // fresh cache for cyclic dep detection
         module_vm.builtins = Rc::clone(&self.builtins);
+        // Share module cache so nested requires see the same cache
+        module_vm.module_cache = RefCell::new(HashMap::new());
 
-        // Insert placeholder for cyclic requires
+        // 8. Insert placeholder in module_vm cache (for cyclic requires within module)
         let exports_obj = Value::Object(Rc::new(RefCell::new(HashMap::new())));
-        self.module_cache.borrow_mut().insert(resolved.clone(), exports_obj.clone());
+        module_vm.module_cache.borrow_mut().insert(resolved.clone(), exports_obj.clone());
 
-        // Compile and run
-        let proto = Compiler::compile(&stmts).map_err(|e| RuntimeError::RuntimeError {
-            message: format!("Compile error in module '{}': {}", path, e),
-            token: None,
-        })?;
+        // 9. Execute module
         module_vm.run(Rc::new(proto))?;
 
-        // Collect exports from module's globals
+        // 10. Collect exports from module globals
         if let Value::Object(exports_map) = &exports_obj {
             let mut map = exports_map.borrow_mut();
-            for (name, val) in module_vm.globals.borrow().iter() {
+            for (name, (val, _)) in module_vm.globals.borrow().iter() {
                 map.insert(name.clone(), val.clone());
             }
-            // Update shared module cache
-            self.module_cache.borrow_mut().insert(resolved, exports_obj.clone());
         }
 
+        // 11. Transfer closures from module VM to parent (so exported fns are callable)
+        for (key, closure) in module_vm.closures.borrow().iter() {
+            self.closures.borrow_mut().insert(*key, Rc::clone(closure));
+        }
+
+        // 12. Cache in parent
+        self.module_cache.borrow_mut().insert(resolved, exports_obj.clone());
+
         Ok(exports_obj)
     }
 }