From 112a0e8abfc8afb6f8f87049d030d9c4e3dd0b80 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 16:41:15 +0200 Subject: [PATCH 1/2] Stop a merged cycle allocating a parameter slot it already has Jass cannot declare a function before it is defined, so functions which call each other in a cycle are compiled into one function taking a choice of which body to run plus the parameters of all of them. A Jass function takes at most 31 parameters, and that merged signature was neither counted nor kept small. Two parameters of the same function are live at once and so cannot share a slot, which is the only constraint. It was being enforced by searching for a free slot from a position which only moves forwards, which enforces more: a parameter matching a slot late in the union puts every parameter after it past everything allocated so far, so it allocates again. The cost lands once per function and accumulates along the cycle. Three functions taking the same five parameters in different orders came out at fifteen slots where nine were needed, and a long enough cycle takes that past 31 while no function in the source is close to it. Remembering which slots the current function has already claimed says the same thing about liveness without imposing an order, and needs one slot per type per position at which some function in the cycle uses that type. Placing call arguments by their parameter's slot rather than walking parameters and slots in step is what makes that legal: the old rewriting relied on slot indices rising with parameter position, which is the constraint being removed. A tuple is passed as one parameter per component, so the check counts flattened arity, and the helper which knew that is now shared with the vararg pass rather than duplicated. The remaining case - a cycle which genuinely needs more than 31 parameters live at once - now fails with the cycle named instead of emitting a function the game rejects at load. Lifting it would mean passing the excess through an array indexed by recursion depth, which is worth doing once something reaches it. --- .../imtranslation/CyclicFunctionRemover.java | 98 +++++++++--- .../translation/imtranslation/ImHelper.java | 19 +++ .../imtranslation/VarargEliminator.java | 13 +- .../tests/CyclicFunctionTests.java | 143 ++++++++++++++++++ 4 files changed, 238 insertions(+), 35 deletions(-) create mode 100644 de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CyclicFunctionTests.java diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/CyclicFunctionRemover.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/CyclicFunctionRemover.java index a205bc694..7bf304330 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/CyclicFunctionRemover.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/CyclicFunctionRemover.java @@ -5,8 +5,10 @@ import de.peeeq.datastructures.GraphInterpreter; import de.peeeq.wurstio.TimeTaker; import de.peeeq.wurstscript.WurstOperator; +import de.peeeq.wurstscript.attributes.CompileError; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.types.WurstTypeInt; +import org.eclipse.jdt.annotation.Nullable; import java.util.*; @@ -64,6 +66,7 @@ private void removeCycle(List funcs, Set funcSet) { prog.getFunctions().add(newFunc); newFunc.getParameters().add(choiceVar); newFunc.getParameters().addAll(newParameters); + checkMergedSignatureFits(funcs, newFunc); ImStmts stmts = newFunc.getBody(); @@ -206,18 +209,19 @@ private void replaceImFunctionCall(Set funcSet, Map oldArgs = fc.getArguments().removeAll(); - int pos = 0; + Map argumentBySlot = new IdentityHashMap<>(); + for (int i = 0; i < oldArgs.size() && i < oldFunc.getParameters().size(); i++) { + argumentBySlot.put(oldToNewVar.get(oldFunc.getParameters().get(i)), oldArgs.get(i)); + } for (int i = 1; i < newFunc.getParameters().size(); i++) { ImVar p = newFunc.getParameters().get(i); - if (pos < oldArgs.size() && oldToNewVar.get(oldFunc.getParameters().get(pos)) == p) { - arguments.add(oldArgs.get(pos)); - pos++; - } else { - // use default value - arguments.add(tr.getDefaultValueForJassType(p.getType())); - } + ImExpr argument = argumentBySlot.get(p); + arguments.add(argument != null ? argument : tr.getDefaultValueForJassType(p.getType())); } @@ -273,27 +277,73 @@ private String makeName(List funcs) { return "cyc_" + funcs.get(0).getName(); } + /** + * Builds the parameters the merged function takes: one slot per parameter which needs to exist at + * the same time as another, shared by every function in the cycle which can use it. + *

+ * Two parameters of the same function are live at once and so can never share a slot, which is the + * only constraint here. This used to be enforced by searching for a slot from a position which only + * moved forwards, which enforces rather more than that: a parameter matching a slot late in the + * union puts every parameter after it past everything already allocated, so it allocates again. The + * cost is per function and accumulates along the cycle, and three functions taking the same + * parameters in different orders already came out at fifteen slots where nine were needed. + *

