From 19618b96ca55e8c528ccc0fdb4ae57cb7836b05d Mon Sep 17 00:00:00 2001 From: Zef Hemel Date: Tue, 10 Jun 2025 11:00:26 +0200 Subject: [PATCH] Fixes #1420 Cleans up vararg handling, previously done really badly by AI... --- web/space_lua/eval.ts | 92 ++-------------------------- web/space_lua/language_core_test.lua | 14 ++++- web/space_lua/lua.test.ts | 4 ++ web/space_lua/lume_test.lua | 39 ++++++++++++ web/space_lua/runtime.ts | 70 ++++----------------- 5 files changed, 74 insertions(+), 145 deletions(-) create mode 100644 web/space_lua/lume_test.lua diff --git a/web/space_lua/eval.ts b/web/space_lua/eval.ts index 8f65e4cc..eb9a686d 100644 --- a/web/space_lua/eval.ts +++ b/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 { - 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 | LuaTable, -): Promise { - 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 => { - // 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]; diff --git a/web/space_lua/language_core_test.lua b/web/space_lua/language_core_test.lua index 979b886c..cea759f1 100644 --- a/web/space_lua/language_core_test.lua +++ b/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) \ No newline at end of file +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) \ No newline at end of file diff --git a/web/space_lua/lua.test.ts b/web/space_lua/lua.test.ts index 57e82cef..786f6c54 100644 --- a/web/space_lua/lua.test.ts +++ b/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)), diff --git a/web/space_lua/lume_test.lua b/web/space_lua/lume_test.lua new file mode 100644 index 00000000..ae4f0f97 --- /dev/null +++ b/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) \ No newline at end of file diff --git a/web/space_lua/runtime.ts b/web/space_lua/runtime.ts index d41602df..8d4dd2a8 100644 --- a/web/space_lua/runtime.ts +++ b/web/space_lua/runtime.ts @@ -164,7 +164,7 @@ export class LuaFunction implements ILuaFunction { this.capturedEnv = closure; } - call(sf: LuaStackFrame, ...args: LuaValue[]): Promise { + async call(sf: LuaStackFrame, ...args: LuaValue[]): Promise { // 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 { - // 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,