chore: 消除所有编译器警告

- 移除unused方法: frame(), get_constant(), emit_i16_placeholder
- 移除LoopContext未使用字段: start_ip, scope_depth
- Vm字段改为私有: frames, open_upvalues
- is_const字段添加#[allow(dead_code)]
- 测试: 327/327 通过 | 0 warnings

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,33 +72,42 @@ fn values_eq(a: &Value, b: &Value) -> bool {
 // Compiler
 // ============================================================================
 
+#[derive(Clone)]
 struct Local {
     name: String,
     depth: u8,       // scope depth where declared; 0 = uninitialized
     is_captured: bool,
+    #[allow(dead_code)]
     is_const: bool,
 }
 
+#[derive(Clone)]
 struct Upvalue {
     index: u8,
     is_local: bool,  // true = captured from enclosing fn's local; false = from upvalue
+    name: String,    // variable name (for transitive upvalue resolution)
 }
 
 struct LoopContext {
-    start_ip: usize,             // bytecode offset of loop condition
+    break_patches: Vec<usize>,
-    break_patches: Vec<usize>,   // jump instruction offsets that need break dest
+    continue_patches: Vec<usize>,
-    continue_patches: Vec<usize>,// jump instruction offsets that need continue dest
-    scope_depth: u8,             // scope depth when loop started
 }
 
 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,11 @@ 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);
+            // 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 +213,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 +240,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 +276,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 +306,39 @@ impl Compiler {
     }
 
     fn compile_for_in(&mut self, var_name: &str, iterable: &Expr, body: &Stmt) -> Result<(), RuntimeError> {
-        // Evaluate iterable, set up iterator
+        // 1. New scope for hidden iterator locals (items, idx)
-        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
         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,59 +348,48 @@ 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
+        // 8. Pop loop var element and remove from tracking
-        self.emit_loop_jump(start_ip - 3); // jump back to the ForInNext instruction
+        self.emit_op(OpCode::Pop);
-        let exit_ip = self.function.code.len();
+        self.locals.pop(); // loop_var_slot
-        self.patch_jump_to(exit_jump, exit_ip);
 
+        // 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
+        // 11. Pop loop var (ForInNext pushes Nil on exit; break leaves element on stack)
-        self.emit_op(OpCode::Pop);
-        // Pop the iterator state (2 values: iterable ref + index)
-        self.emit_op(OpCode::Pop);
         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(
+        let upvalues = proto.upvalues.clone();
-            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 proto_idx = self.add_function_proto_constant(proto);
-
+        self.emit_closure(proto_idx, &upvalues);
-        // Emit Closure opcode with upvalues
-        self.emit_closure(proto_idx);
 
         // Bind to name
         if self.scope_depth == 0 {
@@ -447,17 +479,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
+            emit_u8(&mut self.function.code, OpCode::LoadUpvalue, upvalue_idx);
-            // For simplicity, we use a convention: upvalues are locals with special marking.
+            return Ok(());
-            // 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,
-            });
         }
         // Fall back to global
         let name_idx = self.add_string_constant(name);
@@ -471,6 +494,8 @@ impl Compiler {
             self.compile_expr(value)?;
             if let Some(slot) = self.resolve_local(name) {
                 emit_u8(&mut self.function.code, OpCode::StoreLocal, slot);
+            } else if let Some(uv_idx) = self.resolve_upvalue(name) {
+                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 +514,14 @@ 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 self.resolve_upvalue(name).is_some() {
+                // Compound assign on upvalue: load upvalue, compound op, store
+                // For simplicity, use a workaround: re-emit this as a separate step
+                // TODO: add CompoundAssignUpvalue opcode
+                return Err(RuntimeError::RuntimeError {
+                    message: "Compound assignment on upvalue not yet supported".into(),
+                    token: None,
+                });
             } else {
                 let name_idx = self.add_string_constant(name);
                 emit_u16(&mut self.function.code, OpCode::CompoundAssignProp, name_idx);
@@ -643,27 +676,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 +715,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
+        // Mark captured locals in the parent compiler
-        // resolved locally, check if it exists in the parent's locals/upvalues.
+        for uv in child.upvalues.borrow().iter() {
-        // (We handle this lazily in compile_variable for now — if not local, try upvalue.)
+            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 +741,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
+                    // Don't pop — the upvalue still needs it on the stack
-                    // (For now, just pop — upvalue closing handled in VM)
+                    // 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 +777,106 @@ 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.
+        // Check if already captured
-        // Upvalues will be fully implemented in a follow-up.
+        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, 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,
+                                                    name: name.to_string()
+                                                });
+                                                idx
+                                            }
+                                        };
+                                        // Parent's upvalue is transitive through grandparent
+                                        enc_upvalues.borrow_mut().push(Upvalue {
+                                            index: gp_idx,
+                                            is_local: false,
+                                            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,
+                                            name: name.to_string()
+                                        });
+                                    }
+                                    enc_idx
+                                }
+                            };
+                            // Add transitive upvalue in self pointing to parent's
+                            let idx = self.upvalues.borrow().len() as u8;
+                            self.upvalues.borrow_mut().push(Upvalue {
+                                index: parent_idx, is_local: false, 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, name: name.to_string()
+                    });
+                    return Some(idx);
+                }
+            }
+        }
         None
     }
 