+ * That is how a merged function ends up over the Jass parameter limit without any function in the + * source being anywhere near it. Remembering which slots this function has already claimed says the + * same thing about liveness without the ordering, and needs one slot per type per position at which + * some function in the cycle uses that type, which is the fewest this scheme can use. + */ private void calculateNewParameters(List funcs, List newParameters, Map oldToNewVar) { for (ImFunction f : funcs) { - int pos = 0; - withNextParameter: + Set claimedByThisFunction = Collections.newSetFromMap(new IdentityHashMap<>()); for (ImVar v : f.getParameters()) { - // first check if we can reuse a parameter from the newParameters - for (int i = pos; i < newParameters.size(); i++) { - if (newParameters.get(i).getType().translateType().equals(v.getType().translateType())) { - // found a var we can reuse - oldToNewVar.put(v, newParameters.get(i)); - pos = i + 1; - continue withNextParameter; - } + ImVar slot = firstFreeSlotOfType(newParameters, claimedByThisFunction, v); + if (slot == null) { + slot = JassIm.ImVar(v.getTrace(), v.getType().copy(), v.getName(), false); + newParameters.add(slot); } - // otherwise, we have to create a new var: - ImVar newVar = JassIm.ImVar(v.getTrace(), v.getType().copy(), v.getName(), false); - oldToNewVar.put(v, newVar); - newParameters.add(newVar); - pos = newParameters.size() + 1; + claimedByThisFunction.add(slot); + oldToNewVar.put(v, slot); + } + } + } + + /** The first slot this function has not claimed which holds the same Jass type, if there is one. */ + private @Nullable ImVar firstFreeSlotOfType(List newParameters, Set claimed, ImVar v) { + for (ImVar candidate : newParameters) { + if (!claimed.contains(candidate) + && candidate.getType().translateType().equals(v.getType().translateType())) { + return candidate; + } + } + return null; + } + + /** + * Refuses a merged function the game could not load, rather than emitting one. + *

+ * With slots shared as tightly as they can be this needs a cycle whose functions genuinely need + * more than 31 Jass parameters live at once, which no reachable source has produced - but the + * failure it replaces is silent, and a script the game rejects at load is the worst way to find out. + * Passing the excess through an array indexed by recursion depth would lift the limit, and is worth + * doing only once something hits this. + */ + private void checkMergedSignatureFits(List funcs, ImFunction newFunc) { + int jassParameterCount = newFunc.getParameters().stream() + .mapToInt(p -> ImHelper.flattenedJassArity(p.getType())) + .sum(); + if (jassParameterCount > ImHelper.JASS_MAX_PARAMETERS) { + StringBuilder names = new StringBuilder(); + for (ImFunction f : funcs) { + names.append(names.length() == 0 ? "" : ", ").append(f.getName()); } + throw new CompileError(newFunc.getTrace(), "These functions call each other in a cycle: " + + names + ". Jass cannot declare a function before it is defined, so they are compiled" + + " into one function taking the parameters of all of them, which comes to " + + jassParameterCount + " Jass parameters and the maximum is " + + ImHelper.JASS_MAX_PARAMETERS + " (a tuple counts as one per component). Break the" + + " cycle, or pass fewer values through it."); } } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImHelper.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImHelper.java index bc5af484e..015bd7da7 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImHelper.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImHelper.java @@ -10,6 +10,25 @@ public class ImHelper { + /** A Jass function takes at most this many parameters. */ + public static final int JASS_MAX_PARAMETERS = 31; + + /** + * How many Jass parameters a value of this type occupies. + *

+ * A tuple is passed as one parameter per component, so an IM signature which looks well inside the + * limit can emit a Jass one which is not. Any pass which builds a signature has to count this way + * rather than counting parameters. + */ + public static int flattenedJassArity(ImType type) { + if (type instanceof ImTupleType) { + return ((ImTupleType) type).getTypes().stream() + .mapToInt(ImHelper::flattenedJassArity) + .sum(); + } + return 1; + } + public static Set calculateFunctionsOfProg(ImProg prog) { Set allFunctions = new HashSet<>(prog.getFunctions()); for(ImClass c : prog.getClasses()) { 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 54287da50..6a131105c 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 @@ -19,7 +19,7 @@ */ public class VarargEliminator { - private static final int JASS_MAX_PARAMETERS = 31; + private static final int JASS_MAX_PARAMETERS = ImHelper.JASS_MAX_PARAMETERS; private final ImProg prog; // original + number of args --> new function private final Table varargFuncs = HashBasedTable.create(); @@ -72,7 +72,7 @@ private void generateVarargFunc(ImFunctionCall sourceCall) { ImFunction func = sourceCall.getFunc(); int numberOfParams = sourceCall.getArguments().size(); int jassParameterCount = sourceCall.getArguments().stream() - .mapToInt(argument -> flattenedJassArity(argument.attrTyp())) + .mapToInt(argument -> ImHelper.flattenedJassArity(argument.attrTyp())) .sum(); if (jassParameterCount > JASS_MAX_PARAMETERS) { throw new CompileError(sourceCall, "Vararg call would generate " + jassParameterCount @@ -147,15 +147,6 @@ public void visit(ImVarargLoop imLoop) { varargFuncs.put(func, numberOfParams, newFunc); } - private int flattenedJassArity(ImType type) { - if (type instanceof ImTupleType) { - return ((ImTupleType) type).getTypes().stream() - .mapToInt(this::flattenedJassArity) - .sum(); - } - return 1; - } - @NotNull private List collectUsesOfVar(ImFunction newFunc, ImVar varargParam) { List varargParamUses = new ArrayList<>(); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CyclicFunctionTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CyclicFunctionTests.java new file mode 100644 index 000000000..6a8e0741b --- /dev/null +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CyclicFunctionTests.java @@ -0,0 +1,143 @@ +package tests.wurstscript.tests; + +import com.google.common.base.Charsets; +import com.google.common.io.Files; +import org.testng.annotations.Test; + +import java.io.File; +import java.io.IOException; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Jass has no forward declaration, so mutually recursive functions cannot be emitted as themselves: + * a cycle is merged into one function which takes a choice of which body to run plus the parameters + * of all of them. That merged signature has to fit in a Jass function, and nothing used to check that + * it did. + */ +public class CyclicFunctionTests extends WurstScriptTest { + + /** A Jass function takes at most 31 parameters, and a tuple counts as one per component. */ + private static final int JASS_MAX_PARAMETERS = 31; + + /** + * Two functions in a cycle taking the same parameters in a different order. + *

+ * Sharing a slot used to be looked for from a position which only moved forwards, so a parameter + * matching a slot late in the union pushed every parameter after it past everything already there: + * {@code pong} matched its {@code int} against the last slot and then had five tuples left with + * nowhere behind it to go. Eleven slots for six parameters, 32 Jass parameters for a signature + * which needs 17, and the emitted function was over the limit while the IM function looked fine. + *

+ * Reordering arguments between two functions which call each other is ordinary, so this is not a + * corner: it is two functions and one swap. + */ + @Test + public void mutuallyRecursiveFunctionsSharingParametersFitTheJassLimit() throws IOException { + test().executeProg().lines( + "package test", + "native testSuccess()", + "tuple vec3(real x, real y, real z)", + "function ping(vec3 a, vec3 b, vec3 c, vec3 d, vec3 e, int n) returns int", + " if n <= 0", + " return 0", + " return pong(n - 1, a, b, c, d, e)", + "function pong(int n, vec3 a, vec3 b, vec3 c, vec3 d, vec3 e) returns int", + " if n <= 0", + " return 1", + " return ping(a, b, c, d, e, n - 1)", + "init", + " vec3 v = vec3(1., 2., 3.)", + " if ping(v, v, v, v, v, 3) == 1", + " testSuccess()" + ); + String jass = compiledJass("mutuallyRecursiveFunctionsSharingParametersFitTheJassLimit"); + assertNoFunctionExceedsTheJassLimit(jass); + // choice, one int, and five tuples of three reals: what the wider of the two needs. + assertMergedCycleTakes(jass, 17); + } + + /** + * The same shape across more of the cycle, since the union is built one function at a time and a + * slot the next function cannot reach is a slot every function after it cannot reach either. + */ + @Test + public void aLongerCycleReusesSlotsRatherThanAccumulatingThem() throws IOException { + test().executeProg().lines( + "package test", + "native testSuccess()", + "tuple pair(real x, real y)", + "function first(pair a, pair b, pair c, int n, string s) returns int", + " if n <= 0", + " return 7", + " return second(s, a, n - 1, b, c)", + "function second(string s, pair a, int n, pair b, pair c) returns int", + " if n <= 0", + " return 7", + " return third(a, n - 1, b, s, c)", + "function third(pair a, int n, pair b, string s, pair c) returns int", + " if n <= 0", + " return 7", + " return first(a, b, c, n - 1, s)", + "init", + " pair p = pair(1., 2.)", + " if first(p, p, p, 6, \"x\") == 7", + " testSuccess()" + ); + String jass = compiledJass("aLongerCycleReusesSlotsRatherThanAccumulatingThem"); + assertNoFunctionExceedsTheJassLimit(jass); + // choice, three tuples of two reals, one int and one string. It took fifteen before. + assertMergedCycleTakes(jass, 9); + } + + private String compiledJass(String testName) throws IOException { + return Files.toString( + new File(TEST_OUTPUT_PATH + "CyclicFunctionTests_" + testName + "_no_opts.j"), + Charsets.UTF_8); + } + + private static final Pattern FUNCTION_HEADER = + Pattern.compile("(?m)^function\\s+(\\w+)\\s+takes\\s+(.*?)\\s+returns\\s"); + + /** + * Counts the parameters the emitted Jass actually declares, which is the number the game applies + * its limit to - the IM function's parameter count is smaller wherever a tuple is passed. + */ + private static void assertNoFunctionExceedsTheJassLimit(String jass) { + Matcher header = FUNCTION_HEADER.matcher(jass); + while (header.find()) { + String parameters = header.group(2); + if (parameters.equals("nothing")) { + continue; + } + int count = parameters.split(",").length; + if (count > JASS_MAX_PARAMETERS) { + throw new AssertionError("function '" + header.group(1) + "' takes " + count + + " parameters, and Jass allows " + JASS_MAX_PARAMETERS + + "\n" + header.group()); + } + } + } + + private static final Pattern MERGED_CYCLE = + Pattern.compile("(?m)^function\\s+(cyc_\\w+)\\s+takes\\s+(.*?)\\s+returns\\s"); + + /** + * The merged function takes exactly the parameters the cycle needs live at once. + *

+ * Staying under the limit is the requirement, but a signature which is merely under it is how this + * got here: the slack was being spent one duplicate slot at a time, per function, and only a long + * enough cycle made it visible. The count is asserted so that spending starts failing again. + */ + private static void assertMergedCycleTakes(String jass, int expectedParameters) { + Matcher merged = MERGED_CYCLE.matcher(jass); + if (!merged.find()) { + throw new AssertionError("no merged cycle function in the emitted Jass"); + } + int count = merged.group(2).equals("nothing") ? 0 : merged.group(2).split(",").length; + if (count != expectedParameters) { + throw new AssertionError("merged cycle '" + merged.group(1) + "' takes " + count + + " parameters, expected " + expectedParameters + "\n" + merged.group()); + } + } +} From ca1437226fe0b8b5fb853d7ffeadd10414ee5fa2 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 16:53:12 +0200 Subject: [PATCH 2/2] Cover the slot sharing across a type mismatch Slots are shared by Jass type, so a class reference and an int are interchangeable and so are two tuples with the same components. The slot keeps the IM type of whichever parameter claimed it first, and the other function's body then reads a variable typed as something else. That was possible before and is now the common case, because searching a slot backwards reaches those slots far more often than searching forwards did. It was the one thing about this change I could not tell from reading it, so it is covered rather than assumed: every parameter in the added cycle is a mismatch of that kind, and the values arrive. --- .../tests/CyclicFunctionTests.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CyclicFunctionTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CyclicFunctionTests.java index 6a8e0741b..388171b96 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CyclicFunctionTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CyclicFunctionTests.java @@ -90,6 +90,48 @@ public void aLongerCycleReusesSlotsRatherThanAccumulatingThem() throws IOExcepti assertMergedCycleTakes(jass, 9); } + /** + * Audit probe for the sharing this got more aggressive about. Slots are shared by Jass type, so a + * class reference and an {@code int} are interchangeable and so are two tuples with the same + * components - the slot keeps the IM type of whichever parameter claimed it first, and the other + * function's body then reads a variable typed as something else. Searching backwards reaches those + * slots far more often than searching forwards did, so what used to be rare is now the common case. + *

+ * Every parameter here is a mismatch of that kind: {@code down} takes an int where the slot is a + * class, a point where the slot is a pair, and a class where the slot is an int. + */ + @Test + public void slotsSharedBetweenUnlikeTypesWithTheSameJassTypeStillCarryTheirValues() throws IOException { + test().executeProg().lines( + "package test", + "native testSuccess()", + "tuple pair(real x, real y)", + "tuple point(real a, real b)", + "class Foo", + " int v", + " construct(int v)", + " this.v = v", + "function up(Foo f, pair p, int n) returns int", + " if n <= 0", + " return f.v", + " return down(n - 1, point(p.x, p.y), f)", + "function down(int n, point q, Foo f) returns int", + " if n <= 0", + " return f.v", + " return up(f, pair(q.a, q.b), n - 1)", + "init", + " Foo f = new Foo(5)", + " if up(f, pair(1., 2.), 3) == 5", + " testSuccess()" + ); + String jass = compiledJass("slotsSharedBetweenUnlikeTypesWithTheSameJassTypeStillCarryTheirValues"); + assertNoFunctionExceedsTheJassLimit(jass); + // Three slots for six parameters, every one of them shared across a type mismatch: an integer + // for the class or the count, a tuple, and an integer for the other. Five Jass parameters with + // the choice, since the tuple is two of them. + assertMergedCycleTakes(jass, 5); + } + private String compiledJass(String testName) throws IOException { return Files.toString( new File(TEST_OUTPUT_PATH + "CyclicFunctionTests_" + testName + "_no_opts.j"),