From cb4346f97c7ef79d970ce4e2e2fa4ab934da61f9 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 12:17:02 +0200 Subject: [PATCH 1/7] Let a keyed table be destroyed. The Jass body of a keyed table is built on a Table, whose instances come from a finite pool, so a keyed set that is cleared or discarded would burn one permanently - clearing cannot empty the table in place because that needs pairs(), which desyncs a lockstep game. Lua has a collector and nothing to free, so the stub is empty, but the call still has to lower there: left alone, the Jass body would try to destroy a Table that does not exist on that backend. --- .../imtranslation/LuaKeyedTable.java | 7 ++++ .../lua/translation/LuaNatives.java | 5 +++ .../tests/LuaTranslationTests.java | 34 +++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedTable.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedTable.java index 2bd9aab17..7af6bf945 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedTable.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedTable.java @@ -36,12 +36,14 @@ public final class LuaKeyedTable { private static final String ADD = "keyedTableAdd"; private static final String CONTAINS = "keyedTableContains"; private static final String REMOVE = "keyedTableRemove"; + private static final String DESTROY = "keyedTableDestroy"; /** Stub names whose Lua bodies live in {@code LuaNatives}. */ public static final String NATIVE_CREATE = "__wurst_keyedTableCreate"; public static final String NATIVE_ADD = "__wurst_keyedTableAdd"; public static final String NATIVE_CONTAINS = "__wurst_keyedTableContains"; public static final String NATIVE_REMOVE = "__wurst_keyedTableRemove"; + public static final String NATIVE_DESTROY = "__wurst_keyedTableDestroy"; private LuaKeyedTable() { } @@ -65,6 +67,11 @@ public static String nativeStubFor(ImFunction f) { ? NATIVE_REMOVE : null; case CONTAINS -> isKeyedPair(f) && TypesHelper.isBoolType(f.getReturnType()) ? NATIVE_CONTAINS : null; + // Jass frees the Table the keyed table is built on; Lua leaves it to the collector. + case DESTROY -> params == 1 + && TypesHelper.isIntType(f.getParameters().get(0).getType()) + && f.getReturnType() instanceof ImVoid + ? NATIVE_DESTROY : null; default -> null; }; } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java index 5e17ada7f..fa5846577 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java @@ -292,6 +292,11 @@ public class LuaNatives { f.getParams().add(LuaAst.LuaVariable("k", LuaAst.LuaNoExpr())); f.getBody().add(LuaAst.LuaLiteral("t[k] = nil")); }); + // Nothing to do: the table is garbage once the caller drops it. The Jass body this + // replaces frees a Table instance, which comes from a finite pool - without a destroy + // operation every cleared or discarded keyed set would burn one there permanently. + addNative("__wurst_keyedTableDestroy", f -> + f.getParams().add(LuaAst.LuaVariable("t", LuaAst.LuaNoExpr()))); addNative(Arrays.asList("InitHashtable", "__wurst_InitHashtable"), f -> f.getBody().add(LuaAst.LuaLiteral("return { __wurst_ht_int = {}, __wurst_ht_bool = {}, __wurst_ht_real = {}, __wurst_ht_str = {}, __wurst_ht_handle = {} }"))); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index ddab3f693..26bbdbee4 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -1984,6 +1984,8 @@ private static String[] keyedTableSource(String... usage) { " return (tbl castTo Table).loadBoolean(key)", "@compilerintrinsic public function keyedTableRemove(int tbl, int key)", " (tbl castTo Table).removeBoolean(key)", + "@compilerintrinsic public function keyedTableDestroy(int tbl)", + " destroy (tbl castTo Table)", "endpackage")); lines.addAll(java.util.Arrays.asList(usage)); return lines.toArray(new String[0]); @@ -2017,6 +2019,38 @@ public void keyedTableLowersToASingleLuaIndex() throws IOException { assertFalse("contains must not go through the hashtable natives: " + contains, contains.contains("LoadBoolean")); } + /** + * A keyed table has to be disposable: the Jass body frees a Table instance, which comes from a + * finite pool, so a set that is cleared or discarded would otherwise burn one permanently. Lua + * has a collector and nothing to free, so the stub is empty - but the call must still lower, + * or the Jass body would run against a bare Lua table. + */ + @Test + public void keyedTableDestroyLowersToAnEmptyLuaStub() throws IOException { + test().testLua(true).withStdLib().lines(keyedTableSource( + "package Test", + "import KeyedTable", + "init", + " let t = keyedTableCreate()", + " keyedTableAdd(t, 7)", + " keyedTableDestroy(t)", + "endpackage")); + + String compiled = Files.toString( + new File("test-output/lua/LuaTranslationTests_keyedTableDestroyLowersToAnEmptyLuaStub.lua"), + Charsets.UTF_8); + + assertTrue("destroy must lower to the keyed-table stub", + compiled.contains("__wurst_keyedTableDestroy")); + + String destroy = getFunctionBody(compiled, "__wurst_keyedTableDestroy"); + assertTrue("destroy must do nothing on Lua, was: " + destroy, destroy.trim().isEmpty()); + + // The Jass body must not survive: it would destroy a Table that does not exist here. + assertFalse("destroy must not reach the Table machinery: " + destroy, + destroy.contains("Table") || destroy.contains("Flush")); + } + /** * Membership must mean the same thing on both backends: the Jass path runs the hashtable * body, the Lua path runs a bare table, and the observable behaviour has to agree. From b3a13cc93138afcafa24834f395bd794b7a20ab0 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 12:41:07 +0200 Subject: [PATCH 2/7] Empty the destroy operation on Lua instead of stubbing it. A native stub is an analysis barrier, so the inliner would not cross it and every clear and every destroy kept a call doing no work - and clearing is not rare: a spatial index reusing its sets clears them as often as it refills them. Emptying the function leaves an ordinary one, which the inliner removes along with the call, while the argument is still evaluated. --- .../imtranslation/LuaKeyedTable.java | 24 +++++++++++++----- .../imtranslation/LuaNativeLowering.java | 10 ++++++++ .../lua/translation/LuaNatives.java | 5 ---- .../tests/LuaTranslationTests.java | 25 +++++++++++-------- 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedTable.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedTable.java index 7af6bf945..f2fe2d535 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedTable.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedTable.java @@ -43,7 +43,6 @@ public final class LuaKeyedTable { public static final String NATIVE_ADD = "__wurst_keyedTableAdd"; public static final String NATIVE_CONTAINS = "__wurst_keyedTableContains"; public static final String NATIVE_REMOVE = "__wurst_keyedTableRemove"; - public static final String NATIVE_DESTROY = "__wurst_keyedTableDestroy"; private LuaKeyedTable() { } @@ -67,15 +66,28 @@ public static String nativeStubFor(ImFunction f) { ? NATIVE_REMOVE : null; case CONTAINS -> isKeyedPair(f) && TypesHelper.isBoolType(f.getReturnType()) ? NATIVE_CONTAINS : null; - // Jass frees the Table the keyed table is built on; Lua leaves it to the collector. - case DESTROY -> params == 1 - && TypesHelper.isIntType(f.getParameters().get(0).getType()) - && f.getReturnType() instanceof ImVoid - ? NATIVE_DESTROY : null; default -> null; }; } + /** + * Whether {@code f} frees a keyed table. + * + *

Unlike the four operations above this gets no stub: Jass frees the Table the keyed table + * is built on, Lua leaves it to the collector, so there is nothing for a Lua body to do. + * Emptying the function rather than replacing calls to it with an IS_NATIVE stub is what lets + * the inliner remove the call - a native is an analysis barrier, so a stub would leave a call + * that does nothing on every clear and every destroy. + */ + public static boolean isDestroy(ImFunction f) { + return f.attrTrace() instanceof FuncDef fd + && DESTROY.equals(fd.getName()) + && fd.attrHasAnnotation(CompilerIntrinsics.ANNOTATION) + && f.getParameters().size() == 1 + && TypesHelper.isIntType(f.getParameters().get(0).getType()) + && f.getReturnType() instanceof ImVoid; + } + /** * A (table, key) parameter pair. Both are {@code int} at source level: on Jass everything is an * integer anyway, and on Lua {@code castTo int} is the identity for class types, so the value diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java index 6d278d64b..03db847bf 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java @@ -125,6 +125,16 @@ public static void transform(ImProg prog, ImTranslator translator) { } } + // Freeing a keyed table means nothing on Lua: the table is garbage once the caller drops + // it. Emptying the function leaves an ordinary one the inliner can remove, where a native + // stub would be an analysis barrier and leave a call doing nothing on every clear. + for (ImFunction f : prog.getFunctions()) { + if (LuaKeyedTable.isDestroy(f)) { + f.getBody().clear(); + f.getLocals().clear(); + } + } + lowerStringConcatenation(prog, translator); lowerDivMod(prog, translator); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java index fa5846577..5e17ada7f 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaNatives.java @@ -292,11 +292,6 @@ public class LuaNatives { f.getParams().add(LuaAst.LuaVariable("k", LuaAst.LuaNoExpr())); f.getBody().add(LuaAst.LuaLiteral("t[k] = nil")); }); - // Nothing to do: the table is garbage once the caller drops it. The Jass body this - // replaces frees a Table instance, which comes from a finite pool - without a destroy - // operation every cleared or discarded keyed set would burn one there permanently. - addNative("__wurst_keyedTableDestroy", f -> - f.getParams().add(LuaAst.LuaVariable("t", LuaAst.LuaNoExpr()))); addNative(Arrays.asList("InitHashtable", "__wurst_InitHashtable"), f -> f.getBody().add(LuaAst.LuaLiteral("return { __wurst_ht_int = {}, __wurst_ht_bool = {}, __wurst_ht_real = {}, __wurst_ht_str = {}, __wurst_ht_handle = {} }"))); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index 26bbdbee4..796820c5b 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -2021,13 +2021,15 @@ public void keyedTableLowersToASingleLuaIndex() throws IOException { /** * A keyed table has to be disposable: the Jass body frees a Table instance, which comes from a - * finite pool, so a set that is cleared or discarded would otherwise burn one permanently. Lua - * has a collector and nothing to free, so the stub is empty - but the call must still lower, - * or the Jass body would run against a bare Lua table. + * finite pool, so a set that is cleared or discarded would otherwise burn one permanently. + * + *

Lua has a collector and nothing to free, so the operation must cost nothing there. It is + * emptied rather than replaced by a native stub, because a native is an analysis barrier the + * inliner will not cross - which would leave a call doing no work on every clear and destroy. */ @Test - public void keyedTableDestroyLowersToAnEmptyLuaStub() throws IOException { - test().testLua(true).withStdLib().lines(keyedTableSource( + public void keyedTableDestroyCostsNothingOnLua() throws IOException { + test().testLua(true).inline().withStdLib().lines(keyedTableSource( "package Test", "import KeyedTable", "init", @@ -2037,18 +2039,19 @@ public void keyedTableDestroyLowersToAnEmptyLuaStub() throws IOException { "endpackage")); String compiled = Files.toString( - new File("test-output/lua/LuaTranslationTests_keyedTableDestroyLowersToAnEmptyLuaStub.lua"), + new File("test-output/lua/LuaTranslationTests_keyedTableDestroyCostsNothingOnLua.lua"), Charsets.UTF_8); - assertTrue("destroy must lower to the keyed-table stub", + assertFalse("destroy must not become a native stub, which the inliner cannot remove", compiled.contains("__wurst_keyedTableDestroy")); - String destroy = getFunctionBody(compiled, "__wurst_keyedTableDestroy"); - assertTrue("destroy must do nothing on Lua, was: " + destroy, destroy.trim().isEmpty()); + String init = getFunctionBody(compiled, "init_Test"); + assertFalse("no call should remain to free a keyed table on Lua: " + init, + init.contains("keyedTableDestroy")); // The Jass body must not survive: it would destroy a Table that does not exist here. - assertFalse("destroy must not reach the Table machinery: " + destroy, - destroy.contains("Table") || destroy.contains("Flush")); + assertFalse("the Table machinery must not reach Lua: " + init, + init.contains("FlushChildHashtable") || init.contains("Table_destroy")); } /** From 1fbca014e622dd4788789cb3e4d2423d6290a2d9 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 15:24:51 +0200 Subject: [PATCH 3/7] Lower keyed tables before stack traces are injected. Stack-trace injection appends a parameter to every affected function, and on Lua that is every non-native function. The keyed-table operations are recognised by their exact IM signature, so once the injector had run nothing matched and the lowering quietly did not happen: the Jass bodies survived onto Lua, where wurstKeyOf is never lowered and answers with its placeholder, so every element shared one key and a set claimed to contain everything it was asked about. Nothing reported it, and a release build emits stack traces by default, so this was the common case rather than an exotic one. The Jass side was never affected - JassKeyOfLowering already runs before the injector. The lowering moves into its own pass, run before injection. The test harness gains a stacktraces() toggle for the Lua path, which had no way to emit them and so could not have caught this. --- .../peeeq/wurstio/WurstCompilerJassImpl.java | 8 ++ .../imtranslation/LuaNativeLowering.java | 85 +++++++++++++------ .../tests/LuaTranslationTests.java | 42 +++++++++ .../wurstscript/tests/WurstScriptTest.java | 15 +++- 4 files changed, 121 insertions(+), 29 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 57edf33d2..37a0cd677 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 @@ -892,6 +892,14 @@ public LuaCompilationUnit transformProgToLua() { RemoveGarbage.removePhantomGenericStaticInitializers(getImProg(), getImTranslator()); timeTaker.endPhase(); } + // Before stack traces: that pass appends a parameter to every affected function, and on + // Lua every non-native function is affected, so the exact signatures the keyed-table + // operations are recognised by would stop matching - silently leaving their Jass bodies on + // Lua, where wurstKeyOf answers with its placeholder and every element shares one key. + beginPhase(4, "lower keyed tables"); + LuaNativeLowering.lowerKeyedTables(imProg); + timeTaker.endPhase(); + if (runArgs.isNoDebugMessages()) { beginPhase(3, "remove debug messages"); DebugMessageRemover.removeDebugMessages(imProg); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java index 03db847bf..94648d9e7 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java @@ -110,6 +110,61 @@ private LuaNativeLowering() {} * creating wrappers for every BJ function in the IM (common.j declares hundreds of * functions, most of which are unreachable in any given program). */ + /** + * Replaces the KeyedTable operations with their Lua stubs, and empties the destroy operation. + * + *

Separate from {@link #transform} so it can run before stack-trace injection. That + * pass appends a parameter to every affected function, and on Lua every non-native function is + * affected, so the exact signatures these operations are recognised by stop matching. Nothing + * reported that: the Jass bodies simply survived onto Lua, where {@code wurstKeyOf} is never + * lowered and answers with its placeholder, so every element shared one key and a set claimed + * to hold everything. Stack traces are on by default in a release build, so that was the + * common case rather than an exotic one. + * + *

Membership becomes a table keyed directly by the element. Done before optimization rather + * than at emission because the inliner runs in between: a call inlined before an emission-time + * rewrite would keep the hashtable body while a surviving one got the Lua table, mixing an + * integer class id with a table index for the same value. Replacing the call makes every site + * agree. + * + *

Idempotent: once the calls point at stubs, nothing matches on a second run. + */ + public static void lowerKeyedTables(ImProg prog) { + // Freeing a keyed table means nothing on Lua: the table is garbage once the caller drops + // it. Emptying the function leaves an ordinary one the inliner can remove, where a native + // stub would be an analysis barrier and leave a call doing nothing on every clear. + for (ImFunction f : prog.getFunctions()) { + if (LuaKeyedTable.isDestroy(f)) { + f.getBody().clear(); + f.getLocals().clear(); + } + } + + Map stubs = new LinkedHashMap<>(); + List additions = new ArrayList<>(); + prog.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImFunctionCall call) { + super.visit(call); + ImFunction f = call.getFunc(); + String stubName = LuaKeyedTable.nativeStubFor(f); + if (stubName == null) { + return; + } + ImFunction replacement = stubs.computeIfAbsent(stubName, name -> createNativeStub(name, f)); + if (!additions.contains(replacement)) { + additions.add(replacement); + } + call.replaceBy(JassIm.ImFunctionCall( + call.attrTrace(), replacement, + JassIm.ImTypeArguments(), + call.getArguments().copy(), + false, CallType.NORMAL)); + } + }); + prog.getFunctions().addAll(additions); + } + public static void transform(ImProg prog, ImTranslator translator) { // Replace all reads of MagicFunctions_isLua with true. // This must happen before any optimizer passes so that dead-code elimination @@ -125,15 +180,8 @@ public static void transform(ImProg prog, ImTranslator translator) { } } - // Freeing a keyed table means nothing on Lua: the table is garbage once the caller drops - // it. Emptying the function leaves an ordinary one the inliner can remove, where a native - // stub would be an analysis barrier and leave a call doing nothing on every clear. - for (ImFunction f : prog.getFunctions()) { - if (LuaKeyedTable.isDestroy(f)) { - f.getBody().clear(); - f.getLocals().clear(); - } - } + // Idempotent: transformProgToLua runs this earlier, before stack-trace injection. + lowerKeyedTables(prog); lowerStringConcatenation(prog, translator); lowerDivMod(prog, translator); @@ -156,25 +204,6 @@ public static void transform(ImProg prog, ImTranslator translator) { public void visit(ImFunctionCall call) { super.visit(call); ImFunction f = call.getFunc(); - // KeyedTable membership becomes a table keyed directly by the element. Done here, - // before optimization, rather than at emission: the inliner runs in between, and a - // call inlined before an emission-time rewrite would keep the hashtable body while - // a surviving one got the Lua table - mixing an integer class id with a table index - // for the same value. Replacing the call makes every site agree. - String keyedStub = LuaKeyedTable.nativeStubFor(f); - if (keyedStub != null) { - ImFunction replacement = specialNativeStubs.computeIfAbsent(keyedStub, - name -> createNativeStub(name, f)); - if (!deferredAdditions.contains(replacement)) { - deferredAdditions.add(replacement); - } - call.replaceBy(JassIm.ImFunctionCall( - call.attrTrace(), replacement, - JassIm.ImTypeArguments(), - call.getArguments().copy(), - false, CallType.NORMAL)); - return; - } if (ENABLE_SELECTIVE_GET_HANDLE_ID_SHIMMING && isCompatGetHandleIdFunction(f)) { if (shouldRewriteGetHandleId(call)) { ImFunction replacement = specialNativeStubs.computeIfAbsent("__wurst_GetHandleId", diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index 796820c5b..d7878bb76 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -2019,6 +2019,48 @@ public void keyedTableLowersToASingleLuaIndex() throws IOException { assertFalse("contains must not go through the hashtable natives: " + contains, contains.contains("LoadBoolean")); } + /** + * Stack traces must not cost the keyed table its native representation. + * + *

Stack-trace injection appends a parameter to every affected function, and on Lua that is + * every non-native function, so the exact signatures the keyed-table operations are recognised + * by stop matching once it has run. Nothing reported that when it happened: the Jass bodies + * simply survived onto Lua, where `wurstKeyOf` is never lowered and answers with its + * placeholder, so every element shared one key and a set claimed to contain everything. + * + *

A release build emits stack traces by default, so this was the common case. + */ + @Test + public void keyedTableStaysNativeWithStackTraces() throws IOException { + test().testLua(true).stacktraces().inline().withStdLib().lines(keyedTableSource( + "package Test", + "import KeyedTable", + "init", + " let t = keyedTableCreate()", + " keyedTableAdd(t, 7)", + " if keyedTableContains(t, 7) and not keyedTableContains(t, 9)", + " print(\"distinct\")", + "endpackage")); + + String compiled = Files.toString( + new File("test-output/lua/LuaTranslationTests_keyedTableStaysNativeWithStackTraces.lua"), + Charsets.UTF_8); + + assertTrue("membership must still lower to the keyed-table stubs under -stacktraces", + compiled.contains("__wurst_keyedTableContains") && compiled.contains("__wurst_keyedTableAdd")); + assertTrue("contains must still be a single index", + getFunctionBody(compiled, "__wurst_keyedTableContains").contains("] ~= nil")); + + // The Jass body is the failure mode. On a keyed set it keys through wurstKeyOf, which is + // never lowered on Lua, so every element would collapse onto the same key. Scoped to the + // caller: Table itself is compiled in and legitimately uses the hashtable natives. + String init = getFunctionBody(compiled, "init_Test"); + assertFalse("the caller must not reach the hashtable natives: " + init, + init.contains("SaveBoolean") || init.contains("LoadBoolean")); + assertTrue("the caller must call the stubs directly: " + init, + init.contains("__wurst_keyedTableContains")); + } + /** * A keyed table has to be disposable: the Jass body frees a Table instance, which comes from a * finite pool, so a set that is cleared or discarded would otherwise burn one permanently. diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/WurstScriptTest.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/WurstScriptTest.java index 9e17d5f24..4531cbf8a 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/WurstScriptTest.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/WurstScriptTest.java @@ -118,6 +118,7 @@ class TestConfig { private boolean runCompiletimeFunctions; private boolean optimize; private boolean inline; + private boolean stacktraces; private boolean testLua = false; private boolean luaOnly = false; private boolean uncheckedDispatch = false; @@ -183,6 +184,18 @@ TestConfig expectWarning(String expectedWarning) { return this; } + /** + * Emit stack traces, as a release build does by default. + * + *

Worth testing on the Lua path: stack-trace injection appends a parameter to every + * affected function there, so a lowering which recognises a function by its exact + * signature stops matching once it has run. + */ + public TestConfig stacktraces() { + this.stacktraces = true; + return this; + } + public TestConfig executeProgOnlyAfterTransforms() { this.executeProgOnlyAfterTransforms = true; return this; @@ -342,7 +355,7 @@ private CompilationResult testScript() { if (testLua) { // test lua translation - runArgs = runArgs.with("-lua"); + runArgs = stacktraces ? runArgs.with("-lua", "-stacktraces") : runArgs.with("-lua"); compiler.setRunArgs(runArgs); translateAndTestLua(name, executeProg, gui, model, compiler); } From ed6cb2002431b4185ddf518f6bd709eda31f973a Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 16:47:16 +0200 Subject: [PATCH 4/7] Do not instrument the emptied destroy operation. Freeing a keyed table means nothing on Lua, so the lowering empties it, but it stayed an ordinary function and stack-trace injection then put the cost back: a trace argument at every call site, and a push and pop around a body that does nothing. That bookkeeping is also what stopped the inliner removing the call, so the operation was free only in a build without stack traces - which a release build is not. There is nothing to trace in an empty body, so the injector skips it. The regression now emits stack traces, since without them it could not have seen this. --- .../translation/imtranslation/StackTraceInjector2.java | 7 ++++++- .../java/tests/wurstscript/tests/LuaTranslationTests.java | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java index fa516d5ab..29c33ca5d 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java @@ -117,7 +117,12 @@ public void visit(ImFuncRef imFuncRef) { .filter((ImFunction f) -> !f.hasFlag(FunctionFlagEnum.IS_NATIVE) && !f.hasFlag(FunctionFlagEnum.IS_BJ) - && !f.hasFlag(FunctionFlagEnum.IS_EXTERN)) + && !f.hasFlag(FunctionFlagEnum.IS_EXTERN) + // Freeing a keyed table means nothing on Lua, so LuaNativeLowering has + // already emptied this one. Instrumenting it would put the cost back: + // a trace parameter at every call site and a push and pop around a body + // that does nothing, which is also what stops the inliner removing it. + && !LuaKeyedTable.isDestroy(f)) .collect(Collectors.toCollection(() -> affectedFuncs)); affectedFuncs.removeAll(configOnlyFuncs); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index d7878bb76..1b90794b0 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -2071,7 +2071,9 @@ public void keyedTableStaysNativeWithStackTraces() throws IOException { */ @Test public void keyedTableDestroyCostsNothingOnLua() throws IOException { - test().testLua(true).inline().withStdLib().lines(keyedTableSource( + // With stack traces, because that is what a release build emits, and instrumenting an + // emptied function is exactly how the cost comes back. + test().testLua(true).stacktraces().inline().withStdLib().lines(keyedTableSource( "package Test", "import KeyedTable", "init", @@ -2090,6 +2092,9 @@ public void keyedTableDestroyCostsNothingOnLua() throws IOException { String init = getFunctionBody(compiled, "init_Test"); assertFalse("no call should remain to free a keyed table on Lua: " + init, init.contains("keyedTableDestroy")); + // Nor the stack-trace bookkeeping that instrumenting it would have inlined in its place. + assertFalse("freeing a keyed table must leave no trace bookkeeping behind: " + init, + init.contains("keyedTableDestroy in")); // The Jass body must not survive: it would destroy a Table that does not exist here. assertFalse("the Table machinery must not reach Lua: " + init, From bed6d0b056f321757434b3ffbf180e6a85a1df1c Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 17:01:29 +0200 Subject: [PATCH 5/7] Remove the destroy calls instead of leaving them to the inliner. Emptying the function only makes the call removable, not removed: inlining runs only under -inline, and even then the Lua register budget can refuse a caller, so a call to a function that means nothing on Lua survived into an ordinary build. The call sites are rewritten directly now. Arguments move into a statement expression so anything they do still happens, which is the same shape UselessFunctionCallsRemover uses to drop a call it does not need. The regression drops -inline, so it fails if the call ever depends on it again. --- .../imtranslation/LuaNativeLowering.java | 30 +++++++++++++++++++ .../tests/LuaTranslationTests.java | 6 ++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java index 94648d9e7..7f1b841d6 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaNativeLowering.java @@ -140,6 +140,13 @@ public static void lowerKeyedTables(ImProg prog) { } } + // Remove the destroy calls outright rather than leaving an empty function for the inliner + // to clean up: inlining only runs under -inline, and even then the Lua register budget can + // refuse a caller, so a call to a function that means nothing would survive into a normal + // build. Arguments move into a statement expression so anything they do still happens - + // the same shape UselessFunctionCallsRemover uses to drop a call it does not need. + removeDestroyCalls(prog); + Map stubs = new LinkedHashMap<>(); List additions = new ArrayList<>(); prog.accept(new Element.DefaultVisitor() { @@ -165,6 +172,29 @@ public void visit(ImFunctionCall call) { prog.getFunctions().addAll(additions); } + private static void removeDestroyCalls(Element e) { + if (e instanceof ImStmts stmts) { + ListIterator it = stmts.listIterator(); + while (it.hasNext()) { + ImStmt s = it.next(); + if (s instanceof ImFunctionCall call && LuaKeyedTable.isDestroy(call.getFunc())) { + ImStmts argStmts = JassIm.ImStmts(); + for (ImExpr arg : new ArrayList<>(call.getArguments())) { + arg.setParent(null); + argStmts.add(arg); + } + s = ImHelper.statementExprVoid(argStmts); + it.set(s); + } + removeDestroyCalls(s); + } + } else { + for (int i = 0; i < e.size(); i++) { + removeDestroyCalls(e.get(i)); + } + } + } + public static void transform(ImProg prog, ImTranslator translator) { // Replace all reads of MagicFunctions_isLua with true. // This must happen before any optimizer passes so that dead-code elimination diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index 1b90794b0..2ca4a2c36 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -2072,8 +2072,10 @@ public void keyedTableStaysNativeWithStackTraces() throws IOException { @Test public void keyedTableDestroyCostsNothingOnLua() throws IOException { // With stack traces, because that is what a release build emits, and instrumenting an - // emptied function is exactly how the cost comes back. - test().testLua(true).stacktraces().inline().withStdLib().lines(keyedTableSource( + // emptied function is exactly how the cost comes back. Without inlining, because the call + // must not survive a build that never runs the inliner - or one where the Lua register + // budget refuses the caller. + test().testLua(true).stacktraces().withStdLib().lines(keyedTableSource( "package Test", "import KeyedTable", "init", From 24c4f73da83d755d6e3682e0376445760051a00c Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 17:12:34 +0200 Subject: [PATCH 6/7] Keep stack traces out of compiler-owned declarations. Three separate defects on this branch had one cause: stack trace injection rewrites every non-native function on Lua, and the lowerings identify a compiler-owned declaration by its exact signature. An instrumented one carries an extra parameter, so it stops being recognised and its lowering silently does not happen - which is how a keyed set kept its Jass body on Lua, where the key projection is never lowered and every element ended up sharing a key. Fixed where the mutation happens rather than at each thing it broke: the injector now leaves any @compilerintrinsic declaration alone, on both backends. A frame for one says nothing about where a program went wrong anyway, since its body is plumbing or a placeholder a lowering replaces. The rule is also self-enforcing. The injector records the parameter count of every compiler-owned declaration before it runs and fails with a readable error if one differs afterwards, so the next thing to break this cannot break it quietly. --- .../imtranslation/StackTraceInjector2.java | 63 +++++++++++++++++-- .../tests/LuaTranslationTests.java | 43 +++++++++++++ 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java index 29c33ca5d..98fa666fa 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java @@ -1,4 +1,6 @@ package de.peeeq.wurstscript.translation.imtranslation; +import de.peeeq.wurstscript.CompilerIntrinsics; +import de.peeeq.wurstscript.ast.FuncDef; import com.google.common.base.Preconditions; import com.google.common.collect.LinkedListMultimap; @@ -94,6 +96,11 @@ public void visit(ImFuncRef imFuncRef) { } }); + // A compiler-owned declaration has to come out of here exactly as it went in - see + // checkCompilerOwnedUntouched. + Map compilerOwnedArity = new LinkedHashMap<>(); + compilerOwnedFunctions(prog).forEach(f -> compilerOwnedArity.put(f, f.getParameters().size())); + de.peeeq.wurstscript.ast.Element trace = prog.attrTrace(); stackSize = JassIm.ImVar(trace, TypesHelper.imInt(), "wurst_stack_depth", false); prog.getGlobals().add(stackSize); @@ -118,11 +125,7 @@ public void visit(ImFuncRef imFuncRef) { !f.hasFlag(FunctionFlagEnum.IS_NATIVE) && !f.hasFlag(FunctionFlagEnum.IS_BJ) && !f.hasFlag(FunctionFlagEnum.IS_EXTERN) - // Freeing a keyed table means nothing on Lua, so LuaNativeLowering has - // already emptied this one. Instrumenting it would put the cost back: - // a trace parameter at every call site and a push and pop around a body - // that does nothing, which is also what stops the inliner removing it. - && !LuaKeyedTable.isDestroy(f)) + && !isCompilerOwned(f)) .collect(Collectors.toCollection(() -> affectedFuncs)); affectedFuncs.removeAll(configOnlyFuncs); @@ -130,6 +133,7 @@ public void visit(ImFuncRef imFuncRef) { for (ImFunction stackTraceUse : stackTraceGets.keys()) { callRelationTr.get(stackTraceUse).forEach(affectedFuncs::add); } + affectedFuncs.removeIf(StackTraceInjector2::isCompilerOwned); } @@ -140,6 +144,55 @@ public void visit(ImFuncRef imFuncRef) { rewriteErrorStatements(stackTraceGets); rewriteMethodCalls(affectedFuncs); + checkCompilerOwnedUntouched(compilerOwnedArity); + + } + + /** + * Declarations the compiler owns rather than the user. + * + *

These are not instrumented. Their bodies are plumbing or a placeholder that a lowering + * replaces, so a frame for one says nothing about where a program went wrong - and the cost of + * the frame lands on whatever the lowering produced, which on Lua is often meant to be nothing + * at all. + * + *

The stronger reason is that instrumenting one changes its signature. Every lowering + * identifies a compiler-owned declaration by its exact signature, so a function carrying an + * extra trace parameter is no longer recognised, and the lowering silently does not happen. + * On Lua that is not a corner: every non-native function is affected there, and a release + * build emits stack traces by default. + */ + private static boolean isCompilerOwned(ImFunction f) { + return f.attrTrace() instanceof FuncDef fd + && fd.attrHasAnnotation(CompilerIntrinsics.ANNOTATION); + } + + private static Stream compilerOwnedFunctions(ImProg prog) { + return Stream.concat( + prog.getFunctions().stream(), + prog.getClasses().stream().flatMap(c -> c.getFunctions().stream())) + .filter(StackTraceInjector2::isCompilerOwned); + } + + /** + * Fails loudly if a compiler-owned declaration was instrumented after all. + * + *

The failure this guards against is silent by nature: the lowering that should have + * recognised the declaration simply does not fire, and what ships is the unlowered body. That + * cost a correctness bug once already - a keyed set on Lua kept its Jass body, whose key + * projection is never lowered there, so every element shared one key. + */ + private void checkCompilerOwnedUntouched(Map arityBefore) { + for (Map.Entry e : arityBefore.entrySet()) { + int now = e.getKey().getParameters().size(); + if (now != e.getValue()) { + throw new CompileError(e.getKey().attrTrace().attrErrorPos(), + "Stack trace injection changed the signature of the compiler-owned function " + + e.getKey().getName() + " from " + e.getValue() + " to " + now + + " parameters. Lowerings recognise it by that signature and would stop" + + " matching it."); + } + } } private Set getFunctionsReachableFrom(String functionName, Multimap directCalls) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index 2ca4a2c36..7b4851d39 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -2030,6 +2030,49 @@ public void keyedTableLowersToASingleLuaIndex() throws IOException { * *

A release build emits stack traces by default, so this was the common case. */ + /** + * Stack traces must leave a compiler-owned declaration alone, whatever it is. + * + *

Instrumenting one appends a trace parameter, and every lowering identifies such a + * declaration by its exact signature - so an instrumented one stops being recognised and its + * lowering silently does not happen. On Lua that is not a corner: every non-native function is + * affected there, and a release build emits stack traces by default. It cost a correctness bug + * once, when a keyed set kept its Jass body on Lua and every element ended up sharing one key. + * + *

This uses an intrinsic no lowering touches, so what is being checked is the general rule + * rather than the keyed-table lowering that motivated it. + */ + @Test + public void stackTracesLeaveCompilerOwnedDeclarationsAlone() throws IOException { + test().testLua(true).stacktraces().withStdLib().lines( + "package Test", + "@compilerintrinsic public function wurstUntouched(int a, int b) returns int", + " return a + b", + "init", + " print(wurstUntouched(2, 3).toString())", + "endpackage"); + + String compiled = Files.toString( + new File("test-output/lua/LuaTranslationTests_stackTracesLeaveCompilerOwnedDeclarationsAlone.lua"), + Charsets.UTF_8); + + // Present, so the assertions below are about its shape rather than its absence. + assertTrue("the intrinsic should still be emitted", compiled.contains("wurstUntouched")); + + String signature = compiled.substring(compiled.indexOf("function wurstUntouched")); + signature = signature.substring(0, signature.indexOf(")") + 1); + assertFalse("a compiler-owned declaration must not gain a trace parameter: " + signature, + signature.contains("stackPos")); + + String body = getFunctionBody(compiled, "wurstUntouched"); + assertFalse("nor stack bookkeeping in its body: " + body, + body.contains("wurst_stack_depth") || body.contains("wurst_stack[")); + + // The surrounding program is still instrumented, so the test would pass vacuously if + // stack traces were simply off. + assertTrue("stack traces must actually be on", compiled.contains("wurst_stack_depth")); + } + @Test public void keyedTableStaysNativeWithStackTraces() throws IOException { test().testLua(true).stacktraces().inline().withStdLib().lines(keyedTableSource( From d3aec5d0170c2ca9d1990271eaca629295fed52a Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 17:44:03 +0200 Subject: [PATCH 7/7] Exclude compiler-owned declarations however they got into the set. The affected set is seeded from the functions which ask for a stack trace before any filtering happens, so filtering what the traversal adds missed an intrinsic seeded that way. Jass removed those and Lua did not, and on Lua the signature check then failed the build rather than letting the lowering quietly not happen - either way -lua -stacktraces was broken for that input. The removal now happens once, after both branches. --- .../imtranslation/StackTraceInjector2.java | 10 +++++-- .../tests/LuaTranslationTests.java | 29 +++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java index 98fa666fa..a13543479 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java @@ -124,8 +124,7 @@ public void visit(ImFuncRef imFuncRef) { .filter((ImFunction f) -> !f.hasFlag(FunctionFlagEnum.IS_NATIVE) && !f.hasFlag(FunctionFlagEnum.IS_BJ) - && !f.hasFlag(FunctionFlagEnum.IS_EXTERN) - && !isCompilerOwned(f)) + && !f.hasFlag(FunctionFlagEnum.IS_EXTERN)) .collect(Collectors.toCollection(() -> affectedFuncs)); affectedFuncs.removeAll(configOnlyFuncs); @@ -133,10 +132,15 @@ public void visit(ImFuncRef imFuncRef) { for (ImFunction stackTraceUse : stackTraceGets.keys()) { callRelationTr.get(stackTraceUse).forEach(affectedFuncs::add); } - affectedFuncs.removeIf(StackTraceInjector2::isCompilerOwned); } + // After both branches, and after the seeding from stackTraceGets above: a declaration + // the compiler owns is never instrumented, however it came to be in the set. Filtering + // only what each branch adds would miss one seeded there by its own use of a stack trace, + // and would then trip the check below rather than doing nothing. + affectedFuncs.removeIf(StackTraceInjector2::isCompilerOwned); + passStacktraceParams(calls, affectedFuncs); addStackTracePush(calls, affectedFuncs); addStackTracePop(affectedFuncs); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java index 7b4851d39..c4702f331 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java @@ -2073,6 +2073,35 @@ public void stackTracesLeaveCompilerOwnedDeclarationsAlone() throws IOException assertTrue("stack traces must actually be on", compiled.contains("wurst_stack_depth")); } + /** + * The rule has to hold for an intrinsic which asks for a stack trace itself. + * + *

Such a function is seeded into the affected set before any filtering, so excluding only + * what the traversal adds would leave it instrumented - and the signature check would then + * fail the build rather than let the lowering quietly not happen. Either way `-lua + * -stacktraces` would be broken for this input. + */ + @Test + public void aCompilerOwnedDeclarationUsingAStackTraceIsStillLeftAlone() throws IOException { + test().testLua(true).stacktraces().withStdLib().lines( + "package Test", + "@compilerintrinsic public function wurstTraced(int a) returns string", + " return getStackTraceString() + a.toString()", + "init", + " print(wurstTraced(1))", + "endpackage"); + + String compiled = Files.toString( + new File("test-output/lua/LuaTranslationTests_aCompilerOwnedDeclarationUsingAStackTraceIsStillLeftAlone.lua"), + Charsets.UTF_8); + + String signature = compiled.substring(compiled.indexOf("function wurstTraced")); + signature = signature.substring(0, signature.indexOf(")") + 1); + assertFalse("a compiler-owned declaration must not gain a trace parameter: " + signature, + signature.contains("stackPos")); + assertTrue("stack traces must actually be on", compiled.contains("wurst_stack_depth")); + } + @Test public void keyedTableStaysNativeWithStackTraces() throws IOException { test().testLua(true).stacktraces().inline().withStdLib().lines(keyedTableSource(