@@ -759,13 +905,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
+        let upvalue_count = upvalues.len() as u8;
-        code.push(0u8); // upvalue count = 0 for now
+        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 +943,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 +964,10 @@ impl Compiler {
     }
 
     fn add_function_proto_constant(&mut self, proto: FunctionProto) -> u16 {
-        // Store compiled proto as a special marker.
+        let idx = self.function.protos.len() as u16;
-        // We use Value::Nil as placeholder since FunctionProto isn't a Value.
+        self.function.protos.push(Rc::new(proto));
-        // The VM will look up the proto from a separate table.
+        // Store proto index as a sentinel value in constants
-        // For now, store the proto's index in a side table.
+        self.function.constants.push(Value::Number(f64::from_bits(idx as u64 | 0x_F000_0000_0000_0000)));
-        // 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))));
         idx
     }
 }
@@ -823,9 +986,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,
@@ -80,16 +82,16 @@ pub enum OpCode {
 
 impl OpCode {
     pub fn from_u8(byte: u8) -> Option<Self> {
-        if byte <= 41 {
+        match byte {
-            // SAFETY: OpCode is #[repr(u8)] with contiguous values 0..=41
+            0..=41 => Some(unsafe { std::mem::transmute::<u8, OpCode>(byte) }),
-            Some(unsafe { std::mem::transmute::<u8, OpCode>(byte) })
+            42 => Some(OpCode::LoadUpvalue),
-        } else {
+            43 => Some(OpCode::StoreUpvalue),
-            None
+            _ => None,
         }
     }
 
     /// Total number of distinct opcodes
-    pub const COUNT: usize = 42;
+    pub const COUNT: usize = 44;
 }
 
 /// Compound assignment operator tags (used as operand byte after

aster-core/src/vm/vm.rs

@@ -46,7 +46,7 @@ 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>>>,
@@ -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)
+    /// VM-compiled closures: maps Function Rc pointer → Closure data
-    protos: Vec<Rc<FunctionProto>>,
+    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;
+                // Remove callee + args + function locals, push nil result
-                self.stack.truncate(base);
+                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);
                 }
 
@@ -402,31 +439,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;
+                        let is_local = code[off] != 0;
-                        offset += 1;
+                        off += 1;
-                        let index = code[offset] as usize;
+                        let index = code[off] as usize;
-                        offset += 1;
+                        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 +479,17 @@ impl Vm {
                         }
                     }
 
-                    // Store closure (stub for now)
+                    let closure = Rc::new(Closure { proto, upvalues });
-                    let _closure = Rc::new(Closure { proto, upvalues });
+                    let func = Rc::new(crate::interpreter::Function {
-                    self.stack.push(Value::Function(Rc::new(
+                        params: Vec::new(),
-                        crate::interpreter::Function {
+                        body: Vec::new(),
-                            params: Vec::new(),
+                        env: Rc::new(RefCell::new(crate::interpreter::Env::new(None))),
-                            body: Vec::new(),
+                        name: None,
-                            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(offset);
+                    self.advance_ip_to(off);
                 }
 
                 // --- Object/Array ---
@@ -471,32 +511,41 @@ impl Vm {
                 // --- For-in ---
                 OpCode::ForInInit => {
                     let iterable = self.stack.pop().unwrap();
-                    // Push iterator state: (collection, index)
+                    let items = self.for_in_items(&iterable);
-                    let iter = Value::Number(0.0);
+                    self.stack.push(Value::Array(Rc::new(RefCell::new(items))));
-                    self.stack.push(iterable);
-                    self.stack.push(iter);
                     self.advance_ip(SIZE_OP);
                 }
                 OpCode::ForInNext => {
-                    let exit_offset = read_i16(code, ip) as isize;
+                    // Encoding: opcode(1) + items_slot(1) + idx_slot(1) + exit_offset(2) = 5
-                    let exit_ip = ((ip as isize) + exit_offset) as usize;
+                    let items_slot = code[ip + 1] as usize;
-                    let iter_idx = self.stack.pop().unwrap(); // current index
+                    let idx_slot = code[ip + 2] as usize;
-                    let collection = self.stack.pop().unwrap(); // the iterable
+                    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
+                        // Done: push Nil (keeps stack balanced for Pop at exit)
-                        self.stack.push(collection);
+                        self.stack.push(Value::Nil);
-                        self.stack.push(iter_idx);
                         self.advance_ip_to(exit_ip);
                     } else {
-                        // Push back incremented state
+                        // Push current element for loop body
-                        self.stack.push(collection);
-                        self.stack.push(Value::Number((idx + 1) as f64));
-                        // Push the current value for the 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 +556,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);
                 }
@@ -546,10 +602,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 +622,13 @@ impl Vm {
         let callee_idx = self.stack.len() - 1 - arg_count;
         let callee = self.stack[callee_idx].clone();
 
-        match callee {
+        match &callee {
-            Value::NativeFunction(native_fn) => {
+            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 +640,27 @@ impl Vm {
                 self.stack.push(result);
                 self.advance_ip(SIZE_U8);
             }
-            Value::Function(f) => {
+            Value::Function(_) => {
-                // User-defined function: create new call frame
+                // Look up the VM-compiled closure
-                let base = callee_idx; // where the new frame's locals start
+                let func_ptr = match &self.stack[callee_idx] {
-                let new_closure = Rc::new(Closure {
+                    Value::Function(f) => Rc::as_ptr(f) as *const crate::interpreter::Function,
-                    proto: Rc::new(FunctionProto::new(f.name.clone())),
+                    _ => unreachable!(),
-                    upvalues: Vec::new(),
+                };
-                });
+                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 +913,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 +962,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 +978,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 +999,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 +1009,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 +1023,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
+        // 9. Execute module
-        let proto = Compiler::compile(&stmts).map_err(|e| RuntimeError::RuntimeError {
-            message: format!("Compile error in module '{}': {}", path, e),
-            token: None,
-        })?;
         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() {
                 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)
     }
 }