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..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 @@ -110,6 +110,91 @@ 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();
+ }
+ }
+
+ // 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 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 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 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.
+ */
+ /**
+ * 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"));
+ }
+
+ /**
+ * 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(
+ "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.
+ *
+ * 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 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. 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",
+ " let t = keyedTableCreate()",
+ " keyedTableAdd(t, 7)",
+ " keyedTableDestroy(t)",
+ "endpackage"));
+
+ String compiled = Files.toString(
+ new File("test-output/lua/LuaTranslationTests_keyedTableDestroyCostsNothingOnLua.lua"),
+ Charsets.UTF_8);
+
+ assertFalse("destroy must not become a native stub, which the inliner cannot remove",
+ compiled.contains("__wurst_keyedTableDestroy"));
+
+ 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,
+ init.contains("FlushChildHashtable") || init.contains("Table_destroy"));
+ }
+
/**
* 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.
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);
}