Fixes #1420

Cleans up vararg handling, previously done really badly by AI...

Author: zefhemel

web/space_lua/eval.ts

@@ -38,39 +38,6 @@ import {
 import { luaValueToJS } from "../space_lua/runtime.ts";
 import { jsToLuaValue } from "../space_lua/runtime.ts";
 
-function handleVarargSync(env: LuaEnv): LuaValue[] | Promise<LuaValue[]> {
-  const varargs = env.get("...");
-  if (varargs instanceof Promise) {
-    return handleVarargAsync(varargs);
-  }
-  if (varargs instanceof LuaTable) {
-    const args = [];
-    for (let i = 1; i <= varargs.length; i++) {
-      const val = varargs.get(i);
-      if (val instanceof Promise) {
-        return handleVarargAsync(varargs);
-      }
-      args.push(val);
-    }
-    return args;
-  }
-  return [];
-}
-
-async function handleVarargAsync(
-  varargs: Promise<LuaValue> | LuaTable,
-): Promise<LuaValue[]> {
-  const resolvedVarargs = await varargs;
-  if (resolvedVarargs instanceof LuaTable) {
-    const args = [];
-    for (let i = 1; i <= resolvedVarargs.length; i++) {
-      args.push(await resolvedVarargs.get(i));
-    }
-    return args;
-  }
-  return [];
-}
-
 async function handleTableFieldSync(
   table: LuaTable,
   field: any,
@@ -91,18 +58,13 @@ async function handleTableFieldSync(
       break;
     }
     case "ExpressionField": {
-      if (field.value.type === "Variable" && field.value.name === "...") {
-        const varargs = await handleVarargSync(env);
-        varargs.forEach((val, i) => table.set(i + 1, val, sf));
-      } else {
-        const value = await evalExpression(field.value, env, sf);
-        if (value instanceof LuaMultiRes) {
-          for (const val of value.values) {
-            table.set(table.length + 1, val, sf);
-          }
-        } else {
-          table.set(table.length + 1, singleResult(value), sf);
+      const value = await evalExpression(field.value, env, sf);
+      if (value instanceof LuaMultiRes) {
+        for (const val of value.values) {
+          table.set(table.length + 1, val, sf);
         }
+      } else {
+        table.set(table.length + 1, singleResult(value), sf);
       }
       break;
     }
@@ -229,16 +191,6 @@ export function evalExpression(
       case "TableConstructor": {
         return Promise.resolve().then(async () => {
           const table = new LuaTable();
-          if (
-            e.fields.length === 1 &&
-            e.fields[0].type === "ExpressionField" &&
-            e.fields[0].value.type === "Variable" &&
-            e.fields[0].value.name === "..."
-          ) {
-            const varargs = await handleVarargSync(env);
-            varargs.forEach((val, i) => table.set(i + 1, val, sf));
-            return table;
-          }
 
           for (const field of e.fields) {
             await handleTableFieldSync(table, field, env, sf);
@@ -396,38 +348,6 @@ function evalPrefixExpression(
       const handleFunctionCall = (
         prefixValue: LuaValue,
       ): LuaValue | Promise<LuaValue> => {
-        // Special handling for f(...) - propagate varargs
-        if (
-          e.args.length === 1 && e.args[0].type === "Variable" &&
-          e.args[0].name === "..."
-        ) {
-          // TODO: Clean this up
-          const varargs = env.get("...");
-          const resolveVarargs = async () => {
-            const resolvedVarargs = await Promise.resolve(varargs);
-            if (resolvedVarargs instanceof LuaTable) {
-              const args = [];
-              for (let i = 1; i <= resolvedVarargs.length; i++) {
-                const val = await Promise.resolve(resolvedVarargs.get(i));
-                args.push(val);
-              }
-              return args;
-            }
-            return [];
-          };
-
-          if (prefixValue instanceof Promise) {
-            return prefixValue.then(async (resolvedPrefix) => {
-              const args = await resolveVarargs();
-              return luaCall(resolvedPrefix, args, e.ctx, sf.withCtx(e.ctx));
-            });
-          } else {
-            return resolveVarargs().then((args) =>
-              luaCall(prefixValue, args, e.ctx, sf.withCtx(e.ctx))
-            );
-          }
-        }
-
         // Normal argument handling for hello:there(a, b, c) type calls
         if (e.name) {
           selfArgs = [prefixValue];

web/space_lua/language_core_test.lua

@@ -559,4 +559,16 @@ local b, c = select(-2, 1, 2, 3)
 assertEqual(b, 2)
 assertEqual(c, 3)
 -- Special "#" case
-assertEqual(select("#", 1, 2, 3), 3)
+assertEqual(select("#", 1, 2, 3), 3)
+
+
+-- Some more vararg verification
+function varArgTest(a0, ...)
+    -- ... gets the multi arg treatment
+    local a1, a2 = ...
+    assertEqual(a0, 1)
+    assertEqual(a1, 2)
+    assertEqual(a2, 3)
+end
+
+varArgTest(1, 2, 3, 4)

web/space_lua/lua.test.ts

@@ -42,6 +42,10 @@ Deno.test("[Lua] Global functions tests", async () => {
   await runLuaTest("./stdlib/global_test.lua");
 });
 
+Deno.test("[Lua] Lua functions tests", async () => {
+  await runLuaTest("./lume_test.lua");
+});
+
 async function runLuaTest(luaPath: string) {
   const luaFile = await Deno.readTextFile(
     fileURLToPath(new URL(luaPath, import.meta.url)),

web/space_lua/lume_test.lua

@@ -0,0 +1,39 @@
+local function assertEqual(a, b, message)
+    if a ~= b then
+        error("Assertion failed: " .. a .. " is not equal to " .. b .. " " .. message)
+    end
+end
+
+lume = lume or {}
+
+local getiter = function(x)
+  if lume.isarray(x) then
+    return ipairs
+  elseif type(x) == "table" then
+    return pairs
+  end
+  error("expected table", 3)
+end
+
+function lume.isarray(x)
+  return type(x) == "table" and x[1] ~= nil
+end
+
+function lume.concat(...)
+  local rtn = {}
+  for i = 1, select("#", ...) do
+    local t = select(i, ...)
+    if t ~= nil then
+      local iter = getiter(t)
+      for _, v in iter(t) do
+        rtn[#rtn + 1] = v
+      end
+    end
+  end
+  return rtn
+end
+
+local r = lume.concat({1, 2}, {3, 4})
+assertEqual(#r, 4)
+assertEqual(r[1], 1)
+assertEqual(r[4], 4)

web/space_lua/runtime.ts

@@ -164,7 +164,7 @@ export class LuaFunction implements ILuaFunction {
     this.capturedEnv = closure;
   }
 
-  call(sf: LuaStackFrame, ...args: LuaValue[]): Promise<LuaValue> {
+  async call(sf: LuaStackFrame, ...args: LuaValue[]): Promise<LuaValue> {
     // Create a new environment that chains to the captured environment
     const env = new LuaEnv(this.capturedEnv);
     if (!sf) {
@@ -173,36 +173,25 @@ export class LuaFunction implements ILuaFunction {
     // Set _CTX to the thread local environment from the stack frame
     env.setLocal("_CTX", sf.threadLocal);
 
-    // Assign the passed arguments to the parameters
+    // Evaluate all arguments
+    const resolvedArgs = await evalPromiseValues(args);
+
+    // Assign parameter values to variable names in env
+    let varargs: LuaValue[] = [];
     for (let i = 0; i < this.body.parameters.length; i++) {
       const paramName = this.body.parameters[i];
       if (paramName === "...") {
-        // Handle varargs by creating a table with all remaining arguments
-        const varargs = new LuaTable();
-        // Include all remaining arguments (might be none)
-        for (let j = i; j < args.length; j++) {
-          varargs.set(j - i + 1, args[j], sf);
-        }
-        env.setLocal("...", varargs);
+        // Vararg parameter, let's collect the remainder of the resolved args into the varargs array
+        varargs = resolvedArgs.slice(i);
+        // Done, break out of this loop
         break;
       }
-      let arg = args[i];
-      if (arg === undefined) {
-        arg = null;
-      }
-      env.setLocal(this.body.parameters[i], arg);
+      env.setLocal(paramName, resolvedArgs[i] ?? null);
     }
 
-    // If the function has varargs parameter but it wasn't set above, set an empty varargs table
-    if (this.body.parameters.includes("...") && !env.has("...")) {
-      env.setLocal("...", new LuaTable());
-    }
+    env.setLocal("...", new LuaMultiRes(varargs));
 
-    const resolvedArgs = evalPromiseValues(args);
-    if (resolvedArgs instanceof Promise) {
-      return resolvedArgs.then((args) => this.callWithArgs(args, env, sf));
-    }
-    return this.callWithArgs(resolvedArgs, env, sf);
+    return this.evalBody(env, sf);
   }
 
   asString(): string {
@@ -213,41 +202,6 @@ export class LuaFunction implements ILuaFunction {
     return this.asString();
   }
 
-  private callWithArgs(
-    args: LuaValue[],
-    env: LuaEnv,
-    sf: LuaStackFrame,
-  ): Promise<LuaValue> {
-    // Set up parameters and varargs
-    for (let i = 0; i < this.body.parameters.length; i++) {
-      const paramName = this.body.parameters[i];
-      if (paramName === "...") {
-        const varargs = new LuaTable();
-        for (let j = i; j < args.length; j++) {
-          if (args[j] instanceof Promise) {
-            return Promise.all(args.slice(i)).then((resolvedArgs) => {
-              const varargs = new LuaTable();
-              resolvedArgs.forEach((val, idx) => varargs.set(idx + 1, val, sf));
-              env.setLocal("...", varargs);
-              return this.evalBody(env, sf);
-            });
-          }
-          varargs.set(j - i + 1, args[j], sf);
-        }
-        env.setLocal("...", varargs);
-        break;
-      }
-      env.setLocal(paramName, args[i] ?? null);
-    }
-
-    // Ensure empty varargs table exists if needed
-    if (this.body.parameters.includes("...") && !env.has("...")) {
-      env.setLocal("...", new LuaTable());
-    }
-
-    return this.evalBody(env, sf);
-  }
-
   private async evalBody(
     env: LuaEnv,
     sf: LuaStackFrame,