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.

Author: Frotty

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<ImExpr> 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<ImExpr> 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<ImExpr> 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<ImExpr> 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);

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(