Space Lua: <close> attribute, __close metamethod and extensive tests (#1791)

Signed-off-by: Matouš Jan Fialka <mjf@mjf.cz>

client/space_lua/ast.ts

@@ -20,6 +20,8 @@ export type LuaBlock = {
   needsEnv?: boolean;
   // true if this block itself (not nested ones) defines at least one label
   hasLabelHere?: boolean;
+  // true if this block itself (not nested ones) may create to-be-closed values
+  hasCloseHere?: boolean;
 } & ASTContext;
 
 // STATEMENTS
@@ -141,6 +143,7 @@ export type LuaLocalStatement = {
 
 export enum LuaAttribute {
   Const = "const",
+  Close = "close",
 }
 
 export type LuaAttName = {

client/space_lua/const_attribute.test.ts

@@ -41,10 +41,10 @@ async function runAndCatch(code: string, ref = "const_attribute.lua") {
 Deno.test("const: Unknown attribute (parse-time)", () => {
   let threw = false;
   try {
-    parse(`local x<close> = 1`, { ref: "unknown_attribute.lua" });
+    parse(`local x<nope> = 1`, { ref: "unknown_attribute.lua" });
   } catch (e: any) {
     threw = true;
-    assertStringIncludes(String(e?.message ?? e), "unknown attribute 'close'");
+    assertStringIncludes(String(e?.message ?? e), "unknown attribute 'nope'");
   }
   if (!threw) {
     throw new Error("Expected parse error for unknown attribute");

client/space_lua/labels.ts

@@ -12,6 +12,7 @@ import type {
   LuaRepeatStatement,
   LuaWhileStatement,
 } from "./ast.ts";
+import { LuaAttribute } from "./ast.ts";
 import { asBlock } from "./ast_narrow.ts";
 
 type BlockGotoMeta = {
@@ -66,6 +67,7 @@ type ValidationCtx = {
   gotos: GotoInfo[];
   hasGoto: boolean;
   nextLocalId: number;
+  closeLocals: Set<LocalID>;
 };
 
 type BlockRole =
@@ -90,6 +92,7 @@ function resolveFunction(root: LuaBlock): FunctionMeta {
     gotos: [],
     hasGoto: false,
     nextLocalId: 1,
+    closeLocals: new Set<LocalID>(),
   };
 
   const seenBlocks = new Set<LuaBlock>();
@@ -101,7 +104,6 @@ function resolveFunction(root: LuaBlock): FunctionMeta {
     undefined,
     "Root",
     new Set<LocalID>(),
-    new Set<string>(),
     blockMeta,
     vctx,
     seenBlocks,
@@ -155,12 +157,25 @@ function resolveFunction(root: LuaBlock): FunctionMeta {
 
     // Local scope forward jump check
     let entersLocalScope = false;
+    let entersCloseScope = false;
+
     for (const id of lset) {
       if (!g.active.has(id)) {
         entersLocalScope = true;
-        break;
+        if (vctx.closeLocals.has(id)) {
+          entersCloseScope = true;
+          break;
+        }
       }
     }
+
+    if (entersCloseScope) {
+      throw new LabelResolveError(
+        `goto '${target}' jumps into the scope of a local variable`,
+        g.node.ctx,
+      );
+    }
+
     if (entersLocalScope) {
       const safeEnd = isSafeEndLabel(labelDefBlock, lloc.index, roleByBlock);
       if (!safeEnd) {
@@ -216,7 +231,6 @@ function processBlock(
   parent: LuaBlock | undefined,
   role: BlockRole,
   active: Set<LocalID>,
-  visibleLabels: Set<string>,
   blockMeta: WeakMap<LuaBlock, BlockGotoMeta>,
   vctx: ValidationCtx,
   seen: Set<LuaBlock>,
@@ -236,7 +250,6 @@ function processBlock(
   vctx.labelLocByBlock.set(block, labelLocMap);
 
   const curActive = cloneSet(active);
-  const vis = new Set<string>(visibleLabels);
 
   const stmts = block.statements;
   for (let i = 0; i < stmts.length; i++) {
@@ -254,7 +267,6 @@ function processBlock(
         const actSet = cloneSet(curActive);
         labelActiveMap.set(lab.name, actSet);
         labelLocMap.set(lab.name, { index: i, ctx: lab.ctx });
-        vis.add(lab.name);
         break;
       }
       case "Goto": {
@@ -266,7 +278,15 @@ function processBlock(
       case "Local": {
         const l = s as LuaLocalStatement;
         for (let j = 0; j < l.names.length; j++) {
-          curActive.add(vctx.nextLocalId++);
+          const id = vctx.nextLocalId++;
+          curActive.add(id);
+
+          const isClose =
+            l.names[j].attributes?.includes(LuaAttribute.Close) ===
+              true;
+          if (isClose) {
+            vctx.closeLocals.add(id);
+          }
         }
         break;
       }
@@ -286,7 +306,6 @@ function processBlock(
           block,
           "For",
           childActive,
-          new Set<string>(vis),
           blockMeta,
           vctx,
           seen,
@@ -306,7 +325,6 @@ function processBlock(
           block,
           "ForIn",
           childActive,
-          new Set<string>(vis),
           blockMeta,
           vctx,
           seen,
@@ -322,7 +340,6 @@ function processBlock(
           block,
           "While",
           cloneSet(curActive),
-          new Set<string>(vis),
           blockMeta,
           vctx,
           seen,
@@ -338,7 +355,6 @@ function processBlock(
           block,
           "Repeat",
           cloneSet(curActive),
-          new Set<string>(vis),
           blockMeta,
           vctx,
           seen,
@@ -355,7 +371,6 @@ function processBlock(
             block,
             "If",
             cloneSet(curActive),
-            new Set<string>(vis),
             blockMeta,
             vctx,
             seen,
@@ -369,7 +384,6 @@ function processBlock(
             block,
             "If",
             cloneSet(curActive),
-            new Set<string>(vis),
             blockMeta,
             vctx,
             seen,
@@ -386,7 +400,6 @@ function processBlock(
           block,
           "Do",
           cloneSet(curActive),
-          new Set<string>(vis),
           blockMeta,
           vctx,
           seen,

client/space_lua/parse.ts

@@ -24,6 +24,8 @@ import type {
   LuaTableField,
 } from "./ast.ts";
 import { LuaAttribute } from "./ast.ts";
+import { getBlockGotoMeta } from "./labels.ts";
+import { LuaRuntimeError, LuaStackFrame } from "./runtime.ts";
 
 const luaStyleTags = styleTags({
   Name: t.variableName,
@@ -69,6 +71,18 @@ function parseChunk(t: ParseTree, ctx: ASTCtx): LuaBlock {
   return parseBlock(t.children![0], ctx);
 }
 
+function hasCloseLocal(names: LuaAttName[] | undefined): boolean {
+  if (!names) {
+    return false;
+  }
+  for (const n of names) {
+    if (n.attributes?.includes(LuaAttribute.Close) === true) {
+      return true;
+    }
+  }
+  return false;
+}
+
 function parseBlock(t: ParseTree, ctx: ASTCtx): LuaBlock {
   if (t.type !== "Block") {
     throw new Error(`Expected Block, got ${t.type}`);
@@ -82,6 +96,7 @@ function parseBlock(t: ParseTree, ctx: ASTCtx): LuaBlock {
   let hasLocalDecl = false;
   let dup: { name: string; ctx: ASTCtx } | undefined;
   let hasLabelHere = false;
+  let hasCloseHere = false;
 
   const seen = new Set<string>();
 
@@ -105,7 +120,13 @@ function parseBlock(t: ParseTree, ctx: ASTCtx): LuaBlock {
         hasGoto = true;
         break;
       }
-      case "Local":
+      case "Local": {
+        hasLocalDecl = true;
+        if (!hasCloseHere) {
+          hasCloseHere = hasCloseLocal((s as any).names as LuaAttName[]);
+        }
+        break;
+      }
       case "LocalFunction": {
         hasLocalDecl = true;
         break;
@@ -114,6 +135,7 @@ function parseBlock(t: ParseTree, ctx: ASTCtx): LuaBlock {
         const child = s as LuaBlock;
         hasLabel = hasLabel || !!child.hasLabel;
         hasGoto = hasGoto || !!child.hasGoto;
+        hasCloseHere = hasCloseHere || !!child.hasCloseHere;
         break;
       }
       case "If": {
@@ -121,20 +143,29 @@ function parseBlock(t: ParseTree, ctx: ASTCtx): LuaBlock {
         for (const c of iff.conditions) {
           hasLabel = hasLabel || !!c.block.hasLabel;
           hasGoto = hasGoto || !!c.block.hasGoto;
+          hasCloseHere = hasCloseHere || !!c.block.hasCloseHere;
         }
         if (iff.elseBlock) {
           hasLabel = hasLabel || !!iff.elseBlock.hasLabel;
           hasGoto = hasGoto || !!iff.elseBlock.hasGoto;
+          hasCloseHere = hasCloseHere || !!iff.elseBlock.hasCloseHere;
         }
         break;
       }
       case "While":
       case "Repeat":
-      case "For":
+      case "For": {
+        const child = (s as any).block as LuaBlock;
+        hasLabel = hasLabel || !!child.hasLabel;
+        hasGoto = hasGoto || !!child.hasGoto;
+        hasCloseHere = hasCloseHere || !!child.hasCloseHere;
+        break;
+      }
       case "ForIn": {
         const child = (s as any).block as LuaBlock;
         hasLabel = hasLabel || !!child.hasLabel;
         hasGoto = hasGoto || !!child.hasGoto;
+        hasCloseHere = true;
         break;
       }
       default: {
@@ -158,6 +189,9 @@ function parseBlock(t: ParseTree, ctx: ASTCtx): LuaBlock {
   if (hasLabelHere) {
     block.hasLabelHere = true;
   }
+  if (hasCloseHere) {
+    block.hasCloseHere = true;
+  }
 
   return block;
 }
@@ -295,13 +329,26 @@ function parseStatement(t: ParseTree, ctx: ASTCtx): LuaStatement {
         expressions: parseExpList(t.children![2], ctx),
         ctx: context(t, ctx),
       };
-    case "Local":
+    case "Local": {
+      const names = parseAttNames(t.children![1], ctx);
+
+      let closeCount = 0;
+      for (const n of names) {
+        if (n.attributes?.includes(LuaAttribute.Close) === true) {
+          closeCount++;
+          if (closeCount > 1) {
+            throw new Error("multiple <close> variables in local list");
+          }
+        }
+      }
+
       return {
         type: "Local",
-        names: parseAttNames(t.children![1], ctx),
+        names,
         expressions: t.children![3] ? parseExpList(t.children![3], ctx) : [],
         ctx: context(t, ctx),
       };
+    }
     case "ReturnStatement": {
       const expressions = t.children![1]
         ? parseExpList(t.children![1], ctx)
@@ -366,14 +413,19 @@ function parseAttName(t: ParseTree, ctx: ASTCtx): LuaAttName {
   const attribute = t.children![1].children![1]
     ? t.children![1].children![1].children![0].text!
     : undefined;
-  if (attribute && attribute !== LuaAttribute.Const) {
+  if (
+    attribute &&
+    attribute !== LuaAttribute.Const &&
+    attribute !== LuaAttribute.Close
+  ) {
     throw new Error(`unknown attribute '${attribute}'`);
   }
+  const attributes = attribute ? [attribute as LuaAttribute] : undefined;
   return {
     type: "AttName",
     name: t.children![0].children![0].text!,
     attribute,
-    attributes: attribute ? [LuaAttribute.Const] : undefined,
+    attributes,
     ctx: context(t, ctx),
   };
 }
@@ -867,14 +919,27 @@ export function stripLuaComments(s: string): string {
 }
 
 export function parse(s: string, ctx: ASTCtx = {}): LuaBlock {
-  const t = parseToCrudeAST(stripLuaComments(s));
-  // console.log("Clean tree", JSON.stringify(t, null, 2));
-  const result = parseChunk(t, ctx);
-  // console.log("Parsed AST", JSON.stringify(result, null, 2));
-  return result;
+  try {
+    const t = parseToAST(stripLuaComments(s));
+    // console.log("Clean tree", JSON.stringify(t, null, 2));
+    const result = parseChunk(t, ctx);
+    // console.log("Parsed AST", JSON.stringify(result, null, 2));
+    getBlockGotoMeta(result);
+    return result;
+  } catch (e: any) {
+    if (e && typeof e === "object" && "astCtx" in e) {
+      throw new LuaRuntimeError(
+        e.message,
+        LuaStackFrame.lostFrame.withCtx(
+          (e as any).astCtx as ASTCtx,
+        ),
+      );
+    }
+    throw e;
+  }
 }
 
-export function parseToCrudeAST(t: string): ParseTree {
+export function parseToAST(t: string): ParseTree {
   const n = lezerToParseTree(t, parser.parse(t).topNode);
   return cleanTree(n, true);
 }

client/space_lua/rp.ts

@@ -4,7 +4,8 @@ export type RP<T> = T | Promise<T>;
 
 // Returns true when v is a Promise or a has a then function.
 export function isPromise<T>(v: RP<T>): v is Promise<T> {
-  return v !== null && typeof v === "object" &&
+  return v !== null &&
+    (typeof v === "object" || typeof v === "function") &&
     typeof (v as any).then === "function";
 }
 
@@ -17,22 +18,10 @@ export function rpThen<A, B>(v: RP<A>, f: (a: A) => RP<B>): RP<B> {
  * avoiding Promise allocation when all inputs are synchronous.
  */
 export function rpAll<T>(arr: RP<T>[]): RP<T[]> {
-  let hasPromise = false;
   for (let i = 0; i < arr.length; i++) {
     if (isPromise(arr[i])) {
-      hasPromise = true;
-      break;
+      return Promise.all(arr as Promise<T>[]);
     }
   }
-  if (!hasPromise) {
-    // All sync values: return as-is
-    return arr as T[];
-  }
-
-  // At least one Promise: allocate a new array
-  const out = new Array<T | Promise<T>>(arr.length);
-  for (let i = 0; i < arr.length; i++) {
-    out[i] = arr[i] as any;
-  }
-  return Promise.all(out as Promise<T>[]);
+  return arr as T[];
 }

client/space_lua/runtime.ts

@@ -24,11 +24,11 @@ export interface ILuaFunction {
 }
 
 export interface ILuaSettable {
-  set(key: LuaValue, value: LuaValue, sf?: LuaStackFrame): void;
+  set(key: LuaValue, value: LuaValue, sf?: LuaStackFrame): void | Promise<void>;
 }
 
 export interface ILuaGettable {
-  get(key: LuaValue, sf?: LuaStackFrame): LuaValue | undefined;
+  get(key: LuaValue, sf?: LuaStackFrame): LuaValue | Promise<LuaValue> | null;
 }
 
 // Small helpers for type safety/readability
@@ -57,6 +57,156 @@ export function ctxOrNull(sf?: LuaStackFrame): ASTCtx | null {
 // Reuse a single empty context to avoid allocating `{}` in hot paths
 const EMPTY_CTX = {} as ASTCtx;
 
+// Close-stack support
+export type LuaCloseEntry = { value: LuaValue; ctx: ASTCtx };
+
+type LuaThreadState = {
+  closeStack?: LuaCloseEntry[];
+};
+
+// Check whether a value is callable without invoking it.
+export function luaIsCallable(
+  v: LuaValue,
+  sf: LuaStackFrame,
+): boolean {
+  if (v === null || v === undefined) {
+    return false;
+  }
+  if (typeof v === "function") {
+    return true;
+  }
+  if (isILuaFunction(v)) {
+    return true;
+  }
+  if (v instanceof LuaTable) {
+    const mt = getMetatable(v, sf);
+    if (mt && mt.has("__call")) {
+      const mm = mt.get("__call", sf);
+      return !!mm && (typeof mm === "function" || isILuaFunction(mm));
+    }
+  }
+  return false;
+}
+
+// In Lua, `__close` must be a function (no `__call` fallback).
+function luaIsCloseMethod(
+  v: LuaValue,
+): boolean {
+  return typeof v === "function" || isILuaFunction(v);
+}
+
+export function luaEnsureCloseStack(sf: LuaStackFrame): LuaCloseEntry[] {
+  if (!sf.threadState.closeStack) {
+    sf.threadState.closeStack = [];
+  }
+  return sf.threadState.closeStack as LuaCloseEntry[];
+}
+
+export function luaMarkToBeClosed(
+  sf: LuaStackFrame,
+  value: LuaValue,
+  ctx: ASTCtx,
+): void {
+  const closeStack = luaEnsureCloseStack(sf);
+
+  // In Lua, `nil` is not closed.
+  if (value === null) {
+    return;
+  }
+
+  const mt = getMetatable(value, sf);
+  if (!mt || !mt.has("__close")) {
+    throw new LuaRuntimeError(
+      "variable got a non-closable value",
+      sf.withCtx(ctx),
+    );
+  }
+
+  const mm = mt.get("__close");
+  if (!luaIsCloseMethod(mm)) {
+    throw new LuaRuntimeError(
+      "variable got a non-closable value",
+      sf.withCtx(ctx),
+    );
+  }
+
+  closeStack.push({ value, ctx });
+}
+
+// Close entries from a mark (LIFO) and shrink stack back to mark.  This
+// is the core semantic for block exits and protected call boundaries.
+export function luaCloseFromMark(
+  sf: LuaStackFrame,
+  mark: number,
+  errObj: LuaValue | null,
+): Promise<void> | void {
+  const closeStack = sf.threadState?.closeStack as LuaCloseEntry[] | undefined;
+  if (!closeStack) {
+    return;
+  }
+  if (closeStack.length <= mark) {
+    return;
+  }
+
+  const callClose = (entry: LuaCloseEntry): LuaValue | Promise<LuaValue> => {
+    const mt = getMetatable(entry.value, sf);
+    const mm = mt ? mt.get("__close", sf) : null;
+    if (!luaIsCloseMethod(mm)) {
+      throw new LuaRuntimeError(
+        "metamethod '__close' is not callable",
+        sf.withCtx(entry.ctx),
+      );
+    }
+    if (errObj === null) {
+      return luaCall(mm, [entry.value], entry.ctx, sf);
+    }
+    return luaCall(mm, [entry.value, errObj], entry.ctx, sf);
+  };
+
+  // Close all to-be-closed variables (LIFO) even if one close errors.
+  // The reported error should be the first close error encountered.
+  const runFrom = (i: number): void | Promise<void> => {
+    let firstErr: unknown | null = null;
+
+    const recordErr = (e: unknown) => {
+      if (firstErr === null) {
+        firstErr = e;
+      }
+    };
+
+    const next = (idx: number): void | Promise<void> => {
+      for (let j = idx; j >= mark; j--) {
+        let r: LuaValue | Promise<LuaValue>;
+        try {
+          r = callClose(closeStack[j]);
+        } catch (e) {
+          recordErr(e);
+          continue;
+        }
+
+        if (isPromise(r)) {
+          return (r as Promise<any>).then(
+            () => next(j - 1),
+            (e: any) => {
+              recordErr(e);
+              return next(j - 1);
+            },
+          );
+        }
+      }
+
+      closeStack.length = mark;
+      if (firstErr !== null) {
+        throw firstErr;
+      }
+    };
+
+    return next(i);
+  };
+
+  return runFrom(closeStack.length - 1);
+}
+
 export class LuaEnv implements ILuaSettable, ILuaGettable {
   variables = new Map<string, LuaValue>();
   private readonly consts = new Set<string>();
@@ -99,13 +249,13 @@ export class LuaEnv implements ILuaSettable, ILuaGettable {
 
   get(
     name: string,
-    sf?: LuaStackFrame,
-  ): Promise<LuaValue> | LuaValue | undefined {
+    _sf?: LuaStackFrame,
+  ): Promise<LuaValue> | LuaValue | null {
     if (this.variables.has(name)) {
       return this.variables.get(name);
     }
     if (this.parent) {
-      return this.parent.get(name, sf);
+      return this.parent.get(name, _sf);
     }
     return null;
   }
@@ -134,13 +284,20 @@ export class LuaEnv implements ILuaSettable, ILuaGettable {
 }
 
 export class LuaStackFrame {
-  static lostFrame = new LuaStackFrame(new LuaEnv(), null);
+  // Must not share mutable per-thread state across calls/tests. This is
+  // a getter that returns a fresh frame each time.
+  static get lostFrame(): LuaStackFrame {
+    return new LuaStackFrame(new LuaEnv(), null, undefined, undefined, {
+      closeStack: undefined,
+    });
+  }
 
   constructor(
     readonly threadLocal: LuaEnv,
     readonly astCtx: ASTCtx | null,
     readonly parent?: LuaStackFrame,
     readonly currentFunction?: LuaFunction,
+    readonly threadState: LuaThreadState = { closeStack: undefined },
   ) {
   }
 
@@ -150,15 +307,29 @@ export class LuaStackFrame {
   ): LuaStackFrame {
     const env = new LuaEnv();
     env.setLocal("_GLOBAL", globalEnv);
-    return new LuaStackFrame(env, ctx);
+    return new LuaStackFrame(env, ctx, undefined, undefined, {
+      closeStack: undefined,
+    });
   }
 
   withCtx(ctx: ASTCtx): LuaStackFrame {
-    return new LuaStackFrame(this.threadLocal, ctx, this, this.currentFunction);
+    return new LuaStackFrame(
+      this.threadLocal,
+      ctx,
+      this,
+      this.currentFunction,
+      this.threadState,
+    );
   }
 
   withFunction(fn: LuaFunction): LuaStackFrame {
-    return new LuaStackFrame(this.threadLocal, this.astCtx, this.parent, fn);
+    return new LuaStackFrame(
+      this.threadLocal,
+      this.astCtx,
+      this.parent,
+      fn,
+      this.threadState,
+    );
   }
 }
 
@@ -240,28 +411,16 @@ export class LuaFunction implements ILuaFunction {
       env.setLocal("...", new LuaMultiRes(varargs));
 
       // Evaluate the function body with returnOnReturn set to true
-      try {
-        const r = evalStatement(this.body.block, env, sfWithFn, true);
-        const map = (val: any) => {
-          if (val !== undefined) {
-            return mapFunctionReturnValue(val);
-          }
-        };
-        if (isPromise(r)) {
-          return r.then(map).catch((e: any) => {
-            if (e instanceof LuaReturn) {
-              return mapFunctionReturnValue(e.values);
-            }
-            throw e;
-          });
-        } else {
-          return map(r);
+      const r = evalStatement(this.body.block, env, sfWithFn, true);
+      const map = (val: any) => {
+        if (val !== undefined) {
+          return mapFunctionReturnValue(val);
         }
-      } catch (e: any) {
-        if (e instanceof LuaReturn) {
-          return mapFunctionReturnValue(e.values);
-        }
-        throw e;
+      };
+      if (isPromise(r)) {
+        return r.then(map);
+      } else {
+        return map(r);
       }
     };
 
@@ -604,12 +763,27 @@ export function luaIndexValue(
   }
   // If not, let's see if the value has a metatable and if it has a __index metamethod
   const metatable = getMetatable(value, sf);
-  if (metatable && metatable.has("__index")) {
-    // Invoke the meta table
-    const metaValue = metatable.get("__index", sf);
-    if (isPromise(metaValue)) {
-      // Got a promise, we need to wait for it
-      return (metaValue as Promise<any>).then((mv: any) => {
+  if (metatable) {
+    const mm = metatable.rawGet("__index");
+    if (!(mm === undefined || mm === null)) {
+      // Invoke the meta table
+      const metaValue = mm;
+      if (isPromise(metaValue)) {
+        // Got a promise, we need to wait for it
+        return (metaValue as Promise<any>).then((mv: any) => {
+          if (mv?.call) {
+            return luaCall(mv, [value, key], sf?.astCtx ?? EMPTY_CTX, sf);
+          } else if (mv instanceof LuaTable) {
+            return mv.get(key, sf);
+          } else {
+            throw new LuaRuntimeError(
+              "Meta table __index must be a function or table",
+              sf || LuaStackFrame.lostFrame,
+            );
+          }
+        });
+      } else {
+        const mv = metaValue;
         if (mv?.call) {
           return luaCall(mv, [value, key], sf?.astCtx ?? EMPTY_CTX, sf);
         } else if (mv instanceof LuaTable) {
@@ -620,18 +794,6 @@ export function luaIndexValue(
             sf || LuaStackFrame.lostFrame,
           );
         }
-      });
-    } else {
-      const mv = metaValue as any;
-      if (mv?.call) {
-        return luaCall(mv, [value, key], sf?.astCtx ?? EMPTY_CTX, sf);
-      } else if (mv instanceof LuaTable) {
-        return mv.get(key, sf);
-      } else {
-        throw new LuaRuntimeError(
-          "Meta table __index must be a function or table",
-          sf || LuaStackFrame.lostFrame,
-        );
       }
     }
   }
@@ -1007,7 +1169,7 @@ export function luaToString(
 export function getMetatable(
   value: LuaValue,
   sf?: LuaStackFrame,
-): LuaValue | null {
+): LuaTable | null {
   if (value === null || value === undefined) {
     return null;
   }
@@ -1033,7 +1195,7 @@ export function getMetatable(
   }
 
   if ((value as any).metatable) {
-    return (value as any).metatable;
+    return (value as any).metatable as LuaTable;
   } else {
     return null;
   }

client/space_lua/stdlib.ts

@@ -1,14 +1,18 @@
 import {
   type ILuaFunction,
+  isILuaFunction,
   isLuaTable,
   LuaBuiltinFunction,
   luaCall,
+  luaCloseFromMark,
+  luaEnsureCloseStack,
   LuaEnv,
   luaGet,
   luaKeys,
   luaLen,
   LuaMultiRes,
   LuaRuntimeError,
+  type LuaStackFrame,
   type LuaTable,
   luaToString,
   luaTypeOf,
@@ -60,6 +64,18 @@ const ipairsFunction = new LuaBuiltinFunction((sf, ar: LuaTable | any[]) => {
 
 const pairsFunction = new LuaBuiltinFunction(
   (sf, t: LuaTable | any[] | Record<string, any>) => {
+    // Respect `__pairs` metamethod for Lua tables
+    if (isLuaTable(t)) {
+      const mt = (t as any).metatable as LuaTable | null | undefined;
+      if (mt) {
+        const mm = mt.get("__pairs", sf);
+        if (mm && (typeof mm === "function" || isILuaFunction(mm))) {
+          // __pairs must return (iter, state, control, closing)
+          return luaCall(mm, [t], sf.astCtx ?? {}, sf);
+        }
+      }
+    }
+
     let keys: (string | number)[];
     if (Array.isArray(t)) {
       keys = Array.from({ length: t.length }, (_, i) => i + 1); // For arrays, generate 1-based indices
@@ -74,7 +90,7 @@ const pairsFunction = new LuaBuiltinFunction(
     }
 
     let i = 0;
-    return async () => {
+    const iter = async () => {
       if (i >= keys.length) {
         return;
       }
@@ -83,6 +99,9 @@ const pairsFunction = new LuaBuiltinFunction(
       const value = await luaGet(t, key, sf.astCtx ?? null, sf);
       return new LuaMultiRes([key, value]);
     };
+
+    // Must return (iter, state, control) for generic for
+    return new LuaMultiRes([iter, t, null]);
   },
 );
 
@@ -129,30 +148,68 @@ const errorFunction = new LuaBuiltinFunction((sf, message: string) => {
   throw new LuaRuntimeError(message, sf);
 });
 
+async function pcallBoundary(
+  sf: LuaStackFrame,
+  fn: ILuaFunction,
+  args: LuaValue[],
+): Promise<
+  | { ok: true; values: LuaValue[] }
+  | { ok: false; message: string }
+> {
+  const closeStack = luaEnsureCloseStack(sf);
+  const mark = closeStack.length;
+
+  const errMsgOf = (e: any): string =>
+    e instanceof LuaRuntimeError ? e.message : (e?.message ?? String(e));
+
+  try {
+    const r = await luaCall(fn, args, sf.astCtx!, sf);
+    await luaCloseFromMark(sf, mark, null);
+    const values = r instanceof LuaMultiRes ? r.flatten().values : [r];
+    return { ok: true, values };
+  } catch (e: any) {
+    const msg = errMsgOf(e);
+    try {
+      await luaCloseFromMark(sf, mark, msg);
+      return { ok: false, message: msg };
+    } catch (closeErr: any) {
+      return { ok: false, message: errMsgOf(closeErr) };
+    }
+  }
+}
+
 const pcallFunction = new LuaBuiltinFunction(
   async (sf, fn: ILuaFunction, ...args) => {
-    try {
-      return new LuaMultiRes([true, await luaCall(fn, args, sf.astCtx!, sf)]);
-    } catch (e: any) {
-      if (e instanceof LuaRuntimeError) {
-        return new LuaMultiRes([false, e.message]);
-      }
-      return new LuaMultiRes([false, e.message]);
+    // To-be-closed variables must be closed when unwinding to the
+    // protected call boundary. Space Lua uses a per-thread close
+    // stack, so we snapshot its length and close anything pushed
+    // after that.
+    //
+    // The protected call boundary must be established *before*
+    // evaluating the function and its arguments.  Otherwise, any
+    // `<close>` locals created while evaluating `pcall`'s arguments
+    // will be wrongly treated as "inside" the protected call, and
+    // `pcall` may end up closing them (or affecting close ordering).
+    //
+    // `threadState` is read-only on the stack frame; do not reassign!
+    const res = await pcallBoundary(sf, fn, args);
+    if (res.ok) {
+      return new LuaMultiRes([true, ...res.values]);
     }
+    return new LuaMultiRes([false, res.message]);
   },
 );
 
 const xpcallFunction = new LuaBuiltinFunction(
   async (sf, fn: ILuaFunction, errorHandler: ILuaFunction, ...args) => {
-    try {
-      return new LuaMultiRes([true, await fn.call(sf, ...args)]);
-    } catch (e: any) {
-      const errorMsg = e instanceof LuaRuntimeError ? e.message : e.message;
-      return new LuaMultiRes([
-        false,
-        await luaCall(errorHandler, [errorMsg], sf.astCtx!, sf),
-      ]);
+    // Same semantic as `pcall` (see comments there)
+    const res = await pcallBoundary(sf, fn, args);
+    if (res.ok) {
+      return new LuaMultiRes([true, ...res.values]);
     }
+    const hr = await luaCall(errorHandler, [res.message], sf.astCtx!, sf);
+    const outVals = hr instanceof LuaMultiRes ? hr.flatten().values : [hr];
+    return new LuaMultiRes([false, ...outVals]);
   },
 );
 

client/space_lua/util.ts

@@ -1,9 +1,13 @@
+import { isPromise } from "./rp.ts";
+
 export function evalPromiseValues(vals: any[]): Promise<any[]> | any[] {
   const promises = [];
   const promiseResults = new Array(vals.length);
   for (let i = 0; i < vals.length; i++) {
-    if (vals[i] instanceof Promise) {
-      promises.push(vals[i].then((v: any) => promiseResults[i] = v));
+    if (isPromise(vals[i])) {
+      promises.push(
+        (vals[i] as Promise<any>).then((v: any) => promiseResults[i] = v),
+      );
     } else {
       promiseResults[i] = vals[i];
     }