From e2dde9b8113e90884174f8ecee0037d2d38b5c5a Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 3 Sep 2026 20:34:59 +0200 Subject: [PATCH 1/5] Give vararg calls a fixed-arity copy on Lua On Lua a vararg function kept its `...` parameter and every call packed the arguments into a table, and the inliner refused vararg functions outright. `max`, `min` and `ArrayList.add` are vararg, so every relink in the spatial index allocated four tables and every element added to a list allocated one. Jass has always eliminated varargs before inlining by generating one copy per call arity. The same pass now runs on Lua at the same position, after stack traces and before lowering, with three differences the target needs. Method calls are handled as well as function calls. Classes still exist when this runs on Lua, so `list.add(x)` is an ImMethodCall; the backend already turns a call with exactly one possible implementation into a direct call, and the eliminator does the same for vararg methods. Without it ArrayList.add would never have been specialised. There is no Jass parameter cap. Instead a call with more than 64 vararg arguments keeps the original, which is always still present on Lua. Originals are kept. A vararg function may still be reached through a polymorphic method call, a function reference, or a call above the bound; unreferenced originals go with garbage removal. Measured on the stdlib probe with release flags: table.pack is gone from the output, max and min are max_2 and min_2, and ArrayList.add no longer exists as a function at all because the one-element copy inlines at every call site to a capacity check, one store and one increment. --- .../peeeq/wurstio/WurstCompilerJassImpl.java | 8 + .../imtranslation/VarargEliminator.java | 115 +++++++++++++-- .../tests/LuaBackendAuditTests.java | 137 +++++++++++++++++- 3 files changed, 249 insertions(+), 11 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java index 105d4520f..c37625a02 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java @@ -897,6 +897,14 @@ public LuaCompilationUnit transformProgToLua() { timeTaker.endPhase(); } } + // Same position as on Jass: after stack traces, before lowering and inlining. Calls with a + // static argument count go to fixed-arity copies, so the emitted Lua packs no table and the + // copies can inline; originals stay for dispatch, function references and calls above the bound. + beginPhase(4, "eliminate varargs"); + new VarargEliminator(imProg, true).run(); + imTranslator.assertProperties(); + timeTaker.endPhase(); + ImTranslator imTranslator2 = getImTranslator(); ImOptimizer optimizer = new ImOptimizer(timeTaker, imTranslator2); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java index 36b3fcffd..61a802dc4 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java @@ -21,30 +21,119 @@ public class VarargEliminator { private static final int JASS_MAX_PARAMETERS = ImHelper.JASS_MAX_PARAMETERS; + /** + * Largest vararg arity given a fixed-arity copy on Lua. Lua caps a function at 200 locals + * including parameters; this leaves room for the body's own locals. A call above it keeps the + * original `...` function, which is always still present on that target. + */ + public static final int LUA_MAX_SPECIALISED_VARARG_ARITY = 64; private final ImProg prog; + /** + * On Lua classes are still present when this runs, so a vararg function can also be reached + * through a method dispatch or a function reference. Originals are therefore kept, only direct + * calls are redirected, and unreferenced originals are left to garbage removal. + */ + private final boolean luaTarget; // original + number of args --> new function private final Table varargFuncs = HashBasedTable.create(); public VarargEliminator(ImProg prog) { + this(prog, false); + } + + public VarargEliminator(ImProg prog, boolean luaTarget) { this.prog = prog; + this.luaTarget = luaTarget; } public void run() { // create new vararg functions for (ImFunctionCall c : collectVarargCalls()) { - if (c.getFunc().hasFlag(IS_VARARG)) { + if (c.getFunc().hasFlag(IS_VARARG) && shouldSpecialise(c)) { generateVarargFunc(c); } } + if (luaTarget) { + // The Lua backend already turns a method call with exactly one possible implementation + // into a direct call of that implementation. Doing the same here for vararg methods is + // what lets ArrayList.add and friends get a fixed-arity copy at all: on this target the + // call is still an ImMethodCall when varargs are eliminated. + for (ImMethodCall c : collectMonomorphicVarargMethodCalls()) { + ImFunction implementation = c.getMethod().getImplementation(); + List arguments = receiverAndArguments(c); + if (shouldSpecialise(implementation, arguments.size())) { + generateVarargFunc(implementation, arguments, c); + } + } + } - // remove original vararg functions: - prog.getFunctions().removeIf(f -> f.hasFlag(IS_VARARG)); + if (!luaTarget) { + // remove original vararg functions: + prog.getFunctions().removeIf(f -> f.hasFlag(IS_VARARG)); + } // rewrite calls to use new functions: // (need to collect vararg calls again, because first phase can create copies of calls) for (ImFunctionCall call : collectVarargCalls()) { - redirectCall(call, varargFuncs.get(call.getFunc(), call.getArguments().size())); + ImFunction newFunc = varargFuncs.get(call.getFunc(), call.getArguments().size()); + if (newFunc != null) { + redirectCall(call, newFunc); + } + } + if (luaTarget) { + for (ImMethodCall call : collectMonomorphicVarargMethodCalls()) { + ImFunction implementation = call.getMethod().getImplementation(); + ImFunction newFunc = varargFuncs.get(implementation, 1 + call.getArguments().size()); + if (newFunc != null) { + redirectMethodCall(call, newFunc); + } + } + } + } + + /** A method call which can only ever reach one implementation, and that implementation is vararg. */ + private Collection collectMonomorphicVarargMethodCalls() { + final Collection calls = new ArrayList<>(); + prog.accept(new ImProg.DefaultVisitor() { + @Override + public void visit(ImMethodCall c) { + super.visit(c); + ImMethod method = c.getMethod(); + if (method != null && !method.getIsAbstract() && method.getImplementation() != null + && method.getSubMethods().isEmpty() && method.getImplementation().hasFlag(IS_VARARG)) { + calls.add(c); + } + } + }); + return calls; + } + + /** The implementation's argument list: the receiver is its first parameter. */ + private static List receiverAndArguments(ImMethodCall call) { + List arguments = new ArrayList<>(1 + call.getArguments().size()); + arguments.add(call.getReceiver()); + arguments.addAll(call.getArguments()); + return arguments; + } + + private void redirectMethodCall(ImMethodCall call, ImFunction newFunc) { + ImExprs args = JassIm.ImExprs(call.getReceiver().copy()); + args.addAll(call.getArguments().removeAll()); + call.replaceBy(JassIm.ImFunctionCall(call.getTrace(), newFunc, JassIm.ImTypeArguments(), args, + call.getTuplesEliminated(), CallType.NORMAL)); + } + + /** Whether a call gets a fixed-arity copy. Always on Jass; on Lua only up to the arity bound. */ + private boolean shouldSpecialise(ImFunctionCall call) { + return shouldSpecialise(call.getFunc(), call.getArguments().size()); + } + + private boolean shouldSpecialise(ImFunction func, int totalArguments) { + if (!luaTarget) { + return true; } + int varargCount = 1 + totalArguments - func.getParameters().size(); + return varargCount <= LUA_MAX_SPECIALISED_VARARG_ARITY; } @NotNull @@ -70,13 +159,17 @@ public void visit(ImFunctionCall c) { * for the function call. */ private void generateVarargFunc(ImFunctionCall sourceCall) { - ImFunction func = sourceCall.getFunc(); - int numberOfParams = sourceCall.getArguments().size(); - int jassParameterCount = sourceCall.getArguments().stream() + generateVarargFunc(sourceCall.getFunc(), sourceCall.getArguments(), sourceCall); + } + + /** {@code arguments} are in the callee's parameter order, so for a method they start with the receiver. */ + private void generateVarargFunc(ImFunction func, List arguments, Element trace) { + int numberOfParams = arguments.size(); + int jassParameterCount = arguments.stream() .mapToInt(argument -> ImHelper.flattenedJassArity(argument.attrTyp())) .sum(); - if (jassParameterCount > JASS_MAX_PARAMETERS) { - throw new CompileError(sourceCall, "Vararg call would generate " + jassParameterCount + if (!luaTarget && jassParameterCount > JASS_MAX_PARAMETERS) { + throw new CompileError(trace, "Vararg call would generate " + jassParameterCount + " Jass parameters; the maximum is " + JASS_MAX_PARAMETERS + ". Use multiple calls (for example with the cascade operator) or pass a collection instead."); } @@ -131,7 +224,9 @@ public void visit(ImVarargLoop imLoop) { params.addAll(list); // generate function for this new call - generateVarargFunc(call); + if (shouldSpecialise(call)) { + generateVarargFunc(call); + } } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 00fbd7f68..b557eb4dd 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -271,7 +271,7 @@ public void optimizedTupleVarargLoopUsesAttachedScalarLocals() { " let bag = new Bag()", " bag.add(handles(makeFrame(), makeFrame()))" ); - assertTrue(compiled.contains("table.pack(...)")); + assertFalse("a static-arity vararg call must not pack a table on Lua", compiled.contains("table.pack(...)")); assertFalse(compiled.contains("tupleCopy")); } @@ -1988,6 +1988,141 @@ public void localPlayerEffectfulBooleanOperandSurvivesOptimization() { compiled.indexOf("localProbe()", definitionOrCall + 1) >= 0); } + /** + * On Lua a vararg function used to keep its {@code ...} parameter and pack it into a table on + * every call, and the inliner refused it. With a static argument count at the call site the + * call is redirected to a fixed-arity copy, as Jass has always done, so the pack is gone and + * the copy inlines like any other small function. + */ + @Test + public void staticArityVarargCallsAreFixedArityOnLua() { + String compiled = compileOptimizedLua( + "staticArityVarargCallsAreFixedArityOnLua", + "package Test", + "native consume(int i)", + "int array values", + "function biggest(vararg int xs) returns int", + " var best = -2147483648", + " for x in xs", + " if x > best", + " best = x", + " return best", + "@noinline function query(int a, int b, int c)", + " var i = 0", + " while i < 16", + " consume(biggest(a, values[i]))", + " consume(biggest(a, b, c))", + " i++", + "init", + " query(1, 2, 3)" + ); + assertFalse("no vararg call site may pack a table:\n" + compiled, compiled.contains("table.pack")); + assertFalse("the vararg original must not survive with a ... parameter:\n" + compiled, + compiled.contains("function biggest(...)")); + assertFunctionBodyContains(compiled, "query", "biggest(", false); + } + + @Test + public void fixedArityVarargLoweringKeepsSemanticsOnLua() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "tuple pair(int x, int y)", + "function sum(vararg int xs) returns int", + " var total = 0", + " for x in xs", + " total += x", + " return total", + "function count(vararg int xs) returns int", + " var n = 0", + " for x in xs", + " n++", + " return n", + "function firstOr(vararg int xs) returns int", + " for x in xs", + " return x", + " return -1", + "function pairs(vararg pair ps) returns int", + " var result = 0", + " for p in ps", + " result = result * 100 + p.x * 10 + p.y", + " return result", + "class Bag", + " int total = 0", + " function add(vararg int xs)", + " for x in xs", + " total += x", + "init", + " let bag = new Bag()", + " bag.add(1)", + " bag.add(2, 3)", + " bag.add()", + " if sum() == 0 and sum(5) == 5 and sum(1, 2, 3, 4) == 10", + " and count() == 0 and count(9, 9, 9) == 3", + " and firstOr() == -1 and firstOr(4, 5) == 4", + " and pairs(pair(1, 2), pair(3, 4)) == 1234", + " and bag.total == 6", + " testSuccess()" + ); + String compiled = compiledLua("fixedArityVarargLoweringKeepsSemanticsOnLua"); + assertFalse("every call above has a static arity, so nothing may pack:\n" + compiled, + compiled.contains("table.pack")); + } + + @Test + public void varargCallAboveTheLuaArityBoundKeepsThePackedPath() throws IOException { + StringBuilder args = new StringBuilder(); + int n = 150; + for (int i = 1; i <= n; i++) { + if (i > 1) { + args.append(", "); + } + args.append(i); + } + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "function sum(vararg int xs) returns int", + " var total = 0", + " for x in xs", + " total += x", + " return total", + "init", + " if sum(" + args + ") == " + (n * (n + 1) / 2) + " and sum(1, 2) == 3", + " testSuccess()" + ); + String compiled = compiledLua("varargCallAboveTheLuaArityBoundKeepsThePackedPath"); + assertTrue("a call above the bound keeps the vararg original:\n" + compiled, + compiled.contains("table.pack")); + } + + @Test + public void virtuallyDispatchedVarargMethodKeepsThePackedPath() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "interface Summer", + " function sum(vararg int xs) returns int", + "class Plain implements Summer", + " override function sum(vararg int xs) returns int", + " var total = 0", + " for x in xs", + " total += x", + " return total", + "class Doubling implements Summer", + " override function sum(vararg int xs) returns int", + " var total = 0", + " for x in xs", + " total += 2 * x", + " return total", + "init", + " Summer a = new Plain()", + " Summer b = new Doubling()", + " if a.sum(1, 2, 3) == 6 and b.sum(1, 2, 3) == 12", + " testSuccess()" + ); + } + @Test public void localPlayerTaintFlowsThroughVarargLoopValues() { String compiled = compileOptimizedLua( From 396c21d2a33329c73cfa0a79a71b9676219bcb55 Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 3 Sep 2026 21:32:05 +0200 Subject: [PATCH 2/5] Map recursive vararg calls to their own arity, and bound by flattened parameters Two defects in the Lua vararg elimination, both found in review. A copy retargets the function's own references, so a recursive call inside the two-argument copy named that copy. Whenever the recursion uses a different argument count that is wrong: a call of f(7) inside the two-parameter clone invoked itself with a parameter missing. Self-calls now go back to naming the vararg original, and copies are generated to a fixpoint, so a recursive call at an arity nothing else needed still gets a copy of its own. The Lua bound counted source arguments, but the emitted parameter list costs flattened ones: twenty four-field tuples are eighty parameters, past what the target accepts and past what the locals-table fallback can spill, since that cannot spill formal parameters. The bound is now counted after tuple flattening and the constant says parameters rather than arity. A third report, a vararg parameter forwarded into a method call, is not reachable: a vararg function may have only the one parameter, so a receiver cannot be a second one, and the argument does not type as the element type. Both halves are pinned by a test, since the forwarding branch is otherwise only reached from calls this pass generated itself. --- .../imtranslation/VarargEliminator.java | 84 +++++++++++++------ .../tests/LuaBackendAuditTests.java | 78 +++++++++++++++++ 2 files changed, 138 insertions(+), 24 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java index 61a802dc4..c4771c924 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java @@ -22,11 +22,14 @@ public class VarargEliminator { private static final int JASS_MAX_PARAMETERS = ImHelper.JASS_MAX_PARAMETERS; /** - * Largest vararg arity given a fixed-arity copy on Lua. Lua caps a function at 200 locals - * including parameters; this leaves room for the body's own locals. A call above it keeps the - * original `...` function, which is always still present on that target. + * Largest number of emitted parameters a fixed-arity copy may have on Lua, counted after tuple + * flattening: one four-field tuple argument is four parameters, so an arity that looks modest in + * source can exceed what the target accepts. Lua caps a function at 200 locals including its + * parameters, and the locals-table fallback cannot spill parameters, so this leaves room for the + * body's own locals. A call above it keeps the original `...` function, which is always still + * present on that target. */ - public static final int LUA_MAX_SPECIALISED_VARARG_ARITY = 64; + public static final int LUA_MAX_SPECIALISED_VARARG_PARAMETERS = 64; private final ImProg prog; /** * On Lua classes are still present when this runs, so a vararg function can also be reached @@ -47,22 +50,32 @@ public VarargEliminator(ImProg prog, boolean luaTarget) { } public void run() { - // create new vararg functions - for (ImFunctionCall c : collectVarargCalls()) { - if (c.getFunc().hasFlag(IS_VARARG) && shouldSpecialise(c)) { - generateVarargFunc(c); + // Create new vararg functions. Repeated to a fixpoint: a generated copy can contain a call + // to a vararg function at an arity nothing has needed yet, which is what a recursive vararg + // function calling itself with a different argument count produces. + boolean generated = true; + while (generated) { + generated = false; + for (ImFunctionCall c : collectVarargCalls()) { + if (c.getFunc().hasFlag(IS_VARARG) && shouldSpecialise(c) + && !varargFuncs.contains(c.getFunc(), c.getArguments().size())) { + generateVarargFunc(c); + generated = true; + } } - } - if (luaTarget) { - // The Lua backend already turns a method call with exactly one possible implementation - // into a direct call of that implementation. Doing the same here for vararg methods is - // what lets ArrayList.add and friends get a fixed-arity copy at all: on this target the - // call is still an ImMethodCall when varargs are eliminated. - for (ImMethodCall c : collectMonomorphicVarargMethodCalls()) { - ImFunction implementation = c.getMethod().getImplementation(); - List arguments = receiverAndArguments(c); - if (shouldSpecialise(implementation, arguments.size())) { - generateVarargFunc(implementation, arguments, c); + if (luaTarget) { + // The Lua backend already turns a method call with exactly one possible + // implementation into a direct call of that implementation. Doing the same here for + // vararg methods is what lets ArrayList.add and friends get a fixed-arity copy at + // all: on this target the call is still an ImMethodCall when varargs are eliminated. + for (ImMethodCall c : collectMonomorphicVarargMethodCalls()) { + ImFunction implementation = c.getMethod().getImplementation(); + List arguments = receiverAndArguments(c); + if (shouldSpecialise(arguments) + && !varargFuncs.contains(implementation, arguments.size())) { + generateVarargFunc(implementation, arguments, c); + generated = true; + } } } } @@ -123,17 +136,27 @@ private void redirectMethodCall(ImMethodCall call, ImFunction newFunc) { call.getTuplesEliminated(), CallType.NORMAL)); } - /** Whether a call gets a fixed-arity copy. Always on Jass; on Lua only up to the arity bound. */ + /** Whether a call gets a fixed-arity copy. Always on Jass; on Lua only within the parameter bound. */ private boolean shouldSpecialise(ImFunctionCall call) { - return shouldSpecialise(call.getFunc(), call.getArguments().size()); + return shouldSpecialise(call.getArguments()); } - private boolean shouldSpecialise(ImFunction func, int totalArguments) { + /** + * Counted after tuple flattening, because that is what the emitted parameter list costs: twenty + * four-field tuples are eighty parameters, not twenty. + */ + private boolean shouldSpecialise(List arguments) { if (!luaTarget) { return true; } - int varargCount = 1 + totalArguments - func.getParameters().size(); - return varargCount <= LUA_MAX_SPECIALISED_VARARG_ARITY; + int parameters = 0; + for (ImExpr argument : arguments) { + parameters += ImHelper.flattenedJassArity(argument.attrTyp()); + if (parameters > LUA_MAX_SPECIALISED_VARARG_PARAMETERS) { + return false; + } + } + return true; } @NotNull @@ -184,6 +207,19 @@ private void generateVarargFunc(ImFunction func, List arguments, Element // Create new function ImFunction newFunc = ReferenceRewritingCopy.copy(func); + // The copy retargets the function's own references, so a recursive call inside it now names + // the copy. That is wrong whenever the recursion uses a different argument count: the call + // must go back to naming the vararg original, so the rewrite below maps it to a copy of its + // own arity like any other call. + newFunc.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImFunctionCall call) { + super.visit(call); + if (call.getFunc() == newFunc) { + call.setFunc(func); + } + } + }); newFunc.setName(func.getName() + "_" + argumentSize); // replace vararg with special parameters: ImVar varargParam = newFunc.getParameters().remove(newFunc.getParameters().size() - 1); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index b557eb4dd..c39291b34 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -2123,6 +2123,84 @@ public void virtuallyDispatchedVarargMethodKeepsThePackedPath() throws IOExcepti ); } + /** + * A vararg function may call itself with a different static arity. Copying the function for one + * arity must not retarget that inner call to the copy, which has the wrong parameter count; the + * call has to be mapped to its own arity like every other call. + */ + @Test + public void recursiveVarargCallsAreMappedToTheirOwnArity() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "function depth(vararg int xs) returns int", + " var n = 0", + " for x in xs", + " n++", + " if n == 3", + " return 100 + depth(1, 2)", + " if n == 2", + " return 10 + depth(1)", + " return n", + "init", + " if depth(1, 2, 3) == 111 and depth(5) == 1 and depth() == 0", + " testSuccess()" + ); + } + + /** + * The Lua arity bound is about emitted parameters, which tuple elimination multiplies: twenty + * four-field tuples are eighty formal parameters. Such a call keeps the packed path rather than + * emitting a function Lua refuses to load. + */ + @Test + public void wideTupleVarargCallCountsFlattenedParametersAgainstTheLuaBound() throws IOException { + StringBuilder args = new StringBuilder(); + int n = 20; + for (int i = 1; i <= n; i++) { + if (i > 1) { + args.append(", "); + } + args.append("quad(").append(i).append(", 0, 0, 1)"); + } + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "tuple quad(int a, int b, int c, int d)", + "@noinline function sumFirst(vararg quad qs) returns int", + " var total = 0", + " for q in qs", + " total += q.a + q.d", + " return total", + "init", + " if sumFirst(" + args + ") == " + (n * (n + 1) / 2 + n) + " and sumFirst(quad(1, 2, 3, 4)) == 5", + " testSuccess()" + ); + String compiled = compiledLua("wideTupleVarargCallCountsFlattenedParametersAgainstTheLuaBound"); + assertTrue("eighty flattened parameters must keep the packed original:\n" + compiled, + compiled.contains("table.pack")); + } + + /** + * A vararg parameter cannot be passed on, to a function or to a method. Both halves matter here: + * a vararg function may have only the one parameter, so a receiver cannot be a second one, and + * the argument itself does not type as the element type. The eliminator's forwarding branch is + * therefore reachable only from calls it generated itself, which are always ImFunctionCall. + */ + @Test + public void varargParameterCannotBeForwardedToAMethod() { + testAssertErrorsLines(false, "Found vararg integer", + "package Test", + "class Sink", + " static Sink instance = null", + " function consume(vararg int xs)", + " skip", + "function relay(vararg int xs)", + " Sink.instance.consume(xs)" + ); + } + + @Test public void localPlayerTaintFlowsThroughVarargLoopValues() { String compiled = compileOptimizedLua( From 99f5daee5e8daf6a04be3d2c7efc53149e3433cb Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 3 Sep 2026 22:22:13 +0200 Subject: [PATCH 3/5] Keep a preserved name and a self reference on the vararg original Two ways a generated copy could take over something that belongs only to the original it was copied from. A preserved name is part of the map's Warcraft-facing API, set by @preserveName and by ExecuteFunc. The copy inherits the flag, and because it also shares the original's trace, collectPredefinedNames resets both to the same source name: the emitted Lua then defines that name twice and the later definition wins. ExecuteFunc makes this easy to reach, since it marks its target preserved and emits a zero-argument call, which is exactly what makes a copy. The flag is now dropped from copies on Lua, where the original is retained and keeps the name that external code calls. The other is the reference case of the recursion fix. ReferenceRewritingCopy retargets a function's own references, and the repair visitor undid that for calls but not for function references, so a self reference inside a copy kept naming the copy: registering it as a callback would invoke a fixed-arity body at an arity nobody checked. Those two node types are the only ones the copy retargets that name a function at all, so the visitor now covers the pair rather than the reported half. Lua only, because nothing redirects a reference afterwards and only that target keeps the original; on Jass it is removed and the reference would dangle. Jass keeps both flags and both reference kinds exactly as before. --- .../imtranslation/VarargEliminator.java | 32 ++++++++++++++---- .../tests/LuaBackendAuditTests.java | 33 +++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java index c4771c924..a1bdd25f4 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java @@ -13,6 +13,7 @@ import java.util.stream.Collectors; import static de.peeeq.wurstscript.translation.imtranslation.FunctionFlagEnum.IS_VARARG; +import static de.peeeq.wurstscript.translation.imtranslation.FunctionFlagEnum.PRESERVE_NAME; /** * Takes a program and eliminates vararg functions, replacing them with @@ -207,10 +208,11 @@ private void generateVarargFunc(ImFunction func, List arguments, Element // Create new function ImFunction newFunc = ReferenceRewritingCopy.copy(func); - // The copy retargets the function's own references, so a recursive call inside it now names - // the copy. That is wrong whenever the recursion uses a different argument count: the call - // must go back to naming the vararg original, so the rewrite below maps it to a copy of its - // own arity like any other call. + // ReferenceRewritingCopy retargets the function's own references - both call and reference + // nodes - so inside the copy they now name the copy. That is wrong for either kind. A + // recursive call must go back to naming the vararg original, so the rewrite below maps it to + // a copy of its own arity like any other call; a self reference must name the original too, + // because it is invoked at an arity this pass never sees. newFunc.accept(new Element.DefaultVisitor() { @Override public void visit(ImFunctionCall call) { @@ -219,6 +221,17 @@ public void visit(ImFunctionCall call) { call.setFunc(func); } } + + @Override + public void visit(ImFuncRef ref) { + super.visit(ref); + // Lua only: nothing redirects a reference afterwards, so it keeps naming whatever it + // is set to here, and only this target retains the original. On Jass the original is + // removed below and pointing at it would leave the reference dangling. + if (luaTarget && ref.getFunc() == newFunc) { + ref.setFunc(func); + } + } }); newFunc.setName(func.getName() + "_" + argumentSize); // replace vararg with special parameters: @@ -266,12 +279,17 @@ public void visit(ImVarargLoop imLoop) { } - // Remove vararg flag + // Drop the vararg flag, and on Lua the name preservation with it. A preserved name is part + // of the map's Warcraft-facing API and belongs to the retained original, which is what + // external code calls at an arity this pass never sees. Since a copy shares the original's + // trace, and LuaTranslator.collectPredefinedNames() resets every preserved function to its + // trace's source name, an inherited flag would emit both under one name. List list = new ArrayList<>(); for (FunctionFlag flag : newFunc.getFlags()) { - if (flag != IS_VARARG) { - list.add(flag); + if (flag == IS_VARARG || (luaTarget && flag == PRESERVE_NAME)) { + continue; } + list.add(flag); } newFunc.setFlags(list); // Add new function to prog diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index cc5c73762..ce2c95050 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -2201,6 +2201,39 @@ public void varargParameterCannotBeForwardedToAMethod() { } + /** + * `@preserveName` and `ExecuteFunc` mark a function's emitted name as part of the map's + * WC3-facing API, and `LuaTranslator.collectPredefinedNames()` resets every function carrying + * that flag to its trace's source name. A generated copy shares the original's trace, so an + * inherited flag would emit the original and every copy under one name and let the last + * definition win. The preserved name belongs to the retained original: that is the one external + * code calls, at an arity this pass never gets to see. + */ + @Test + public void preservedNameStaysOnTheVarargOriginalNotItsCopies() { + String compiled = compileOptimizedLua( + "preservedNameStaysOnTheVarargOriginalNotItsCopies", + "package Test", + "native consume(int i)", + "@preserveName @noinline public function tally(vararg int xs) returns int", + " var sum = 0", + " for x in xs", + " sum += x", + " return sum", + "init", + " consume(tally(1, 2))" + ); + assertTrue("the fixed-arity copy must keep its own suffixed name:\n" + compiled, + compiled.contains("function tally_2(")); + int definitions = 0; + for (int at = compiled.indexOf("function tally("); at >= 0; + at = compiled.indexOf("function tally(", at + 1)) { + definitions++; + } + assertEquals("the preserved name must name exactly one function:\n" + compiled, + 1, definitions); + } + /** * The inliner used to refuse every function whose return fact the local-player analysis had * marked, and that fact fires for anything reachable from a client-local branch anywhere in the From 2dbba0c17e371ba18562e39a7cff6aa3612b0118 Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 3 Sep 2026 22:38:55 +0200 Subject: [PATCH 4/5] Leave a forwarded vararg placeholder on the packed path A vararg constructor is reached through a generated new_C wrapper that passes its own vararg parameter to construct_C. When a call above the bound keeps that wrapper as the retained original, its body still holds the forwarding call, and the placeholder is one node standing for however many arguments the caller passed. Counting nodes turned it into an arity, so construct_C_1 was generated and the call rewritten to it: every argument after the first was dropped and the constructor ran on one value. Node count is only an arity when no node is a placeholder, so calls that forward one are now left alone. This is Lua-only in effect. The forwarding call survives in the body of a vararg original, and a copy has its placeholder expanded into real parameters long before anything counts them again, so only originals match - which Jass removes and Lua retains. Both the generation and the rewrite loop consult the same predicate. Skipping generation alone would not have been enough: another call can produce a copy at the same node count, and the rewrite would then redirect the forwarding call to it regardless. The existing above-the-bound test used a plain function, which has no wrapper and so never forwards. The new test uses a constructor and fails on the constructed value before this change. --- .../imtranslation/VarargEliminator.java | 43 +++++++++++++++++-- .../tests/LuaBackendAuditTests.java | 33 ++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java index a1bdd25f4..9731c2368 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java @@ -58,7 +58,7 @@ public void run() { while (generated) { generated = false; for (ImFunctionCall c : collectVarargCalls()) { - if (c.getFunc().hasFlag(IS_VARARG) && shouldSpecialise(c) + if (c.getFunc().hasFlag(IS_VARARG) && shouldSpecialise(c) && !forwardsAVarargParameter(c.getArguments()) && !varargFuncs.contains(c.getFunc(), c.getArguments().size())) { generateVarargFunc(c); generated = true; @@ -72,7 +72,7 @@ public void run() { for (ImMethodCall c : collectMonomorphicVarargMethodCalls()) { ImFunction implementation = c.getMethod().getImplementation(); List arguments = receiverAndArguments(c); - if (shouldSpecialise(arguments) + if (shouldSpecialise(arguments) && !forwardsAVarargParameter(arguments) && !varargFuncs.contains(implementation, arguments.size())) { generateVarargFunc(implementation, arguments, c); generated = true; @@ -90,7 +90,7 @@ public void run() { // (need to collect vararg calls again, because first phase can create copies of calls) for (ImFunctionCall call : collectVarargCalls()) { ImFunction newFunc = varargFuncs.get(call.getFunc(), call.getArguments().size()); - if (newFunc != null) { + if (newFunc != null && !forwardsAVarargParameter(call.getArguments())) { redirectCall(call, newFunc); } } @@ -98,13 +98,48 @@ public void run() { for (ImMethodCall call : collectMonomorphicVarargMethodCalls()) { ImFunction implementation = call.getMethod().getImplementation(); ImFunction newFunc = varargFuncs.get(implementation, 1 + call.getArguments().size()); - if (newFunc != null) { + if (newFunc != null && !forwardsAVarargParameter(receiverAndArguments(call))) { redirectMethodCall(call, newFunc); } } } } + + /** + * Whether a call passes a vararg placeholder straight through, which is what the generated + * `new_C` wrapper of a vararg constructor does with its own parameter. The placeholder is a + * single node standing for however many arguments the caller actually passed, so the call's node + * count is not an arity: specialising by it would produce a fixed-arity callee and drop every + * argument after the first. + * + *

