Keep stack traces out of compiler-owned declarations

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

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedTable.java

@@ -36,6 +36,7 @@ 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";
@@ -69,6 +70,24 @@ public static String nativeStubFor(ImFunction f) {
         };
     }
 
+    /**
+     * Whether {@code f} frees a keyed table.
+     *
+     * <p>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

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.
+     *
+     * <p>Separate from {@link #transform} so it can run <b>before</b> 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.
+     *
+     * <p>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.
+     *
+     * <p>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<String, ImFunction> stubs = new LinkedHashMap<>();
+        List<ImFunction> 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);
+    }
+
+    private static void removeDestroyCalls(Element e) {
+        if (e instanceof ImStmts stmts) {
+            ListIterator<ImStmt> 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
@@ -125,6 +210,9 @@ public static void transform(ImProg prog, ImTranslator translator) {
             }
         }
 
+        // Idempotent: transformProgToLua runs this earlier, before stack-trace injection.
+        lowerKeyedTables(prog);
+
         lowerStringConcatenation(prog, translator);
         lowerDivMod(prog, translator);
 
@@ -146,25 +234,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",

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<ImFunction, Integer> 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);
@@ -128,13 +135,68 @@ public void visit(ImFuncRef imFuncRef) {
         }
 
 
+        // 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);
         rewriteFuncRefs(funcRefs, affectedFuncs);
         rewriteErrorStatements(stackTraceGets);
         rewriteMethodCalls(affectedFuncs);
 
+        checkCompilerOwnedUntouched(compilerOwnedArity);
+
+    }
+
+    /**
+     * Declarations the compiler owns rather than the user.
+     *
+     * <p>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.
+     *
+     * <p>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<ImFunction> 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.
+     *
+     * <p>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<ImFunction, Integer> arityBefore) {
+        for (Map.Entry<ImFunction, Integer> 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<ImFunction> getFunctionsReachableFrom(String functionName, Multimap<ImFunction, ImFunction> directCalls) {