Only reachable on Lua. The forwarding call lives in the body of a vararg original, and a + * copy has its placeholder expanded into real parameters before anything looks at it again, so + * this matches only originals - which Jass removes and Lua retains. + * + *

Both the generation and the rewrite loop consult this. Skipping generation alone would not + * be enough: another call could have produced a copy at the same node count, and the rewrite + * would then redirect the forwarding call to it. + */ + private static boolean forwardsAVarargParameter(List arguments) { + for (ImExpr argument : arguments) { + if (argument instanceof ImVarAccess access && isVarargPlaceholder(access.getVar())) { + return true; + } + } + return false; + } + + /** The trailing parameter of a function still marked vararg, as opposed to a local or a copy's. */ + private static boolean isVarargPlaceholder(ImVar variable) { + if (variable.getParent() == null + || !(variable.getParent().getParent() instanceof ImFunction function) + || !function.hasFlag(IS_VARARG)) { + return false; + } + List parameters = function.getParameters(); + return !parameters.isEmpty() && parameters.get(parameters.size() - 1) == variable; + } /** A method call which can only ever reach one implementation, and that implementation is vararg. */ private Collection collectMonomorphicVarargMethodCalls() { final Collection calls = new ArrayList<>(); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index ce2c95050..05898e3c2 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -2096,6 +2096,39 @@ public void varargCallAboveTheLuaArityBoundKeepsThePackedPath() throws IOExcepti compiled.contains("table.pack")); } + /** + * A vararg constructor is reached through a compiler-generated `new_C` wrapper which forwards its + * vararg placeholder to `construct_C`. When a call above the bound keeps that wrapper as the + * retained original, its body still holds the forwarding call, and the placeholder is one node + * standing for however many arguments the caller passed. Specialising by node count would rewrite + * it to a fixed-arity constructor and silently drop every argument after the first. + */ + @Test + public void varargConstructorAboveTheLuaArityBoundKeepsThePackedPath() { + StringBuilder args = new StringBuilder(); + int n = 70; + for (int i = 1; i <= n; i++) { + if (i > 1) { + args.append(", "); + } + args.append(i); + } + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "class Tally", + " int total = 0", + " construct(vararg int xs)", + " for x in xs", + " total += x", + "init", + " let big = new Tally(" + args + ")", + " let small = new Tally(1, 2)", + " if big.total == " + (n * (n + 1) / 2) + " and small.total == 3", + " testSuccess()" + ); + } + @Test public void virtuallyDispatchedVarargMethodKeepsThePackedPath() throws IOException { test().testLua(true).executeProg().lines( From 4df540c5852953b6f1cb0ecc831f1058740c2a5a Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 3 Sep 2026 22:57:54 +0200 Subject: [PATCH 5/5] Carry type arguments through the vararg call redirects Both redirects rebuilt the call with an empty ImTypeArguments. That was right on Jass, where EliminateGenerics runs at phase 2 and nothing generic survives to the vararg phase, but it is an assumption about the pipeline written into a node constructor, and Lua reaches this pass with a different amount erased. Measured before changing anything, since the reported failure is a strong claim: a probe that threw whenever either redirect saw a non-empty type argument list found no hit anywhere in LuaBackendAuditTests, VarargTests, GenericsTests, LuaTranslationTests, GenericsWithTypeclassesTests or StdLibOwnTests. A generic vararg returning its type parameter, consumed by string concatenation, also produces the correct string without this change. So the reported miscompilation does not occur: Lua erases these before the pass rather than carrying them into it. The list is moved across anyway. It costs nothing, it is a no-op while the list is empty, and rebuilding a call should copy what the call had instead of restating a fact about phase order that only holds on one target. --- .../imtranslation/VarargEliminator.java | 11 +++++++-- .../tests/LuaBackendAuditTests.java | 23 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java index 9731c2368..e522281d0 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/VarargEliminator.java @@ -168,7 +168,8 @@ private static List receiverAndArguments(ImMethodCall call) { private void redirectMethodCall(ImMethodCall call, ImFunction newFunc) { ImExprs args = JassIm.ImExprs(call.getReceiver().copy()); args.addAll(call.getArguments().removeAll()); - call.replaceBy(JassIm.ImFunctionCall(call.getTrace(), newFunc, JassIm.ImTypeArguments(), args, + call.replaceBy(JassIm.ImFunctionCall(call.getTrace(), newFunc, + JassIm.ImTypeArguments(call.getTypeArguments().removeAll()), args, call.getTuplesEliminated(), CallType.NORMAL)); } @@ -350,7 +351,13 @@ public void visit(ImVarAccess va) { private void redirectCall(ImFunctionCall call, ImFunction newFunc) { // Redirect call to new function - ImFunctionCall newCall = JassIm.ImFunctionCall(call.getTrace(), newFunc, JassIm.ImTypeArguments(), JassIm.ImExprs(call.getArguments().removeAll()), call.getTuplesEliminated(), call.getCallType()); + // Carry the type arguments over rather than assuming there are none. Jass erases generics + // long before this pass, so an empty list was always right there; on Lua the erasure happens + // elsewhere and this list is empty in practice too, but rebuilding the call should not be + // the step that decides that. + ImFunctionCall newCall = JassIm.ImFunctionCall(call.getTrace(), newFunc, + JassIm.ImTypeArguments(call.getTypeArguments().removeAll()), + JassIm.ImExprs(call.getArguments().removeAll()), call.getTuplesEliminated(), call.getCallType()); call.replaceBy(newCall); } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 05898e3c2..49da89f0d 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -2129,6 +2129,29 @@ public void varargConstructorAboveTheLuaArityBoundKeepsThePackedPath() { ); } + /** + * Jass erases generics long before this pass, so `redirectCall` could build the replacement with + * an empty type-argument list. Lua only specialises concrete operations at that point and leaves + * generics live, so dropping them leaves the redirected call typed by an unresolved type variable. + * `LuaNativeLowering` decides string concatenation from each operand's type, so a generic vararg + * returning its type parameter silently became a numeric addition on strings. + */ + @Test + public void genericVarargCallKeepsItsTypeArgumentsOnLua() { + test().testLua(true).withStdLib().executeProg().lines( + "package Test", + "function lastOf(vararg T xs) returns T", + " T result = null", + " for x in xs", + " result = x", + " return result", + "init", + " let joined = \"a\" + lastOf(\"b\", \"c\")", + " if joined == \"ac\" and lastOf(1, 2) == 2", + " testSuccess()" + ); + } + @Test public void virtuallyDispatchedVarargMethodKeepsThePackedPath() throws IOException { test().testLua(true).executeProg().lines(