From af1ca5a85289ae1d7909fd772d11b3981210e2d3 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 01:01:13 +0200 Subject: [PATCH 01/12] Lua: write field defaults as IM so the constructor's writes replace them Every `new` on Lua wrote each field twice: the allocation (`X:createN()`, Lua built after the optimiser) set every field to its default, then the inlined constructor set the same fields again; a damage event wrote the 11 fields of DamageInstance 22 times. LuaFieldDefaults now writes the scalar defaults as IM statements after each allocation, once the inlining has put the constructor's writes next to it. The new local pass RedundantFieldStores removes a constant written to a field which the same statement list writes again before any read of that field (through any object), any call, anything which can stop the thread, exitwhen, return or a nested statement list. Before the backend, the defaults which still follow an allocation (the run of writes right after it, each setting a field of the object to exactly its default) move back into that class's shared allocation function, so a field leaves the allocation only where every allocation overwrote it: no construction writes more than before and the script does not grow. On Jass the class elimination has turned fields into arrays before the local optimisations, so only Lua changes. --- .../peeeq/wurstio/WurstCompilerJassImpl.java | 3 + .../optimizer/RedundantFieldStores.java | 165 ++++++++++++++ .../translation/imoptimizer/ImOptimizer.java | 4 + .../imtranslation/ImTranslator.java | 11 + .../imtranslation/LuaFieldDefaults.java | 211 ++++++++++++++++++ .../lua/translation/LuaTranslator.java | 45 ++-- .../tests/LuaBackendAuditTests.java | 93 ++++++++ .../tests/LuaTranslationTests.java | 5 +- .../wurstscript/tests/OptimizerTests.java | 2 +- 9 files changed, 506 insertions(+), 33 deletions(-) create mode 100644 de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java create mode 100644 de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaFieldDefaults.java 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 e052b0217..79d9cb8f1 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 @@ -1034,6 +1034,9 @@ public LuaCompilationUnit transformProgToLua() { optimizer.removeGarbage(); imProg.flatten(imTranslator); timeTaker.endPhase(); + // After the inlining an allocation is followed by the constructor's writes, so the local optimizations can + // drop the defaults those overwrite. The backend no longer writes them, so this runs in every mode. + LuaFieldDefaults.materialize(imProg, imTranslator2); stage = 10; if (runArgs.isLocalOptimizations()) { beginPhase(10, "local optimizations"); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java new file mode 100644 index 000000000..fc7f25726 --- /dev/null +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java @@ -0,0 +1,165 @@ +package de.peeeq.wurstscript.intermediatelang.optimizer; + +import de.peeeq.wurstscript.jassIm.*; +import de.peeeq.wurstscript.translation.imoptimizer.OptimizerPass; +import de.peeeq.wurstscript.translation.imtranslation.Flatten; +import de.peeeq.wurstscript.translation.imtranslation.ImHelper; +import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; + +import java.util.*; + +/** + * Removes a write of a constant to a field which the same statement list writes again before anything can read + * it. + *

+ * Between the two writes there must be no read of the field through any object (so an alias cannot see the first + * value), no call (a callee can read anything, a native can run Wurst code through an event), nothing which can stop + * the thread or leave the list ({@code exitwhen}, {@code return}: the first value would then be what remains) and no + * statement with statement lists of its own (whose paths are not followed). The object is told by value: a local copied from another names the same object until + * either is assigned again. An allocation reads no field and runs no Wurst code, so it is not in the way. + *

+ * This removes the defaults an allocation writes on Lua where the constructor sets the field + * ({@code LuaFieldDefaults}). Fields are only still fields on Lua: on Jass the class elimination has turned them into + * arrays before the local optimisations run. + */ +public class RedundantFieldStores implements OptimizerPass { + + private ImTranslator trans; + private int removed; + + @Override + public int optimize(ImTranslator trans) { + this.trans = trans; + removed = 0; + for (ImFunction func : ImHelper.calculateFunctionsOfProg(trans.getImProg())) { + if (!func.isNative() && !func.isBj()) { + optimizeStatements(func.getBody()); + } + } + return removed; + } + + @Override + public String getName() { + return "Redundant field stores removed"; + } + + /** A field, by its storage, of the object a value number stands for. */ + private record Slot(ImVar field, int object) { + } + + private void optimizeStatements(ImStmts stmts) { + Map objectOf = new IdentityHashMap<>(); + int[] nextObject = {0}; + Map pending = new HashMap<>(); + Set dead = Collections.newSetFromMap(new IdentityHashMap<>()); + for (ImStmt s : stmts) { + if (hasStatementLists(s)) { + // Its paths can assign the locals, so their values are told apart afresh after it. + pending.clear(); + objectOf.clear(); + s.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImStmts nested) { + optimizeStatements(nested); + } + }); + continue; + } + if (isBarrier(s)) { + pending.clear(); + } + if (s instanceof ImSet set) { + forgetReadFields(set.getRight(), pending); + if (set.getLeft() instanceof ImMemberAccess target) { + forgetReadFields(target.getReceiver(), pending); + forgetReadFields(target.getIndexes(), pending); + if (target.getIndexes().isEmpty() + && target.getReceiver() instanceof ImVarAccess receiver + && !receiver.getVar().isGlobal()) { + Slot slot = new Slot(trans.canonical(target.getVar()), + objectOf.computeIfAbsent(receiver.getVar(), v -> nextObject[0]++)); + ImSet earlier = pending.remove(slot); + if (earlier != null) { + dead.add(earlier); + } + if (isConstant(set.getRight())) { + pending.put(slot, set); + } + } + } else { + forgetReadFields(set.getLeft(), pending); + if (set.getLeft() instanceof ImVarAccess assigned && !assigned.getVar().isGlobal()) { + if (set.getRight() instanceof ImVarAccess copied && !copied.getVar().isGlobal()) { + objectOf.put(assigned.getVar(), + objectOf.computeIfAbsent(copied.getVar(), v -> nextObject[0]++)); + } else { + objectOf.put(assigned.getVar(), nextObject[0]++); + } + } + } + } else { + forgetReadFields(s, pending); + } + } + if (!dead.isEmpty()) { + stmts.removeIf(dead::contains); + removed += dead.size(); + } + } + + private static boolean hasStatementLists(ImStmt s) { + return s instanceof ImIf || s instanceof ImLoop || s instanceof ImVarargLoop; + } + + /** + * Whether running {@code s} can read any field or leave the list with the first value in place: a call, an + * operation which can stop the thread, or an {@code exitwhen} or {@code return}. + */ + private static boolean isBarrier(ImStmt s) { + if (s instanceof ImExitwhen || s instanceof ImReturn) { + return true; + } + boolean[] barrier = {false}; + s.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImFunctionCall call) { + barrier[0] = true; + } + + @Override + public void visit(ImMethodCall call) { + barrier[0] = true; + } + + @Override + public void visit(ImOperatorCall call) { + super.visit(call); + if (Flatten.mayStopTheThread(call)) { + barrier[0] = true; + } + } + }); + return barrier[0]; + } + + /** Forgets the pending writes of every field {@code e} reads, through whatever object. */ + private void forgetReadFields(Element e, Map pending) { + if (pending.isEmpty()) { + return; + } + e.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImMemberAccess access) { + super.visit(access); + ImVar field = trans.canonical(access.getVar()); + pending.keySet().removeIf(slot -> slot.field() == field); + } + }); + } + + private static boolean isConstant(ImExpr e) { + return e instanceof ImIntVal || e instanceof ImRealVal || e instanceof ImBoolVal + || e instanceof ImStringVal || e instanceof ImNull; + } +} diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java index 77bb2815d..d59365963 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java @@ -9,6 +9,7 @@ import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerAwareOptimizerPass; import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerContextAnalyzer; import de.peeeq.wurstscript.intermediatelang.optimizer.LocalMerger; +import de.peeeq.wurstscript.intermediatelang.optimizer.RedundantFieldStores; import de.peeeq.wurstscript.intermediatelang.optimizer.SideEffectAnalyzer; import de.peeeq.wurstscript.intermediatelang.optimizer.SimpleRewrites; import de.peeeq.wurstscript.jassIm.*; @@ -37,6 +38,9 @@ public class ImOptimizer { localPasses.add(new BranchMerger()); localPasses.add(new ConstantAndCopyPropagation()); localPasses.add(new UselessFunctionCallsRemover()); + // After the passes which use the local-player analysis: a pass which does not use it discards it, and none + // after this one needs it again in the same sweep. + localPasses.add(new RedundantFieldStores()); localPasses.add(new GlobalsInliner()); localPasses.add(new DispatchCheckDeduplicator()); localPasses.add(new SimpleRewrites()); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java index 2e715bb35..d9a9a57ab 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java @@ -465,6 +465,17 @@ private record Bridge(ImMethod bridge, ImMethod inherited) { private final List bridges = new ArrayList<>(); + /** Whether {@link LuaFieldDefaults} wrote the field defaults after the allocations, which the Lua backend needs. */ + private boolean luaFieldDefaultsMaterialized = false; + + public boolean luaFieldDefaultsMaterialized() { + return luaFieldDefaultsMaterialized; + } + + public void setLuaFieldDefaultsMaterialized() { + luaFieldDefaultsMaterialized = true; + } + /** * Gives {@code bridge} the overrides of {@code inherited} which are below its class as sub-methods, once every * class is translated: the sub-methods of {@code inherited} come from the translation of its class, which may come diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaFieldDefaults.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaFieldDefaults.java new file mode 100644 index 000000000..a70c1394a --- /dev/null +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaFieldDefaults.java @@ -0,0 +1,211 @@ +package de.peeeq.wurstscript.translation.imtranslation; + +import de.peeeq.wurstscript.jassIm.*; +import de.peeeq.wurstscript.types.TypesHelper; + +import java.util.*; + +/** + * Writes the default of each field of a new object on Lua, as IM statements after its allocation, so that the + * optimiser sees the ones the constructor overwrites. + *

+ * Lua keeps a field in a table indexed by the object id. A slot nothing wrote reads as nil, where Jass reads 0, + * and an id is reused, so a new object's fields are set to their defaults. The backend's allocation used to write + * them all, out of the optimiser's sight, so a construction wrote each field the constructor sets twice: a damage + * event wrote the 11 fields of DamageInstance 22 times. + *

+ * {@link #materialize} runs after the inlining, when an allocation is followed by the constructor's own writes, and + * the local optimisations remove a default which is written again before anything can read it + * ({@code RedundantFieldStores}). {@link #moveSurvivorsToAllocations} runs before the backend: a default which + * survives at some allocation of a class goes back into that class's shared allocation function, and its copies + * after the allocations are removed, so a field leaves the allocation only where every allocation overwrote it, and + * no construction writes more than before nor is the script bigger. Without the local optimisations every default + * goes back; only the allocation function of a class nothing allocates (an abstract base), which nothing calls, is + * left without them. + *

+ * An array field gets a table of its own per object, which IM cannot express; the backend's allocation always makes + * those ({@link #isArrayField}). + */ +public final class LuaFieldDefaults { + + private LuaFieldDefaults() { + } + + /** Inserts the defaults after every allocation and returns how many allocations there were. */ + public static int materialize(ImProg prog, ImTranslator tr) { + List allocations = new ArrayList<>(); + prog.accept(new ImProg.DefaultVisitor() { + @Override + public void visit(ImAlloc alloc) { + super.visit(alloc); + if (!(alloc.getParent() instanceof ImSet set) + || set.getRight() != alloc + || !(set.getLeft() instanceof ImVarAccess) + || !(set.getParent() instanceof ImStmts)) { + throw new IllegalStateException("An allocation of " + alloc.getClazz() + + " which is not assigned to a variable as a statement: " + alloc.getParent()); + } + allocations.add(set); + } + }); + for (ImSet allocation : allocations) { + ImAlloc alloc = (ImAlloc) allocation.getRight(); + ImVar object = ((ImVarAccess) allocation.getLeft()).getVar(); + ImStmts stmts = (ImStmts) allocation.getParent(); + int index = stmts.indexOf(allocation); + List defaults = new ArrayList<>(); + for (ImVar field : fieldsOf(alloc.getClazz().getClassDef(), tr)) { + if (isArrayField(field)) { + continue; + } + ImTypeArguments typeArguments = alloc.getClazz().getTypeArguments().copy(); + defaults.add(JassIm.ImSet(allocation.getTrace(), + JassIm.ImMemberAccess(allocation.getTrace(), JassIm.ImVarAccess(object), typeArguments, field, + JassIm.ImExprs()), + defaultValue(field.getType()))); + } + stmts.addAll(index + 1, defaults); + } + tr.setLuaFieldDefaultsMaterialized(); + return allocations.size(); + } + + /** + * Removes the defaults which still follow an allocation and returns, for each class allocated, the fields (by + * storage) whose default survived at one of its allocations, which its allocation function writes instead. + *

+ * A surviving default is found where {@link #materialize} put it: in the run of writes right after the + * allocation, each setting a field of the object to exactly its default. Moving such a write into the + * allocation, which runs where the allocation statement is, changes nothing: nothing runs between them, + * whoever wrote it. At another allocation of the class, where the optimiser removed that default because the + * field is set before anything reads it, the allocation's write is dead, which is what every allocation did + * before. + */ + public static Map> moveSurvivorsToAllocations(ImProg prog, ImTranslator tr) { + if (!tr.luaFieldDefaultsMaterialized()) { + throw new IllegalStateException("The field defaults were not written after the allocations " + + "(LuaFieldDefaults.materialize), so the allocations would leave fields nil."); + } + Map> written = new LinkedHashMap<>(); + List survivors = new ArrayList<>(); + prog.accept(new ImProg.DefaultVisitor() { + @Override + public void visit(ImStmts stmts) { + super.visit(stmts); + for (int i = 0; i < stmts.size(); i++) { + if (!(stmts.get(i) instanceof ImSet set) || !(set.getRight() instanceof ImAlloc alloc) + || !(set.getLeft() instanceof ImVarAccess object)) { + continue; + } + ImClass c = alloc.getClazz().getClassDef(); + Map fields = new IdentityHashMap<>(); + for (ImVar field : fieldsOf(c, tr)) { + if (!isArrayField(field)) { + fields.put(tr.canonical(field), field); + } + } + for (int j = i + 1; j < stmts.size(); j++) { + ImVar field = defaultWrittenTo(stmts.get(j), object.getVar(), fields, tr); + if (field == null) { + break; + } + written.computeIfAbsent(c, k -> Collections.newSetFromMap(new IdentityHashMap<>())) + .add(field); + survivors.add(stmts.get(j)); + } + } + } + }); + for (ImStmt survivor : survivors) { + ((ImStmts) survivor.getParent()).remove(survivor); + } + return written; + } + + /** The field (by storage) which {@code s} sets to exactly its default on {@code object}, or null. */ + private static @org.eclipse.jdt.annotation.Nullable ImVar defaultWrittenTo(ImStmt s, ImVar object, + Map fields, + ImTranslator tr) { + if (!(s instanceof ImSet set) || !(set.getLeft() instanceof ImMemberAccess target) + || !target.getIndexes().isEmpty() + || !(target.getReceiver() instanceof ImVarAccess receiver) || receiver.getVar() != object) { + return null; + } + ImVar storage = tr.canonical(target.getVar()); + ImVar field = fields.get(storage); + if (field == null || !isDefault(set.getRight(), field.getType())) { + return null; + } + return storage; + } + + private static boolean isDefault(ImExpr e, ImType type) { + if (type instanceof ImSimpleType simpleType) { + if (TypesHelper.isIntType(simpleType)) { + return e instanceof ImIntVal v && v.getValI() == 0; + } else if (TypesHelper.isBoolType(simpleType)) { + return e instanceof ImBoolVal v && !v.getValB(); + } else if (TypesHelper.isRealType(simpleType)) { + if (!(e instanceof ImRealVal v)) { + return false; + } + double value = Double.parseDouble(v.getValR()); + return value == 0 && Double.doubleToRawLongBits(value) == 0; + } else if (TypesHelper.isStringType(simpleType)) { + return e instanceof ImStringVal v && v.getValS().isEmpty(); + } + } + return e instanceof ImNull; + } + + /** + * The fields an object of {@code c} has: those of its superclasses, in the order of their names, then its own, + * each storage once ({@link ImTranslator#canonical}). + */ + public static List fieldsOf(ImClass c, ImTranslator tr) { + List result = new ArrayList<>(); + collectFields(c, tr, result, Collections.newSetFromMap(new IdentityHashMap<>()), + Collections.newSetFromMap(new IdentityHashMap<>())); + return result; + } + + private static void collectFields(ImClass c, ImTranslator tr, List out, Set visitedClasses, + Set visitedFields) { + if (!visitedClasses.add(c)) { + return; + } + List superClasses = new ArrayList<>(c.getSuperClasses()); + superClasses.sort(Comparator.comparing(sc -> sc.getClassDef().getName())); + for (ImClassType sc : superClasses) { + collectFields(sc.getClassDef(), tr, out, visitedClasses, visitedFields); + } + for (ImVar field : c.getFields()) { + if (visitedFields.add(tr.canonical(field))) { + out.add(field); + } + } + } + + /** An array field, whose default is a table of its own per object, which the backend makes. */ + public static boolean isArrayField(ImVar field) { + return field.getType() instanceof ImArrayType || field.getType() instanceof ImArrayTypeMulti; + } + + /** The value a field of {@code type} holds before anything writes it, as the Lua backend prints defaults. */ + private static ImExpr defaultValue(ImType type) { + if (type instanceof ImSimpleType simpleType) { + if (TypesHelper.isIntType(simpleType)) { + return JassIm.ImIntVal(0); + } else if (TypesHelper.isBoolType(simpleType)) { + return JassIm.ImBoolVal(false); + } else if (TypesHelper.isRealType(simpleType)) { + return JassIm.ImRealVal("0."); + } else if (TypesHelper.isStringType(simpleType)) { + return JassIm.ImStringVal(""); + } + } else if (type instanceof ImTupleType) { + throw new IllegalStateException("A tuple field after the tuples were eliminated: " + type); + } + return JassIm.ImNull(type); + } +} diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java index 741a324ba..2474994de 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java @@ -8,6 +8,7 @@ import de.peeeq.wurstscript.translation.imtranslation.GetAForB; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; +import de.peeeq.wurstscript.translation.imtranslation.LuaFieldDefaults; import de.peeeq.wurstscript.translation.imtranslation.GenericTypes; import de.peeeq.wurstscript.translation.imtranslation.LuaDispatchPreparation; import de.peeeq.wurstscript.translation.imtranslation.LuaNativeLowering; @@ -383,6 +384,7 @@ protected String uniqueName(String rawName) { public LuaCompilationUnit translate() { collectPredefinedNames(); assertNoDanglingFunctionReferences(prog); + fieldDefaultsOnAllocation = LuaFieldDefaults.moveSurvivorsToAllocations(prog, imTr); normalizeFieldNames(); @@ -748,6 +750,9 @@ private void createInstanceOfFunction() { LuaPolyfillSetup.createInstanceOfFunction(this); } + /** For each class, the fields whose default its allocation writes ({@link LuaFieldDefaults}). */ + private Map> fieldDefaultsOnAllocation = Collections.emptyMap(); + LuaVariable fieldStorage(ImVar field) { return luaFieldStorage.getFor(imTr.canonical(field)); } @@ -1360,38 +1365,18 @@ private void createClassInitFunction(ImClass c, LuaVariable classVar, LuaMethod LuaAst.LuaExprArrayAccess(LuaAst.LuaExprVarAccess(objectClass), LuaAst.LuaExprlist(LuaAst.LuaExprVarAccess(newInst))), LuaAst.LuaExprVarAccess(classVar))); - for (ImVar field : collectFieldsForAllocation(c)) { - body.add(LuaAst.LuaAssignment( - LuaAst.LuaExprArrayAccess(LuaAst.LuaExprVarAccess(fieldStorage(field)), - LuaAst.LuaExprlist(LuaAst.LuaExprVarAccess(newInst))), - defaultValue(field.getType()))); - } - body.add(LuaAst.LuaReturn(LuaAst.LuaExprVarAccess(newInst))); - } - - private List collectFieldsForAllocation(ImClass c) { - List result = new ArrayList<>(); - Set visitedClasses = Collections.newSetFromMap(new IdentityHashMap<>()); - Set visitedFields = Collections.newSetFromMap(new IdentityHashMap<>()); - collectFieldsForAllocation(c, result, visitedClasses, visitedFields); - return result; - } - - private void collectFieldsForAllocation(ImClass c, List out, - Set visitedClasses, Set visitedFields) { - if (!visitedClasses.add(c)) { - return; - } - List superClasses = new ArrayList<>(c.getSuperClasses()); - superClasses.sort(Comparator.comparing(sc -> classSortKey(sc.getClassDef()))); - for (ImClassType sc : superClasses) { - collectFieldsForAllocation(sc.getClassDef(), out, visitedClasses, visitedFields); - } - for (ImVar field : c.getFields()) { - if (visitedFields.add(imTr.canonical(field))) { - out.add(field); + // The defaults the optimiser did not remove at some allocation of this class (LuaFieldDefaults), and the + // table an array field needs per object, which only the backend can make. + Set survivingDefaults = fieldDefaultsOnAllocation.getOrDefault(c, Collections.emptySet()); + for (ImVar field : LuaFieldDefaults.fieldsOf(c, imTr)) { + if (LuaFieldDefaults.isArrayField(field) || survivingDefaults.contains(imTr.canonical(field))) { + body.add(LuaAst.LuaAssignment( + LuaAst.LuaExprArrayAccess(LuaAst.LuaExprVarAccess(fieldStorage(field)), + LuaAst.LuaExprlist(LuaAst.LuaExprVarAccess(newInst))), + defaultValue(field.getType()))); } } + body.add(LuaAst.LuaReturn(LuaAst.LuaExprVarAccess(newInst))); } private void initClassTables(ImClass c) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index ec02e8aa7..7b65908c2 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -7153,4 +7153,97 @@ public void closureMethodsWithUnderscoresKeepTheirSemanticNames() throws IOExcep luaFunctionsWithPrefix(lua, "Under_pass")); assertFalse(lua, lua.contains("passUnder_passUnder")); } + + /** + * A new object's field defaults are IM writes after its allocation, so where the constructor sets a field the + * default is gone: the allocation writes no field, and each field is written once where the object is made. + */ + @Test + public void aConstructedFieldIsWrittenOnceNotFirstWithItsDefault() throws IOException { + test().testLua(true).inline().localOptimizations().executeProg().lines( + "package Test", + "native testSuccess()", + "class Hit", + " int amount", + " real factor", + " boolean crit", + " string label", + " Hit next", + " construct(int amount, real factor)", + " this.amount = amount", + " this.factor = factor", + " this.crit = amount > 10", + " this.label = \"hit\"", + " this.next = null", + "@noinline function make(int amount) returns Hit", + " return new Hit(amount, 1.5)", + "init", + " let h = make(12)", + " if h.amount == 12 and h.factor == 1.5 and h.crit and h.label == \"hit\" and h.next == null", + " testSuccess()"); + String compiled = compiledLua("aConstructedFieldIsWrittenOnceNotFirstWithItsDefault"); + java.util.regex.Matcher create = java.util.regex.Pattern + .compile("function Hit:create\\d*\\(\\) \\n(.*?)\\nend", java.util.regex.Pattern.DOTALL).matcher(compiled); + assertTrue("the allocation:\n" + compiled, create.find()); + assertFalse("the allocation writes no field:\n" + create.group(1), create.group(1).contains("_storage[")); + for (String field : new String[] {"amount", "factor", "crit", "label", "next"}) { + int writes = compiled.split("Hit_" + field + "_storage(_local\\d*)?\\[[^\\]]+\\] = ", -1).length - 1; + assertEquals("Hit." + field + " is written once:\n" + compiled, 1, writes); + } + } + + /** + * A field nothing sets before reading it still starts at its default, also on an id which an earlier object + * had: here the constructor reads count first, and the earlier object had set every field. + */ + @Test + public void aFieldReadBeforeItIsSetStartsAtItsDefaultOnAReusedId() { + String[] lines = { + "package Test", + "native testSuccess()", + "class A", + " int count", + " string name", + " A other", + " construct()", + " count += 1", + "init", + " let a = new A()", + " a.count = 41", + " a.name = \"x\"", + " a.other = a", + " destroy a", + " let b = new A()", + " if b.count == 1 and b.name == \"\" and b.other == null", + " testSuccess()"}; + test().testLua(true).inline().localOptimizations().executeProg().lines(lines); + testNamed("aFieldReadBeforeItIsSetStartsAtItsDefaultOnAReusedId_unoptimised").testLua(true).executeProg() + .lines(lines); + } + + /** + * A call between the allocation and the constructor's write can read the field, so the default before it stays: + * the observer sees 0 on an id whose earlier object had set the field to 7. + */ + @Test + public void aDefaultACallCanReadBeforeTheConstructorSetsItStays() { + test().testLua(true).inline().localOptimizations().executeProg().lines( + "package Test", + "native testSuccess()", + "int seen = -1", + "class A", + " int f", + " construct()", + " observe(this)", + " f = 5", + "@noinline function observe(A a)", + " seen = a.f", + "init", + " let a = new A()", + " a.f = 7", + " destroy a", + " let b = new A()", + " if seen == 0 and b.f == 5", + " testSuccess()"); + } } 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 d0912476d..d6cd26f1c 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 @@ -3979,10 +3979,11 @@ public void subclassAllocationIncludesInheritedFieldsInLua() throws IOException ); String compiled = Files.toString(new File("test-output/lua/LuaTranslationTests_subclassAllocationIncludesInheritedFieldsInLua.lua"), Charsets.UTF_8); + // Within Child's own allocation function (Window is never allocated, so its allocation writes nothing). assertContainsRegex(compiled, - "function\\s+[A-Za-z0-9_]+:create\\d+\\s*\\(\\)[\\s\\S]*?Window_anchorTop_storage\\[new_inst\\] = 0"); + "function\\s+Child:create\\d*\\s*\\(\\)(?:(?!\\nend)[\\s\\S])*?Window_anchorTop_storage\\[new_inst\\] = 0"); assertContainsRegex(compiled, - "function\\s+[A-Za-z0-9_]+:create\\d+\\s*\\(\\)[\\s\\S]*?Window_anchorBottom_storage\\[new_inst\\] = 0"); + "function\\s+Child:create\\d*\\s*\\(\\)(?:(?!\\nend)[\\s\\S])*?Window_anchorBottom_storage\\[new_inst\\] = 0"); } // ----- GetHandleId remapping ----- diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 93e3c1b58..797bf4f48 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -2113,7 +2113,7 @@ public T measure(String name, java.util.function.Supplier f) { new ImOptimizer(timeTaker, translator).localOptimizations(); - assertEquals(timeTaker.measurements, 16, + assertEquals(timeTaker.measurements, 18, "the optimizer should run two fixed sweeps rather than iterating to convergence"); } From 336f31ac5d47a423dbbd4f6bc801422c448d93bb Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 01:04:48 +0200 Subject: [PATCH 02/12] Lua: keep the object runtime state in main-chunk locals The emitted script declared no top-level locals, so every allocation, destroy and old-generics cast looked its state up among the globals. - __wurst_objectClass, __wurst_objectFree, __wurst_objectMax and __wurst_objectFreeCount are main-chunk locals at the very top of the script, and __wurst_deallocObject is a local function there, so every function reaches them as upvalues. A new LuaChunkLocal node prints a definition as a local; the translator inserts them before everything else (declareChunkLocal), at most six main-chunk locals. - The descriptor map is no longer aliased into function locals for loops: an upvalue table is indexed in one instruction (GETTABUP), as a local one is, so the alias only added a copy per call. - __wurst_oldGenericsZero is a main-chunk local too, still initialised from math.mininteger. - Allocation no longer clears the popped free-stack slot: a slot above the count is never read before a destroy writes it again. - A callback adapter of a target without parameters takes no varargs. --- de.peeeq.wurstscript/parserspec/lua.parseq | 1 + .../translation/lua/printing/LuaPrinter.java | 25 +++ .../lua/translation/LuaPolyfillSetup.java | 8 +- .../lua/translation/LuaTranslator.java | 55 ++++-- .../tests/LuaBackendAuditTests.java | 178 +++++++++++++++++- .../tests/LuaTranslationTests.java | 13 +- .../wurstscript/tests/WurstScriptTest.java | 6 +- 7 files changed, 255 insertions(+), 31 deletions(-) diff --git a/de.peeeq.wurstscript/parserspec/lua.parseq b/de.peeeq.wurstscript/parserspec/lua.parseq index 7bdbfae19..ef596655f 100644 --- a/de.peeeq.wurstscript/parserspec/lua.parseq +++ b/de.peeeq.wurstscript/parserspec/lua.parseq @@ -18,6 +18,7 @@ LuaStatements * LuaStatement LuaStatement = LuaDefinition + | LuaChunkLocal(LuaDefinition definition) // a definition printed as a local of the main chunk | LuaAssignment(LuaExpr left, LuaExpr right) | LuaWhile(LuaExpr cond, LuaStatements body) | LuaFor(ref LuaVariable loopVar, LuaExpr from, LuaExpr to, LuaExprOpt step, LuaStatements body) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/printing/LuaPrinter.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/printing/LuaPrinter.java index 802065013..1b7062799 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/printing/LuaPrinter.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/printing/LuaPrinter.java @@ -26,6 +26,10 @@ public static void print(LuaCompilationUnit cu, StringBuilder sb, int indent) { printVariable(luaVariable, sb, indent); sb.append("\n"); statementBlock = true; + } else if (d instanceof LuaChunkLocal local && local.getDefinition() instanceof LuaVariable) { + d.print(sb, indent); + sb.append("\n"); + statementBlock = true; } else if(d instanceof LuaAssignment) { // these are top level assignments that are not inside functions d.print(sb, indent); @@ -275,6 +279,27 @@ public static void print(LuaExprVarAccess e, StringBuilder sb, int indent) { public static void print(LuaFunction f, StringBuilder sb, int indent) { printIndent(sb, indent); + printFunction(f, sb, indent); + } + + /** + * A local of the main chunk. A function printed after it reaches it as an upvalue, one + * instruction, instead of looking its name up in _ENV, the table of every global. + */ + public static void print(LuaChunkLocal l, StringBuilder sb, int indent) { + LuaDefinition definition = l.getDefinition(); + if (definition instanceof LuaVariable v) { + print(v, sb, indent); + } else if (definition instanceof LuaFunction f) { + printIndent(sb, indent); + sb.append("local "); + printFunction(f, sb, indent); + } else { + throw new IllegalArgumentException("A method is a table field and cannot be a local: " + definition); + } + } + + private static void printFunction(LuaFunction f, StringBuilder sb, int indent) { sb.append("function "); sb.append(f.getName()); sb.append("("); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaPolyfillSetup.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaPolyfillSetup.java index 01c4ba272..f6031fdfd 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaPolyfillSetup.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaPolyfillSetup.java @@ -94,10 +94,14 @@ static void createObjectIndexFunctions(LuaTranslator tr) { } } - /** The sentinel an old-generics int value 0 is stored as (see ExprTranslation.translate(ImCast)). */ + /** + * The sentinel an old-generics int value 0 is stored as (see ExprTranslation.translate(ImCast)), + * read by every such cast, so a main-chunk local. It stays {@code math.mininteger} rather than a + * literal: the integer width of the game's Lua need not be the test Lua's 64 bits. + */ static LuaVariable createOldGenericsZero(LuaTranslator tr) { LuaVariable zero = LuaAst.LuaVariable("__wurst_oldGenericsZero", LuaAst.LuaLiteral("math.mininteger")); - tr.luaModel.add(zero); + tr.declareChunkLocal(zero); return zero; } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java index 2474994de..9725279b4 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java @@ -456,18 +456,23 @@ LuaFunction callbackAdapterFor(ImFunction target) { } LuaFunction targetLua = luaFunc.getFor(target); - LuaVariable dots = LuaAst.LuaVariable("...", LuaAst.LuaNoExpr()); LuaFunction adapter = LuaAst.LuaFunction( uniqueName("__wurst_callback_" + targetLua.getName()), - LuaAst.LuaParams(dots), LuaAst.LuaStatements()); + LuaAst.LuaParams(), LuaAst.LuaStatements()); callbackAdapters.put(target, adapter); LuaFunction errorHandler = callbackErrorHandler(); - LuaExprFunctionCallByName xpcall = LuaAst.LuaExprFunctionCallByName("xpcall", - LuaAst.LuaExprlist( - LuaAst.LuaExprFuncRef(targetLua), - LuaAst.LuaExprFuncRef(errorHandler), - LuaAst.LuaExprVarAccess(dots.copy()))); + LuaExprlist xpcallArgs = LuaAst.LuaExprlist( + LuaAst.LuaExprFuncRef(targetLua), + LuaAst.LuaExprFuncRef(errorHandler)); + // A Wurst code value takes no parameters, so whatever the caller passes is dropped by the + // target anyway: such an adapter takes no varargs. Only a target with parameters needs them. + if (!targetLua.getParams().isEmpty()) { + LuaVariable dots = LuaAst.LuaVariable("...", LuaAst.LuaNoExpr()); + adapter.getParams().add(dots); + xpcallArgs.add(LuaAst.LuaExprVarAccess(dots)); + } + LuaExprFunctionCallByName xpcall = LuaAst.LuaExprFunctionCallByName("xpcall", xpcallArgs); if (target.getReturnType() instanceof ImVoid) { adapter.getBody().add(xpcall); } else { @@ -757,12 +762,28 @@ LuaVariable fieldStorage(ImVar field) { return luaFieldStorage.getFor(imTr.canonical(field)); } + /** How many main-chunk locals lead the model; see {@link #declareChunkLocal}. */ + private int chunkLocalCount; + + /** + * Declares a local of the main chunk at the top of the script, before every function, so that + * each function reaches it as an upvalue instead of looking it up among the thousands of globals. + * Only the runtime state read on hot paths belongs here: the main chunk has room for 200 locals + * and a function for 255 upvalues. Declared now: the four object-allocator variables, the + * deallocator and, when used, the old-generics zero sentinel. + */ + void declareChunkLocal(LuaDefinition definition) { + luaModel.add(chunkLocalCount++, LuaAst.LuaChunkLocal(definition)); + } + private void createObjectManagement() { - luaModel.add(objectClass); - localizableStorageTables.add(objectClass); - luaModel.add(objectFree); - luaModel.add(objectMax); - luaModel.add(objectFreeCount); + // Every allocation and destroy reads these. The descriptor map is not aliased into function + // locals like the field stores (localizeHotStorageTables): Lua indexes an upvalue table in + // one instruction (GETTABUP), as it does a local one, so an alias would only add a copy. + declareChunkLocal(objectClass); + declareChunkLocal(objectFree); + declareChunkLocal(objectMax); + declareChunkLocal(objectFreeCount); LuaVariable object = LuaAst.LuaVariable("object", LuaAst.LuaNoExpr()); objectDealloc.getParams().add(object); @@ -786,7 +807,8 @@ private void createObjectManagement() { LuaAst.LuaExprArrayAccess(LuaAst.LuaExprVarAccess(objectFree), LuaAst.LuaExprlist(LuaAst.LuaExprVarAccess(objectFreeCount))), LuaAst.LuaExprVarAccess(object))); - luaModel.add(objectDealloc); + // Called by every destroy, so a local function like the state it pushes onto. + declareChunkLocal(objectDealloc); LuaVariable toIndexObject = LuaAst.LuaVariable("object", LuaAst.LuaNoExpr()); classToIndex.getParams().add(toIndexObject); @@ -1348,14 +1370,13 @@ private void createClassInitFunction(ImClass c, LuaVariable classVar, LuaMethod LuaAst.LuaAssignment(LuaAst.LuaExprVarAccess(objectMax), LuaAst.LuaExprBinary(LuaAst.LuaExprVarAccess(objectMax), LuaAst.LuaOpPlus(), LuaAst.LuaExprIntVal("1"))), LuaAst.LuaAssignment(LuaAst.LuaExprVarAccess(newInst), LuaAst.LuaExprVarAccess(objectMax))); + // The popped slot is not cleared: a slot above the count is never read before a destroy + // writes it again, and it holds an integer id, so it retains nothing. The free stack stays + // as large as its peak. LuaStatements recycled = LuaAst.LuaStatements( LuaAst.LuaAssignment(LuaAst.LuaExprVarAccess(newInst), LuaAst.LuaExprArrayAccess(LuaAst.LuaExprVarAccess(objectFree), LuaAst.LuaExprlist(LuaAst.LuaExprVarAccess(objectFreeCount)))), - LuaAst.LuaAssignment( - LuaAst.LuaExprArrayAccess(LuaAst.LuaExprVarAccess(objectFree), - LuaAst.LuaExprlist(LuaAst.LuaExprVarAccess(objectFreeCount))), - LuaAst.LuaExprNull()), LuaAst.LuaAssignment(LuaAst.LuaExprVarAccess(objectFreeCount), LuaAst.LuaExprBinary(LuaAst.LuaExprVarAccess(objectFreeCount), LuaAst.LuaOpMinus(), LuaAst.LuaExprIntVal("1")))); body.add(LuaAst.LuaIf( diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 7b65908c2..c8a1981bf 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -5900,6 +5900,173 @@ public void classInstancesAllocateIdsWithoutInstanceTables() throws IOException assertFalse("create must not attach an instance metatable", compiled.contains("setmetatable(new_inst")); } + /** The globals a test's script reads or writes, as {@code luac -l} lists them: an access through _ENV. */ + private java.util.Set globalsAccessedBy(String testName) throws IOException, InterruptedException { + File luaFile = new File("test-output/lua/LuaBackendAuditTests_" + testName + ".lua"); + Process luac = new ProcessBuilder(getLuacExecutable(), "-l", "-p", luaFile.getPath()) + .redirectErrorStream(true).start(); + String listing = new String(luac.getInputStream().readAllBytes(), Charsets.UTF_8); + assertTrue("luac did not finish", luac.waitFor(60, java.util.concurrent.TimeUnit.SECONDS)); + assertEquals(listing, 0, luac.exitValue()); + java.util.Set globals = new java.util.TreeSet<>(); + java.util.regex.Matcher access = java.util.regex.Pattern.compile("; _ENV \"(\\w+)\"").matcher(listing); + while (access.find()) { + globals.add(access.group(1)); + } + // The entry point is a global, so a listing which names none has changed its format. + assertTrue("the listing names no global:\n" + listing, globals.contains("main")); + return globals; + } + + /** + * Every allocation and destroy reads the allocator's state and every destroy calls the + * deallocator, so they are locals of the main chunk, declared before any function: a function + * reaches them as upvalues, not by a lookup among the thousands of globals of a map. + */ + @Test + public void objectRuntimeStateIsMainChunkLocal() throws IOException, InterruptedException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "abstract class Shape", + " abstract function area() returns int", + "class Square extends Shape", + " int side", + " construct(int side)", + " this.side = side", + " override function area() returns int", + " return side * side", + "init", + " Shape first = new Square(2)", + " let firstId = first castTo int", + " destroy first", + " Shape second = new Square(3)", + " if second castTo int == firstId and second.area() == 9 and second instanceof Square", + " and second.typeId == Square.typeId", + " testSuccess()"); + String compiled = compiledLua("objectRuntimeStateIsMainChunkLocal"); + assertTrue("the script opens with the allocator state and the deallocator as locals:\n" + compiled, + compiled.startsWith("local __wurst_objectClass = ({})\n" + + "local __wurst_objectFree = ({})\n" + + "local __wurst_objectMax = 0\n" + + "local __wurst_objectFreeCount = 0\n" + + "\n" + + "local function __wurst_deallocObject(object) \n")); + java.util.Set globals = globalsAccessedBy("objectRuntimeStateIsMainChunkLocal"); + for (String local : List.of("__wurst_objectClass", "__wurst_objectFree", "__wurst_objectMax", + "__wurst_objectFreeCount", "__wurst_deallocObject")) { + assertFalse(local + " is reached as a global: " + globals, globals.contains(local)); + } + } + + /** + * Allocation pops a recycled id without clearing its slot: a slot above the count is never read + * before a destroy writes it again, and it holds an integer, so clearing it retained nothing. + */ + @Test + public void recycledIdIsPoppedWithoutClearingItsSlot() throws IOException { + test().testLua(true).luaOnly(false).executeProg().lines( + "package Test", + "native testSuccess()", + "class Node", + "init", + " let a = new Node()", + " let b = new Node()", + " let c = new Node()", + " let ia = a castTo int", + " let ib = b castTo int", + " let ic = c castTo int", + " destroy a", + " destroy b", + " destroy c", + " let x = new Node() castTo int", + " let y = new Node() castTo int", + " let z = new Node() castTo int", + // The free stack is empty again and still holds the three ids above its count. + " let fresh = new Node()", + " let freshId = fresh castTo int", + " destroy fresh", + " let again = new Node() castTo int", + " let following = new Node() castTo int", + " if x == ic and y == ib and z == ia and freshId == ic + 1 and again == freshId", + " and following == freshId + 1", + " testSuccess()"); + String create = topLevelFunctionBodyWithPrefix( + compiledLua("recycledIdIsPoppedWithoutClearingItsSlot"), "Node:create"); + assertTrue("the id comes off the free stack:\n" + create, + create.contains("new_inst = __wurst_objectFree[__wurst_objectFreeCount]")); + assertFalse("the popped slot is not cleared:\n" + create, + create.contains("__wurst_objectFree[__wurst_objectFreeCount] = nil")); + } + + /** + * Every old-generics cast reads the zero sentinel, so it is a main-chunk local too. It stays + * {@code math.mininteger}, never a literal: the game's integers need not have the test Lua's 64 bits. + */ + @Test + public void oldGenericsZeroSentinelIsMainChunkLocal() throws IOException, InterruptedException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "int array slots", + "class Store", + " function put(int key, T value)", + " slots[key] = value castTo int", + " function get(int key) returns T", + " return slots[key] castTo T", + "init", + " let ints = new Store()", + " ints.put(0, 0)", + " ints.put(1, 7)", + " if ints.get(0) == 0 and ints.get(1) == 7", + " testSuccess()"); + String compiled = compiledLua("oldGenericsZeroSentinelIsMainChunkLocal"); + String declaration = "\nlocal __wurst_oldGenericsZero = math.mininteger\n"; + int at = compiled.indexOf(declaration); + assertTrue("the sentinel is declared as a local:\n" + compiled, at >= 0); + assertTrue("the sentinel is declared before the first function which is not a local:\n" + compiled, + at < compiled.indexOf("\nfunction ")); + assertEquals("the sentinel is assigned once:\n" + compiled, + 1, compiled.split("__wurst_oldGenericsZero = ", -1).length - 1); + assertTrue("the casts read the sentinel:\n" + compiled, + compiled.indexOf("__wurst_oldGenericsZero", at + declaration.length()) >= 0); + assertFalse("the sentinel is reached as a global", + globalsAccessedBy("oldGenericsZeroSentinelIsMainChunkLocal").contains("__wurst_oldGenericsZero")); + } + + /** + * A Wurst code value takes no parameters, so the callback adapter of one takes no varargs: the + * target would drop anything it forwarded. + */ + @Test + public void callbackAdapterOfANullaryTargetTakesNoVarargs() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "@extern native pcall(code callback) returns bool", + "int calls = 0", + "function tick()", + " calls++", + "function tickAndAccept() returns boolean", + " calls++", + " return true", + "init", + " if pcall(function tick) and pcall(function tickAndAccept) and calls == 2", + " testSuccess()"); + String compiled = compiledLua("callbackAdapterOfANullaryTargetTakesNoVarargs"); + assertTrue("the adapter calls its target with nothing to forward:\n" + compiled, + java.util.regex.Pattern.compile( + "function __wurst_callback_tick\\w*\\(\\) \\n\\txpcall\\(tick\\w*, __wurst_callback_error\\w*\\)\\n") + .matcher(compiled).find()); + assertTrue("a returning adapter keeps its single result:\n" + compiled, + java.util.regex.Pattern.compile( + "function __wurst_callback_tickAndAccept\\w*\\(\\) \\n(?:\\t.*\\n)*?" + + "\\t_, result = xpcall\\(tickAndAccept\\w*, __wurst_callback_error\\w*\\)\\n") + .matcher(compiled).find()); + assertFalse("no adapter takes varargs:\n" + compiled, + java.util.regex.Pattern.compile("function __wurst_callback_\\w+\\(\\.\\.\\.\\)").matcher(compiled).find()); + } + /** * Reading a never-written array slot must not permanently store an entry * for it - merely probing a sparse array would otherwise grow it @@ -6232,12 +6399,13 @@ public void virtualCallsReadTheSlotAtTheCallSite() { " shapes[1] = new Square()", " consume(total(2))"); String total = topLevelFunctionBodyWithPrefix(compiled, "total"); - java.util.regex.Matcher alias = java.util.regex.Pattern - .compile("local (\\w+) = __wurst_objectClass\\R").matcher(total); - assertTrue("the descriptor table is aliased for the loop:\n" + total, alias.find()); + // An upvalue table is indexed in one instruction, as a local one is, so an alias would only + // add a copy. + assertFalse("the descriptor table is an upvalue and needs no alias for the loop:\n" + total, + total.contains("= __wurst_objectClass\n")); assertTrue("the slot is read from the receiver's descriptor at the call site:\n" + total, - java.util.regex.Pattern.compile(java.util.regex.Pattern.quote(alias.group(1)) - + "\\[\\w+\\[i\\]\\]\\.\\w*area\\w*\\(").matcher(total).find()); + java.util.regex.Pattern.compile("__wurst_objectClass\\[\\w+\\[i\\]\\]\\.\\w*area\\w*\\(") + .matcher(total).find()); assertFalse("no dispatch stub is needed for a table-read receiver:\n" + compiled, compiled.contains("dispatch_")); } 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 d6cd26f1c..3a71e7df4 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 @@ -3654,20 +3654,21 @@ public void hashtableHelpersEmitPerTypeBucketsInLua() throws IOException { assertFunctionBodyContains(compiled, "__wurst_LoadAbilityHandle", "h.__wurst_ht_handle", true); } + /** A code value takes no parameters, so its adapter forwards none: no varargs at all. */ @Test - public void luaFunctionRefWrapperForwardsVarargs() throws IOException { + public void luaFunctionRefWrapperOfNullaryTargetTakesNoVarargs() throws IOException { test().testLua(true).withStdLib().lines( "package Test", "init", " let f = CreateForce()", " ForForce(f, () -> skip)" ); - String compiled = Files.toString(new File("test-output/lua/LuaTranslationTests_luaFunctionRefWrapperForwardsVarargs.lua"), Charsets.UTF_8); - assertContainsRegex(compiled, "function\\s+__wurst_callback_[A-Za-z0-9_]+\\(\\.\\.\\.\\)"); + String compiled = Files.toString(new File("test-output/lua/LuaTranslationTests_luaFunctionRefWrapperOfNullaryTargetTakesNoVarargs.lua"), Charsets.UTF_8); + assertContainsRegex(compiled, "function\\s+__wurst_callback_[A-Za-z0-9_]+\\(\\)"); + assertDoesNotContainRegex(compiled, "function\\s+__wurst_callback_[A-Za-z0-9_]+\\(\\.\\.\\.\\)"); assertFalse(compiled.contains("xpcall(function (...)")); assertContainsRegex(compiled, - "xpcall\\([A-Za-z0-9_]+, __wurst_callback_error[A-Za-z0-9_]*, \\.\\.\\.\\)"); - assertTrue(compiled.contains(", ...)")); + "xpcall\\([A-Za-z0-9_]+, __wurst_callback_error[A-Za-z0-9_]*\\)"); assertFalse(compiled.contains("local temp = ...")); assertFalse(compiled.contains("ForForce(f, function (...) \n\t\t\tlocal tempRes")); } @@ -3704,7 +3705,7 @@ public void luaFunctionRefsReuseOneAdapterAndPreserveSingleReturn() { ); List adapters = uniqueMatches(compiled, - "function\\s+(__wurst_callback_predicate[A-Za-z0-9_]*)\\(\\.\\.\\.\\)", 1); + "function\\s+(__wurst_callback_predicate[A-Za-z0-9_]*)\\(\\)", 1); assertEquals("one adapter must serve every reference to the same function:\n" + compiled, 1, adapters.size()); String adapter = adapters.get(0); 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 d7d5e9e7c..dcf6eb97e 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 @@ -888,7 +888,7 @@ protected String getLuaExecutable() { ); } - private String getLuacExecutable() { + protected String getLuacExecutable() { if (resolvedLuacExecutable != null) { return resolvedLuacExecutable; } @@ -1041,6 +1041,10 @@ private void checkLuaRootPurity(LuaCompilationUnit luaCode) { ); for (LuaStatement stmt : luaCode) { + if (stmt instanceof LuaChunkLocal local) { + // A main-chunk local is initialised at the root like a global. + stmt = local.getDefinition(); + } if (stmt instanceof LuaVariable) { LuaExprOpt initialValue = ((LuaVariable) stmt).getInitialValue(); if (initialValue instanceof LuaExpr) { From a74ba9c4f8ddbe9dae1113402c35445154f54bcb Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 01:01:33 +0200 Subject: [PATCH 03/12] Lua: print a left-nested string concatenation flat, in groups of sixteen A Wurst a + b + c on strings printed as ((a .. b) .. c): one CONCAT per operator, each building an intermediate string, and a chain of 200 parts was more parentheses than luac accepts. The text a .. b .. c parses as a .. (b .. c), which is why the chain printing left .. out, but for strings it is the same value. Long chains are split into groups so the parser levels and registers stay bounded. --- .../translation/lua/printing/LuaPrinter.java | 74 ++++++++++++++++- .../tests/LuaBackendAuditTests.java | 47 +++++++++++ .../tests/LuaTranslationTests.java | 82 +++++++++++++++++-- 3 files changed, 193 insertions(+), 10 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/printing/LuaPrinter.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/printing/LuaPrinter.java index 1b7062799..d3ce94254 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/printing/LuaPrinter.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/printing/LuaPrinter.java @@ -4,6 +4,7 @@ import de.peeeq.wurstscript.utils.Utils; import java.util.ArrayList; +import java.util.Collections; import java.util.List; public class LuaPrinter { @@ -57,6 +58,10 @@ public static void print(LuaExprArrayAccess e, StringBuilder sb, int indent) { } public static void print(LuaExprBinary e, StringBuilder sb, int indent) { + if (e.getOp() instanceof LuaOpConcatString && isConcatenation(e.getLeftExpr())) { + printConcatenation(e, sb, indent); + return; + } sb.append("("); if (continuesChain(e, e.getLeftExpr())) { printChain(e, sb, indent); @@ -115,9 +120,11 @@ private static boolean continuesChain(LuaExprBinary parent, LuaExpr left) { * The Lua precedence level of an operator whose left-nested chains print flat, or -1 for one * which keeps its parentheses. Only the levels (not their order) matter, and they follow the * Lua 5.3 manual, section 3.4.8: {@code or}; {@code and}; {@code + -}; {@code * / // %}. - * Not listed, so always parenthesised: {@code ..}, which is right associative (so - * {@code (a .. b) .. c} is not {@code a .. b .. c}); the comparisons, where a chain reads as - * a range check but compares a boolean; and any operator added later until its level is decided. + * Not listed: {@code ..}, which is right associative, so the text {@code a .. b .. c} parses as + * {@code a .. (b .. c)}, not as {@code (a .. b) .. c}; it has its own printing, see + * {@link #printConcatenation}. Also not listed, so always parenthesised: the comparisons, where + * a chain reads as a range check but compares a boolean; and any operator added later until its + * level is decided. */ private static int chainLevel(LuaOpBinary op) { if (op instanceof LuaOpOr) { @@ -133,6 +140,67 @@ private static int chainLevel(LuaOpBinary op) { return -1; } + /** + * The most operands one flat group of a concatenation holds. A flat {@code a .. b .. c} is one + * CONCAT over consecutive registers and the parser nests one level per {@code ..}, so a group + * costs as many registers and parser levels as it has operands. Sixteen is more than the parts + * of nearly every message, and a chain of up to 4096 parts then holds at most 46 registers for + * its operands, which fit beside the 199 locals a function may keep before they are spilled + * (Lua has 255). + */ + private static final int CONCATENATION_GROUP = 16; + + private static boolean isConcatenation(LuaExpr e) { + return e instanceof LuaExprBinary binary && binary.getOp() instanceof LuaOpConcatString; + } + + /** + * Prints a left-nested chain of {@code ..} flat: {@code (a .. b .. c)}, not {@code ((a .. b) .. c)}. + * The nested text concatenates once per operator and builds every intermediate string; the flat + * one is a single CONCAT into one buffer. The text parses to another tree, {@code a .. (b .. c)}, + * because {@code ..} is right associative, which is why {@link #chainLevel} leaves it out. Here the + * value is still the same: the backend only concatenates strings (operands which might be nil are + * guarded with {@code or ""}), and joining strings is associative, with no metamethod to observe + * the grouping. The operands are still evaluated from left to right. + * + *

A long chain is split into parenthesised groups of at most {@link #CONCATENATION_GROUP} + * operands, and those into groups again, so neither the parser levels (luac stops at 200, + * "too many C levels") nor the registers grow with its length. + */ + private static void printConcatenation(LuaExprBinary outermost, StringBuilder sb, int indent) { + List operands = new ArrayList<>(); + LuaExpr current = outermost; + while (isConcatenation(current)) { + LuaExprBinary binary = (LuaExprBinary) current; + operands.add(binary.getRight()); + current = binary.getLeftExpr(); + } + operands.add(current); + Collections.reverse(operands); + printConcatenation(operands, 0, operands.size(), sb, indent); + } + + private static void printConcatenation(List operands, int from, int to, StringBuilder sb, int indent) { + if (to - from == 1) { + operands.get(from).print(sb, indent); + return; + } + // Each part holds at most 'span' operands, a power of the group size, and there are at most + // CONCATENATION_GROUP parts, so the nesting is logarithmic in the length of the chain. + long span = 1; + while (span * CONCATENATION_GROUP < to - from) { + span *= CONCATENATION_GROUP; + } + sb.append("("); + for (int start = from; start < to; start += (int) span) { + if (start > from) { + sb.append(" .. "); + } + printConcatenation(operands, start, (int) Math.min(to, start + span), sb, indent); + } + sb.append(")"); + } + public static void print(LuaExprBoolVal e, StringBuilder sb, int indent) { sb.append(e.getValB()); } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index c8a1981bf..b45997926 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -7414,4 +7414,51 @@ public void aDefaultACallCanReadBeforeTheConstructorSetsItStays() { " if seen == 0 and b.f == 5", " testSuccess()"); } + + /** + * A message joined with + is one flat Lua concatenation, a single CONCAT, where the nested text + * {@code (("a=" .. x) .. ", b=")} built every intermediate string. + */ + @Test + public void stringConcatenationChainPrintsFlat() throws IOException { + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "native I2S(int i) returns string", + "function describe(int a, int b) returns string", + " return \"a=\" + I2S(a) + \", b=\" + I2S(b) + \".\"", + "init", + " if describe(1, -2) == \"a=1, b=-2.\"", + " testSuccess()"); + String describe = topLevelFunctionBodyWithPrefix(compiledLua("stringConcatenationChainPrintsFlat"), "describe"); + assertTrue(describe, describe.contains("(\"a=\" .. tostring(a) .. \", b=\" .. tostring(b) .. \".\")")); + assertFalse(describe, describe.contains("((")); + } + + /** + * Printed nested, a concatenation of 300 parts was 300 parentheses deep, more than luac accepts + * ("too many C levels"). Printed flat in groups, it loads and runs. + */ + @Test + public void aLongStringConcatenationLoadsAndRuns() throws IOException { + int parts = 300; + StringBuilder chain = new StringBuilder("\"<\""); + StringBuilder expected = new StringBuilder("<"); + for (int i = 0; i < parts; i++) { + chain.append(" + I2S(x + ").append(i).append(")"); + expected.append(1000 + i); + } + test().testLua(true).executeProg().lines( + "package Test", + "native testSuccess()", + "native I2S(int i) returns string", + "function longMessage(int x) returns string", + " return " + chain, + "init", + " if longMessage(1000) == \"" + expected + "\"", + " testSuccess()"); + String longMessage = topLevelFunctionBodyWithPrefix(compiledLua("aLongStringConcatenationLoadsAndRuns"), + "longMessage"); + assertEquals(parts, countOccurrences(longMessage, " .. ")); + } } 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 3a71e7df4..af7e4e047 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 @@ -388,15 +388,11 @@ public void otherLevelsKeepTheirParentheses() { } /** - * {@code ..} is right associative, so {@code (a .. b) .. c} is not {@code a .. b .. c}; a chain - * of comparisons reads as a range check but compares a boolean; unary operators print their - * operand in parentheses. None of them joins a chain. + * A chain of comparisons reads as a range check but compares a boolean; unary operators print + * their operand in parentheses. Neither joins a chain. */ @Test - public void concatenationComparisonsAndUnaryOperatorsKeepTheirParentheses() { - assertEquals("((a .. b) .. c)", renderLuaExpr(concat(concat(v("a"), v("b")), v("c")))); - assertEquals("(a .. (b .. c))", renderLuaExpr(concat(v("a"), concat(v("b"), v("c"))))); - assertEquals("((a + b) .. c)", renderLuaExpr(concat(plus(v("a"), v("b")), v("c")))); + public void comparisonsAndUnaryOperatorsKeepTheirParentheses() { assertEquals("((a < b) == c)", renderLuaExpr(equal(less(v("a"), v("b")), v("c")))); assertEquals("((a == b) == c)", renderLuaExpr(equal(equal(v("a"), v("b")), v("c")))); assertEquals("(-(a) - b)", renderLuaExpr( @@ -420,6 +416,78 @@ public void aVeryLongChainPrintsFlatWithoutRecursion() { assertTrue(rendered.endsWith(" + t" + (terms - 1) + ")")); } + /** + * A left-nested concatenation prints flat, one CONCAT instead of one per operator. The text + * parses as {@code a .. (b .. c)} because {@code ..} is right associative, which gives the same + * string: the backend only joins strings. A right operand, which already is one CONCAT with its + * left neighbours, and another operator as an operand keep their parentheses. + */ + @Test + public void leftNestedConcatenationPrintsFlat() { + assertEquals("(a .. b .. c)", renderLuaExpr(concat(concat(v("a"), v("b")), v("c")))); + assertEquals("(a .. b .. c .. d)", + renderLuaExpr(concat(concat(concat(v("a"), v("b")), v("c")), v("d")))); + assertEquals("(a .. (b .. c))", renderLuaExpr(concat(v("a"), concat(v("b"), v("c"))))); + assertEquals("(a .. b .. (c .. d))", + renderLuaExpr(concat(concat(v("a"), v("b")), concat(v("c"), v("d"))))); + assertEquals("((a + b) .. c)", renderLuaExpr(concat(plus(v("a"), v("b")), v("c")))); + assertEquals("((a or b) .. c .. (d or e))", + renderLuaExpr(concat(concat(or(v("a"), v("b")), v("c")), or(v("d"), v("e"))))); + } + + /** + * A flat concatenation is one parser level per operator and one register per operand, so a long + * chain is split into groups of sixteen, and those into groups again: luac stops at 200 levels + * and a function has 255 registers. + */ + @Test + public void aLongConcatenationIsGroupedWithinLuaLimits() { + LuaExpr chain = v("t0"); + for (int i = 1; i < 20; i++) { + chain = concat(chain, v("t" + i)); + } + StringBuilder expected = new StringBuilder("((t0"); + for (int i = 1; i < 16; i++) { + expected.append(" .. t").append(i); + } + expected.append(") .. (t16 .. t17 .. t18 .. t19))"); + assertEquals(expected.toString(), renderLuaExpr(chain)); + + int terms = 5000; + chain = v("t0"); + for (int i = 1; i < terms; i++) { + chain = concat(chain, v("t" + i)); + } + String rendered = renderLuaExpr(chain); + assertEquals(terms - 1, countOccurrences(rendered, " .. ")); + int depth = 0; + int maxDepth = 0; + int longestRun = 0; + int run = 0; + for (int i = 0; i < rendered.length(); i++) { + char ch = rendered.charAt(i); + if (ch == '(') { + maxDepth = Math.max(maxDepth, ++depth); + run = 0; + } else if (ch == ')') { + depth--; + run = 0; + } else if (rendered.startsWith(" .. ", i)) { + longestRun = Math.max(longestRun, ++run); + } + } + assertEquals("5000 parts are four levels of groups", 4, maxDepth); + assertTrue("no group joins more than sixteen operands: " + longestRun, longestRun < 16); + } + + private static int countOccurrences(String haystack, String needle) { + int count = 0; + for (int at = haystack.indexOf(needle); at >= 0; at = haystack.indexOf(needle, at + needle.length())) { + count++; + } + return count; + } + @Test public void testStdLib() throws IOException { test().testLua(true).withStdLib().lines( From 36a63aad81aaa0a7592c0c53e232d06ee553ad20 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 01:18:46 +0200 Subject: [PATCH 04/12] Lua: print keyed-map puts and removes as the table store __wurst_keyedMapPut and __wurst_keyedMapRemove were a Lua call per write (TimerUtils stores timer data through them). They are now printed where they are called: t[k] = v, or t[k] = nil, guarded by 'if k ~= nil' unless the key is an int or a literal. With the guard, operands with an effect are evaluated into locals first, so each runs once and in the order of the call. The stubs have no definition any more. --- .../imtranslation/LuaKeyedMap.java | 14 ++- .../lua/translation/ExprTranslation.java | 14 ++- .../lua/translation/LuaNatives.java | 15 +-- .../lua/translation/StmtTranslation.java | 111 +++++++++++++++++ .../wurstscript/tests/LuaKeyedMapTests.java | 113 ++++++++++++++---- 5 files changed, 230 insertions(+), 37 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedMap.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedMap.java index c044de110..bd4a8d31c 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedMap.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaKeyedMap.java @@ -60,6 +60,9 @@ public final class LuaKeyedMap { private static final java.util.List READ_STUBS = java.util.List.of( NATIVE_GET, NATIVE_GET_INT, NATIVE_GET_REAL, NATIVE_GET_BOOL, NATIVE_GET_STR, NATIVE_HAS); + /** The stubs which store into a map, printed as the table store they stand for where they are called. */ + private static final java.util.List WRITE_STUBS = java.util.List.of(NATIVE_PUT, NATIVE_REMOVE); + private LuaKeyedMap() { } @@ -68,10 +71,19 @@ private LuaKeyedMap() { * each stub once and a user function that happens to share the name stays an ordinary function. */ public static String readStubName(ImTranslator translator, ImFunction f) { + return stubName(translator, f, READ_STUBS); + } + + /** The name of the put or remove stub {@code f} is, or null; matched like {@link #readStubName}. */ + public static String writeStubName(ImTranslator translator, ImFunction f) { + return stubName(translator, f, WRITE_STUBS); + } + + private static String stubName(ImTranslator translator, ImFunction f, java.util.List names) { if (!f.isNative()) { return null; } - for (String name : READ_STUBS) { + for (String name : names) { if (translator.luaKeyedStubs.get(name) == f) { return name; } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java index 10b712567..f6f53d129 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java @@ -139,6 +139,12 @@ public static LuaExpr translate(ImFunctionCall e, LuaTranslator tr) { return keyedMapRead(keyedRead, e.getArguments().get(0).translateToLua(tr), e.getArguments().get(1).translateToLua(tr)); } + if (LuaKeyedMap.writeStubName(tr.imTr, e.getFunc()) != null) { + // A store returns nothing, so it is always a statement of a flat program; see + // StmtTranslation#translateKeyedMapWrite. Its stub has no definition to call. + throw new CompileError(e.attrTrace().attrSource(), + "Lua backend: " + imFuncName + " used as an expression; a keyed-map store must be a statement."); + } if (isBackendIntrinsic(e.getFunc(), tr)) { if (e.getArguments().size() != 2) { throw new CompileError(e.attrTrace().attrSource(), @@ -173,8 +179,9 @@ public static LuaExpr translate(ImFunctionCall e, LuaTranslator tr) { } /** - * Compiler-synthesised natives which the backend prints as an operator instead of a call. - * Recognised by node identity: an ordinary function of the same name keeps its definition. + * Compiler-synthesised natives which the backend prints as an operator or a table store instead + * of a call, so they have no definition. Recognised by node identity: an ordinary function of the + * same name keeps its definition. */ static boolean isBackendIntrinsic(ImFunction function, LuaTranslator tr) { return unaryIntrinsicName(function, tr) != null @@ -184,7 +191,8 @@ static boolean isBackendIntrinsic(ImFunction function, LuaTranslator tr) { || function == tr.imTr.luaRawFloorModIntFunc || function == tr.imTr.luaRawConcatFunc || function == tr.imTr.luaRawOrEmptyFunc - || LuaKeyedMap.readStubName(tr.imTr, function) != null; + || LuaKeyedMap.readStubName(tr.imTr, function) != null + || LuaKeyedMap.writeStubName(tr.imTr, function) != 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 4f4b9622f..b5c9f046d 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 @@ -309,16 +309,10 @@ public class LuaNatives { // KeyedMap: one native table per map, keyed by the element itself. Typed reads answer the // Wurst default for a missing key so the caller needs no nil normalisation. Lowered by // LuaKeyedMap/LuaNativeLowering before the inliner runs, like KeyedTable. No iteration. + // Puts and removes have no body: they are printed as the store where they are called + // (StmtTranslation#translateKeyedMapWrite), as the reads are (ExprTranslation#keyedMapRead). addNative("__wurst_keyedMapCreate", f -> f.getBody().add(LuaAst.LuaLiteral("return {}"))); - addNative("__wurst_keyedMapPut", f -> { - f.getParams().add(LuaAst.LuaVariable("t", LuaAst.LuaNoExpr())); - f.getParams().add(LuaAst.LuaVariable("k", LuaAst.LuaNoExpr())); - f.getParams().add(LuaAst.LuaVariable("v", LuaAst.LuaNoExpr())); - // Writing under a nil key is an error in Lua, where reading one is only nil: a null - // element stores nothing and reads as absent. - f.getBody().add(LuaAst.LuaLiteral("if k ~= nil then t[k] = v end")); - }); addNative("__wurst_keyedMapGet", f -> { f.getParams().add(LuaAst.LuaVariable("t", LuaAst.LuaNoExpr())); f.getParams().add(LuaAst.LuaVariable("k", LuaAst.LuaNoExpr())); @@ -362,11 +356,6 @@ public class LuaNatives { // as it is behind a trigger. f.getBody().add(LuaAst.LuaLiteral("local i = 1 local c = t[1] while c do c() i = i + 1 c = t[i] end")); }); - addNative("__wurst_keyedMapRemove", f -> { - f.getParams().add(LuaAst.LuaVariable("t", LuaAst.LuaNoExpr())); - f.getParams().add(LuaAst.LuaVariable("k", LuaAst.LuaNoExpr())); - f.getBody().add(LuaAst.LuaLiteral("if k ~= nil then t[k] = nil end")); - }); addNative("__wurst_keyedMapDestroy", f -> { f.getParams().add(LuaAst.LuaVariable("t", LuaAst.LuaNoExpr())); // The owner may still be reachable through an alias after destroy, so clear this table diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java index 2aefb571c..c7ca3a43d 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java @@ -1,8 +1,13 @@ package de.peeeq.wurstscript.translation.lua.translation; +import de.peeeq.wurstscript.attributes.CompileError; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.luaAst.*; +import de.peeeq.wurstscript.translation.imtranslation.LuaKeyedMap; +import de.peeeq.wurstscript.translation.lua.printing.LuaPrinter; +import de.peeeq.wurstscript.types.TypesHelper; +import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @@ -18,11 +23,117 @@ public static void translate(ImExpr e, List res, LuaTranslator tr) emitLuaInitXpcall(call.getFunc(), res, tr); return; } + String keyedWrite = LuaKeyedMap.writeStubName(tr.imTr, call.getFunc()); + if (keyedWrite != null) { + translateKeyedMapWrite(call, keyedWrite, res, tr); + return; + } } LuaExpr expr = e.translateToLua(tr); res.add(expr); } + /** How an operand of a keyed-map store may be moved: see {@link #translateKeyedMapWrite}. */ + private enum WriteOperand { + /** A literal or a local: the same value wherever it is read, and reading it does nothing. */ + STABLE, + /** A global or a table read: reading it does nothing, but an effect before it can change it. */ + READ, + /** Anything else, such as a call. */ + EFFECT + } + + private static WriteOperand writeOperand(ImExpr e) { + if (e instanceof ImIntVal || e instanceof ImRealVal || e instanceof ImStringVal + || e instanceof ImBoolVal || e instanceof ImNull) { + return WriteOperand.STABLE; + } + if (e instanceof ImVarAccess access) { + return access.getVar().isGlobal() ? WriteOperand.READ : WriteOperand.STABLE; + } + if (e instanceof ImVarArrayAccess access && readsOnly(access.getIndexes())) { + return WriteOperand.READ; + } + if (e instanceof ImMemberAccess access && writeOperand(access.getReceiver()) != WriteOperand.EFFECT + && readsOnly(access.getIndexes())) { + return WriteOperand.READ; + } + return WriteOperand.EFFECT; + } + + private static boolean readsOnly(ImExprs exprs) { + for (ImExpr e : exprs) { + if (writeOperand(e) == WriteOperand.EFFECT) { + return false; + } + } + return true; + } + + /** + * A keyed-map put or remove as the table store it stands for, {@code t[k] = v} or + * {@code t[k] = nil}, instead of a call to a stub with that body. Storing under a nil key is an + * error in Lua, where reading one only answers nil, so a key which may be nil (a handle, a + * string, an erased element) is tested first: a null element stores nothing and reads as absent. + * An int key or a literal is never nil and is stored without the test. + * + *

Each operand is evaluated once and in the order of the call. A plain store does that by + * itself. The test reads the key twice and evaluates the table and the value only when the key + * is not nil, so there an operand with an effect, and each operand before it which is not + * {@link WriteOperand#STABLE}, is first evaluated into a local. + */ + private static void translateKeyedMapWrite(ImFunctionCall call, String stub, List res, + LuaTranslator tr) { + boolean put = LuaKeyedMap.NATIVE_PUT.equals(stub); + ImExprs args = call.getArguments(); + int arity = put ? 3 : 2; + if (args.size() != arity) { + throw new CompileError(call.attrTrace().attrSource(), + "Lua backend: " + stub + " expects " + arity + " arguments, got " + args.size() + "."); + } + ImExpr key = args.get(1); + boolean keyNeverNil = TypesHelper.isIntType(key.attrTyp()) + || key instanceof ImIntVal || key instanceof ImRealVal || key instanceof ImStringVal + || key instanceof ImBoolVal; + List operands = new ArrayList<>(); + for (ImExpr arg : args) { + operands.add(arg.translateToLua(tr)); + } + boolean[] intoLocal = new boolean[arity]; + if (!keyNeverNil) { + int lastEffect = -1; + for (int i = 0; i < arity; i++) { + if (writeOperand(args.get(i)) == WriteOperand.EFFECT) { + lastEffect = i; + } + } + for (int i = 0; i <= lastEffect; i++) { + intoLocal[i] = writeOperand(args.get(i)) != WriteOperand.STABLE; + } + } + // A statement must not start with '(' (Lua would join it onto the previous line as a call). + intoLocal[0] |= !LuaPrinter.startsWithName(operands.get(0)); + String[] names = {"__wurst_map", "__wurst_key", "__wurst_value"}; + for (int i = 0; i < arity; i++) { + if (intoLocal[i]) { + LuaVariable local = LuaAst.LuaVariable(tr.uniqueName(names[i]), operands.get(i)); + res.add(local); + operands.set(i, LuaAst.LuaExprVarAccess(local)); + } + } + LuaExpr value = put ? operands.get(2) : LuaAst.LuaExprNull(); + LuaExpr keyExpr = operands.get(1); + LuaAssignment store = LuaAst.LuaAssignment( + LuaAst.LuaExprArrayAccess(operands.get(0), LuaAst.LuaExprlist(keyNeverNil ? keyExpr : keyExpr.copy())), + value); + if (keyNeverNil) { + res.add(store); + } else { + res.add(LuaAst.LuaIf(LuaAst.LuaExprBinary(keyExpr, LuaAst.LuaOpUnequals(), LuaAst.LuaExprNull()), + LuaAst.LuaStatements(store), LuaAst.LuaStatements())); + } + } + private static void emitLuaInitXpcall(ImFunction initFunc, List res, LuaTranslator tr) { String funcName = tr.luaFunc.getFor(initFunc).getName(); String packageName = tr.imTr.luaInitFunctions.getOrDefault(initFunc, "?"); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java index 22779dd60..49453c8aa 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java @@ -183,10 +183,13 @@ public void keyedMapLowersToSingleLuaIndexes() throws IOException { String compiled = compiled("keyedMapLowersToSingleLuaIndexes"); assertTrue("create allocates a bare table", getFunctionBody(compiled, "__wurst_keyedMapCreate").contains("return {}")); - assertTrue("put is one store", getFunctionBody(compiled, "__wurst_keyedMapPut").contains("t[k] = v")); - assertTrue("remove is one store", getFunctionBody(compiled, "__wurst_keyedMapRemove").contains("t[k] = nil")); String init = getFunctionBody(compiled, "init_Test"); + // The writes are printed as the store they stand for; a literal key is never nil. + assertTrue("put is one store: " + init, init.contains("\tm[7] = 70\n")); + assertTrue("remove is one store: " + init, init.endsWith("\tm[7] = nil")); + assertFalse("no write stub is called or defined:\n" + compiled, + compiled.contains("__wurst_keyedMapPut") || compiled.contains("__wurst_keyedMapRemove")); // The reads are printed as the index they stand for, so there is no stub to call or define. assertTrue("an int read is one index that answers 0 for a missing key: " + init, init.contains("] or 0)")); @@ -239,8 +242,8 @@ public void keyedMapStaysNativeWithStackTraces() throws IOException { String compiled = compiled("keyedMapStaysNativeWithStackTraces"); String init = getFunctionBody(compiled, "init_Test"); - assertTrue("the caller must read the table itself and call the put stub directly: " + init, - init.contains("] or 0)") && init.contains("__wurst_keyedMapPut")); + assertTrue("the caller must read and store into the table itself: " + init, + init.contains("] or 0)") && init.contains("m[7] = 70")); assertFalse("the caller must not reach the hashtable natives: " + init, init.contains("SaveInteger") || init.contains("LoadInteger")); } @@ -337,9 +340,9 @@ public void keyedMapGenericKeyAndValueReachLuaUncast() throws IOException { "endpackage"); String compiled = compiled("keyedMapGenericKeyAndValueReachLuaUncast"); - assertTrue("generic operations lower to the keyed-map put stub and the table read", - compiled.contains("__wurst_keyedMapPut") && compiled.contains("] or 0)")); String init = getFunctionBody(compiled, "init_Test"); + assertTrue("generic operations lower to the table store and the table read: " + init, + init.contains("[u] = 5") && init.contains("] or 0)") && !compiled.contains("__wurst_keyedMapPut")); assertFalse("the unit must be handed over as itself: " + init, init.contains("__wurst_objectToIndex") || init.contains("__wurst_classFromIndex") || init.contains("__wurst_classToIndex")); @@ -384,14 +387,14 @@ public void handleBoundWrapperSpecializesToOneLuaTableAccess() throws IOExceptio // The small specialized wrappers are expanded into the caller, which is all that is left to read. String lua = compiled("handleBoundWrapperSpecializesToOneLuaTableAccess"); String init = getFunctionBody(lua, "init_Test"); - assertTrue("the caller calls the unchanged put intrinsic: " + init, init.contains("__wurst_keyedMapPut")); + assertTrue("the unchanged put intrinsic is one direct Lua table store: " + init, + init.contains("[key2] = value6") && init.indexOf("] = ") == init.lastIndexOf("] = ")); assertTrue("and reads the table with the int default: " + init, init.contains("] or 0)") && !init.contains("__wurst_keyedMapGetInt")); assertFalse("the native unit is used as the key without handle-id conversion: " + init, init.contains("GetHandleId") || init.contains("__wurst_objectToIndex") || init.contains("__wurst_classToIndex")); - assertTrue("put is one direct Lua table store", - getFunctionBody(lua, "__wurst_keyedMapPut").contains("t[k] = v")); + assertFalse("put has no stub to call or define", lua.contains("__wurst_keyedMapPut")); assertTrue("get is one direct Lua table read", init.indexOf("] or 0)") == init.lastIndexOf("] or 0)")); } @@ -501,16 +504,17 @@ public void handleBoundGenericValuesKeepTheirNativeLuaRepresentation() throws IO // The small specialized wrappers are expanded into the caller, which is all that is left to read. String lua = compiled("handleBoundGenericValuesKeepTheirNativeLuaRepresentation"); String init = getFunctionBody(lua, "init_Test"); - assertTrue("the caller calls the generic put intrinsic: " + init, init.contains("__wurst_keyedMapPut")); + assertTrue("the caller stores into the table directly: " + init, init.contains("[u] = 7")); assertTrue("and reads the table directly: " + init, init.contains("] or 0)") && !init.contains("__wurst_keyedMapGet")); assertFalse("generic values and handle keys need no index conversions: " + init, init.contains("GetHandleId") || init.contains("__wurst_objectToIndex") || init.contains("__wurst_classToIndex") || init.contains("__wurst_classFromIndex")); - String putStub = getFunctionBody(lua, "__wurst_keyedMapPut"); - assertTrue("generic put has exactly one direct table store: " + putStub, - putStub.contains("t[k] = v") && putStub.indexOf("t[k]") == putStub.lastIndexOf("t[k]")); - assertFalse("generic get is a table index, with no stub to define", lua.contains("function __wurst_keyedMapGet(")); + assertTrue("each generic put is one direct table store: " + init, + init.contains("KeyedMap_map_storage[this10][key1] = value4") + && init.contains("KeyedMap_map_storage[this11][key2] = value5")); + assertFalse("generic get and put are table accesses, with no stub to define", + lua.contains("function __wurst_keyedMapGet(") || lua.contains("function __wurst_keyedMapPut(")); } @Test @@ -1052,9 +1056,9 @@ public void unitKeyedMapKeysTheUnitItselfOnLua() throws IOException { String compiled = compiled("unitKeyedMapKeysTheUnitItselfOnLua"); String init = getFunctionBody(compiled, "init_Test"); - assertTrue("writes lower to the keyed-map stubs and reads to the table index: " + init, - init.contains("__wurst_keyedMapPut") && init.contains("] or 0)") - && init.contains("] ~= nil)") && init.contains("__wurst_keyedMapRemove")); + assertTrue("writes lower to nil-guarded table stores and reads to the table index: " + init, + init.contains("if (u ~= nil) then\n\t\tm[u] = 5\n\tend") && init.contains("] or 0)") + && init.contains("] ~= nil)") && init.contains("if (u ~= nil) then\n\t\tm[u] = nil\n\tend")); assertFalse("the unit must not go through a handle id or an index map: " + init, init.contains("GetHandleId") || init.contains("__wurst_objectToIndex") || init.contains("SaveInteger")); } @@ -1103,6 +1107,73 @@ public void handleKeyedMapAgreesOnBothBackends() { "endpackage"); } + /** + * A put or remove is printed as the store itself, guarded by a nil test where the key is a + * handle. The test reads the key twice and skips the store, so operands with an effect are + * evaluated into locals first: each one still runs once, in the order of the call, also when the + * key is null, and a map read before an effect is the map from before it. Run against the + * interpreter and Jass through the Table bodies. + */ + @Test + public void keyedMapWritesEvaluateEachOperandOnceAndInOrder() throws IOException { + test().testLua(true).luaOnly(false).executeProg(true).withStdLib().lines( + "package KeyedMap", + "import Table", + "@compilerintrinsic public function keyedMapCreate() returns int", + " return (new Table()) castTo int", + "@compilerintrinsic public function keyedMapPut(int tbl, timer key, int value)", + " (tbl castTo Table).saveInt(GetHandleId(key), value)", + "@compilerintrinsic public function keyedMapGetInt(int tbl, timer key) returns int", + " return (tbl castTo Table).loadInt(GetHandleId(key))", + "@compilerintrinsic public function keyedMapRemove(int tbl, timer key)", + " (tbl castTo Table).removeInt(GetHandleId(key))", + "endpackage", + "package Test", + "import KeyedMap", + "string trace = \"\"", + "int theMap", + "int otherMap", + "function mapOf(string tag) returns int", + " trace += tag", + " return theMap", + "function keyOf(string tag, timer t) returns timer", + " trace += tag", + " return t", + "function valueOf(string tag, int v) returns int", + " trace += tag", + " return v", + "function switchingMap(timer t) returns timer", + " theMap = otherMap", + " return t", + "init", + " theMap = keyedMapCreate()", + " otherMap = keyedMapCreate()", + " let firstMap = theMap", + " let key = CreateTimer()", + " timer none = null", + " keyedMapPut(mapOf(\"m\"), keyOf(\"k\", key), valueOf(\"v\", 5))", + " keyedMapPut(mapOf(\"M\"), keyOf(\"K\", none), valueOf(\"V\", 6))", + " if keyedMapGetInt(firstMap, key) != 5", + " testFail(\"put did not store\")", + " keyedMapRemove(mapOf(\"r\"), keyOf(\"q\", key))", + " keyedMapRemove(mapOf(\"R\"), keyOf(\"Q\", none))", + " if trace != \"mkvMKVrqRQ\"", + " testFail(\"operands evaluated as \" + trace)", + " if keyedMapGetInt(firstMap, key) != 0", + " testFail(\"remove did not clear\")", + " keyedMapPut(theMap, switchingMap(key), 7)", + " if keyedMapGetInt(firstMap, key) != 7 or keyedMapGetInt(otherMap, key) != 0", + " testFail(\"the map was read after the key's effect\")", + " testSuccess()", + "endpackage"); + + String init = getFunctionBody(compiled("keyedMapWritesEvaluateEachOperandOnceAndInOrder"), "init_Test"); + assertTrue("operands with effects are evaluated into locals before the guarded store: " + init, + init.contains("local __wurst_key = keyOf(\"k\", ") && init.contains("(__wurst_key ~= nil) then")); + assertFalse("no write stub is called: " + init, + init.contains("__wurst_keyedMapPut") || init.contains("__wurst_keyedMapRemove")); + } + @Test public void keyedMapDestroyClearsLuaStoreWhileAliasRemains() { test().testLua(true).luaOnly(true).executeProg().withStdLib().lines( @@ -1203,9 +1274,11 @@ public void intKeyedClassMapLowersToSingleLuaIndexes() throws IOException { String compiled = compiled("intKeyedClassMapLowersToSingleLuaIndexes"); String init = getFunctionBody(compiled, "init_Test"); - assertTrue("writes lower to the keyed-map stubs and reads to the table index: " + init, - init.contains("__wurst_keyedMapPut") && init.contains("] ~= nil)") - && !init.contains("__wurst_keyedMapGet") && init.contains("__wurst_keyedMapRemove")); + assertTrue("writes under an int key lower to unguarded table stores and reads to the table index: " + init, + init.contains("[1093677104] = this6") && init.contains("m[1093677104] = nil") + && init.contains("] ~= nil)") && !init.contains("__wurst_keyedMapPut") + && !init.contains("__wurst_keyedMapRemove") && !init.contains("__wurst_keyedMapGet")); + assertFalse("an int key is never nil and needs no test: " + init, init.contains("(1093677104 ~= nil)")); assertFalse("the object must be stored as itself: " + init, init.contains("__wurst_objectToIndex") || init.contains("__wurst_classToIndex") || init.contains("__wurst_classFromIndex")); From 0035130864baa8875f2ab41e6c079f26e00687bc Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 01:26:47 +0200 Subject: [PATCH 05/12] Lua: mod by a positive divisor is the % operator The ModuloInteger/ModuloReal helpers now test the divisor: for an int above 0, and a real of at least 1, Lua's floored % gives the Blizzard.j result (the real case stops at 1 because Lua 5.3 decides with fmod * b < 0, which underflows for a subnormal remainder and a smaller divisor). Inlined where the divisor is a literal, the test folds and a mod is one VM operation, without math.fmod and the sign correction; a runtime divisor pays one comparison instead of the C call when it is positive. --- .../providers/LuaEnsureTypeProvider.java | 9 ++ .../imtranslation/ImTranslator.java | 7 +- .../imtranslation/LuaNativeLowering.java | 53 ++++++-- .../lua/translation/ExprTranslation.java | 3 +- .../tests/LuaBackendAuditTests.java | 117 +++++++++++++++++- 5 files changed, 174 insertions(+), 15 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/jassinterpreter/providers/LuaEnsureTypeProvider.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/jassinterpreter/providers/LuaEnsureTypeProvider.java index 3494f39fc..17765ad76 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/jassinterpreter/providers/LuaEnsureTypeProvider.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/jassinterpreter/providers/LuaEnsureTypeProvider.java @@ -66,4 +66,13 @@ public ILconstInt __wurst_rawFloorModInt(ILconstInt a, ILconstInt b) { public ILconstReal __wurst_rawFmodReal(ILconstReal a, ILconstReal b) { return ILconstReal.create(a.getVal() % b.getVal()); } + + /** Lua 5.3's float {@code %}: fmod, plus the divisor when {@code fmod * b < 0}. */ + public ILconstReal __wurst_rawFloorModReal(ILconstReal a, ILconstReal b) { + float m = a.getVal() % b.getVal(); + if (m * b.getVal() < 0) { + m += b.getVal(); + } + return ILconstReal.create(m); + } } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java index d9a9a57ab..6c9ea2276 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java @@ -202,6 +202,7 @@ public T canonical(T copy) { @Nullable public ImFunction luaRawFmodIntFunc = null; @Nullable public ImFunction luaRawFmodRealFunc = null; @Nullable public ImFunction luaRawFloorModIntFunc = null; + @Nullable public ImFunction luaRawFloorModRealFunc = null; @Nullable public ImFunction luaRawConcatFunc = null; /** {@code x or ""}: a string operand which may be nil, made safe for {@link #luaRawConcatFunc}. */ @Nullable public ImFunction luaRawOrEmptyFunc = null; @@ -240,7 +241,8 @@ public boolean isTrapFreeLuaIntrinsicCall(ImFunctionCall call) { || (divisor instanceof ImRealVal realVal && Double.parseDouble(realVal.getValR()) != 0.0); return nonZeroDivisor && (target == luaRawFloorDivIntFunc || target == luaRawFmodIntFunc - || target == luaRawFloorModIntFunc || target == luaRawFmodRealFunc); + || target == luaRawFloorModIntFunc || target == luaRawFmodRealFunc + || target == luaRawFloorModRealFunc); } /** * Whether {@code f} is one of the Lua keyed-map read stubs, which the backend prints as a table @@ -263,7 +265,8 @@ public List pinnedFunctions() { List result = new ArrayList<>(); for (ImFunction f : new ImFunction[]{ ensureIntFunc, ensureBoolFunc, ensureRealFunc, ensureStrFunc, stringConcatFunc, - luaRawFloorDivIntFunc, luaRawFmodIntFunc, luaRawFmodRealFunc, luaRawFloorModIntFunc, luaRawConcatFunc, + luaRawFloorDivIntFunc, luaRawFmodIntFunc, luaRawFmodRealFunc, luaRawFloorModIntFunc, + luaRawFloorModRealFunc, luaRawConcatFunc, luaRawOrEmptyFunc, luaRawR2IFunc, luaRawToNumberIntFunc, luaRawToNumberRealFunc, luaRawToIntegerFunc, luaRawToStringFunc, luaIntDivFunc, luaModIntFunc, luaModRealFunc, debugPrintFunction, errorFunc, genericNewMarker, globalInitFunc}) { 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 bb3595cd6..5facbace4 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 @@ -612,6 +612,7 @@ private static final class DivModFunctions { private ImFunction rawFmodInt; private ImFunction rawFloorModInt; private ImFunction rawFmodReal; + private ImFunction rawFloorModReal; private ImFunction intDiv; private ImFunction modInt; private ImFunction modReal; @@ -633,18 +634,27 @@ ImFunction intDiv() { return intDiv; } + /** For an int divisor b > 0, Lua's floored % is exactly the ModuloInteger result. */ ImFunction modInt() { if (modInt == null) { - modInt = buildMod("__wurst_modInt", TypesHelper.imInt(), JassIm.ImIntVal(0), rawFmodInt()); + modInt = buildMod("__wurst_modInt", TypesHelper.imInt(), JassIm.ImIntVal(0), + WurstOperator.GREATER, JassIm.ImIntVal(0), rawFmodInt(), rawFloorModInt()); translator.luaModIntFunc = modInt; created.add(modInt); } return modInt; } + /** + * Lua 5.3 computes a float % as fmod plus the divisor when {@code fmod * b < 0}, which for a + * positive b is the ModuloReal correction (-0.0 stays -0.0 on both), except where that product + * underflows to zero: a subnormal remainder and a divisor below 1. So the operator is taken + * from b >= 1 on, and never for NaN. + */ ImFunction modReal() { if (modReal == null) { - modReal = buildMod("__wurst_modReal", TypesHelper.imReal(), JassIm.ImRealVal("0."), rawFmodReal()); + modReal = buildMod("__wurst_modReal", TypesHelper.imReal(), JassIm.ImRealVal("0."), + WurstOperator.GREATER_EQ, JassIm.ImRealVal("1."), rawFmodReal(), rawFloorModReal()); translator.luaModRealFunc = modReal; created.add(modReal); } @@ -692,6 +702,16 @@ private ImFunction rawFmodReal() { return rawFmodReal; } + /** Lua's float {@code %}; only correct for a divisor of at least 1, which the caller guarantees. */ + private ImFunction rawFloorModReal() { + if (rawFloorModReal == null) { + rawFloorModReal = rawNative("__wurst_rawFloorModReal", TypesHelper.imReal()); + translator.luaRawFloorModRealFunc = rawFloorModReal; + created.add(rawFloorModReal); + } + return rawFloorModReal; + } + /** A native leaf with two params and a return, translated as a Lua backend intrinsic. */ private static ImFunction rawNative(String name, ImType numType) { ImVar a = JassIm.ImVar(SYNTHETIC_TRACE, numType.copy(), "a", false); @@ -733,24 +753,35 @@ private static ImFunction buildIntDiv(ImFunction rawFloorDiv) { } /** - * local r = rawFmod(a, b) - * if r < 0 then r = r + b end + * local r + * if b [> 0 | >= 1.] then r = rawFloorMod(a, b) + * else r = rawFmod(a, b); if r < 0 then r = r + b end end * return r - * (Lua's % is floored; Wurst mod follows Blizzard.j's ModuloInteger/ModuloReal: - * truncated remainder, plus the divisor when the remainder is negative.) + * (Wurst mod follows Blizzard.j's ModuloInteger/ModuloReal: truncated remainder, plus the + * divisor when the remainder is negative. Lua's % is floored, which gives the same for the + * divisors {@code positiveTest} admits, as one VM operation instead of a C call and a branch. + * Once inlined where the divisor is a literal, the test folds and only one branch is left.) */ - private static ImFunction buildMod(String name, ImType numType, ImExpr zeroLiteral, ImFunction rawFmod) { + private static ImFunction buildMod(String name, ImType numType, ImExpr zeroLiteral, + WurstOperator positiveTest, ImExpr positiveBound, + ImFunction rawFmod, ImFunction rawFloorMod) { ImVar a = JassIm.ImVar(SYNTHETIC_TRACE, numType.copy(), "a", false); ImVar b = JassIm.ImVar(SYNTHETIC_TRACE, numType.copy(), "b", false); ImVar r = JassIm.ImVar(SYNTHETIC_TRACE, numType.copy(), "r", false); ImStmts body = JassIm.ImStmts( - JassIm.ImSet(SYNTHETIC_TRACE, JassIm.ImVarAccess(r), call(rawFmod, JassIm.ImVarAccess(a), JassIm.ImVarAccess(b))), JassIm.ImIf(SYNTHETIC_TRACE, - JassIm.ImOperatorCall(WurstOperator.LESS, JassIm.ImExprs(JassIm.ImVarAccess(r), zeroLiteral)), + JassIm.ImOperatorCall(positiveTest, JassIm.ImExprs(JassIm.ImVarAccess(b), positiveBound)), JassIm.ImStmts(JassIm.ImSet(SYNTHETIC_TRACE, JassIm.ImVarAccess(r), - JassIm.ImOperatorCall(WurstOperator.PLUS, JassIm.ImExprs(JassIm.ImVarAccess(r), JassIm.ImVarAccess(b))))), - JassIm.ImStmts() + call(rawFloorMod, JassIm.ImVarAccess(a), JassIm.ImVarAccess(b)))), + JassIm.ImStmts( + JassIm.ImSet(SYNTHETIC_TRACE, JassIm.ImVarAccess(r), call(rawFmod, JassIm.ImVarAccess(a), JassIm.ImVarAccess(b))), + JassIm.ImIf(SYNTHETIC_TRACE, + JassIm.ImOperatorCall(WurstOperator.LESS, JassIm.ImExprs(JassIm.ImVarAccess(r), zeroLiteral)), + JassIm.ImStmts(JassIm.ImSet(SYNTHETIC_TRACE, JassIm.ImVarAccess(r), + JassIm.ImOperatorCall(WurstOperator.PLUS, JassIm.ImExprs(JassIm.ImVarAccess(r), JassIm.ImVarAccess(b))))), + JassIm.ImStmts() + )) ), JassIm.ImReturn(SYNTHETIC_TRACE, JassIm.ImVarAccess(r)) ); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java index f6f53d129..bc5c77b77 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java @@ -155,7 +155,7 @@ public static LuaExpr translate(ImFunctionCall e, LuaTranslator tr) { if (e.getFunc() == tr.imTr.luaRawFloorDivIntFunc) { return LuaAst.LuaExprBinary(left, LuaAst.LuaOpFloorDiv(), right); } - if (e.getFunc() == tr.imTr.luaRawFloorModIntFunc) { + if (e.getFunc() == tr.imTr.luaRawFloorModIntFunc || e.getFunc() == tr.imTr.luaRawFloorModRealFunc) { return LuaAst.LuaExprBinary(left, LuaAst.LuaOpMod(), right); } if (e.getFunc() == tr.imTr.luaRawConcatFunc) { @@ -189,6 +189,7 @@ static boolean isBackendIntrinsic(ImFunction function, LuaTranslator tr) { || function == tr.imTr.luaRawFmodIntFunc || function == tr.imTr.luaRawFmodRealFunc || function == tr.imTr.luaRawFloorModIntFunc + || function == tr.imTr.luaRawFloorModRealFunc || function == tr.imTr.luaRawConcatFunc || function == tr.imTr.luaRawOrEmptyFunc || LuaKeyedMap.readStubName(tr.imTr, function) != null diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index b45997926..00b5cb276 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -5805,7 +5805,9 @@ public void optimizedMovedImHelpersHaveNoDanglingReferences() { assertTrue("repro must exercise string concat lowering", compiled.contains(" .. ")); assertFalse("the raw concat primitive is an operator, not a call", compiled.contains("__wurst_rawConcat")); assertTrue("repro must exercise integer div lowering", compiled.contains(" // ")); - assertTrue("repro must exercise integer and real mod lowering", compiled.contains("math.fmod(")); + // the divisors are literals once inlined, so both lower to Lua's % (see modByADivisorThatBecomesALiteral...) + assertTrue("repro must exercise integer and real mod lowering:\n" + compiled, + compiled.contains(" % 2)") && compiled.contains(" % 2.)")); assertFalse("raw numeric primitive calls must not survive Lua emission", compiled.contains("__wurst_rawF")); } @@ -7461,4 +7463,117 @@ public void aLongStringConcatenationLoadsAndRuns() throws IOException { "longMessage"); assertEquals(parts, countOccurrences(longMessage, " .. ")); } + + /** + * A divisor which only becomes a literal once the helper is inlined (here a parameter, in a map + * often a constant) is Lua's %, with no fmod and no sign correction: the helper tests the divisor + * and the test folds. A real divisor below 1 keeps fmod, and so does a runtime divisor, whose + * test runs. + */ + @Test + public void modByADivisorThatBecomesALiteralIsTheLuaOperator() { + String compiled = compileOptimizedLua("modByADivisorThatBecomesALiteralIsTheLuaOperator", + "package Test", + "native consume(int value)", + "native consumeReal(real value)", + "function wrap(int a, int m) returns int", + " return a mod m", + "function wrapReal(real a, real m) returns real", + " return a % m", + "@noinline function fixed(int x) returns int", + " return wrap(x, 1000003)", + "@noinline function fixedReal(real x) returns real", + " return wrapReal(x, 360.)", + "@noinline function belowOne(real x) returns real", + " return wrapReal(x, 0.5)", + "@noinline function varying(int x, int y) returns int", + " return x mod y", + "init", + " consume(fixed(5))", + " consumeReal(fixedReal(5.))", + " consumeReal(belowOne(5.))", + " consume(varying(5, 3))"); + String fixed = topLevelFunctionBodyWithPrefix(compiled, "fixed("); + assertTrue(fixed, fixed.contains(" % 1000003)")); + assertFalse(fixed, fixed.contains("fmod") || fixed.contains("< 0")); + String fixedReal = topLevelFunctionBodyWithPrefix(compiled, "fixedReal"); + assertTrue(fixedReal, fixedReal.contains(" % 360.)")); + assertFalse(fixedReal, fixedReal.contains("fmod") || fixedReal.contains("< 0")); + String belowOne = topLevelFunctionBodyWithPrefix(compiled, "belowOne"); + assertTrue("a real divisor below 1 keeps fmod:\n" + belowOne, belowOne.contains("math.fmod(")); + assertFalse(belowOne, belowOne.contains(" % ")); + String varying = topLevelFunctionBodyWithPrefix(compiled, "varying"); + assertTrue("a runtime divisor is tested:\n" + varying, + varying.contains(" % ") && varying.contains("math.fmod(")); + } + + private static final String[] MOD_SEMANTICS_PROG = { + "package Test", + "native testSuccess()", + "real zero = 0.", + "function im(int a, int b) returns int", + " return a mod b", + "function rm(real a, real b) returns real", + " return a % b", + "@noinline function v(int x) returns int", + " return x", + "@noinline function w(real x) returns real", + " return x", + "function near(real a, real b) returns boolean", + " return a - b < 0.0001 and b - a < 0.0001", + "init", + " let negZero = zero * -1.", + " boolean ok = true", + // runtime dividends, literal divisors + " ok = ok and im(v(7), 3) == 1 and im(v(-7), 3) == 2 and im(v(0), 3) == 0", + " ok = ok and im(v(-3), 3) == 0 and im(v(-1), 8) == 7 and im(v(-2147483647), 2) == 1", + // runtime divisors, both signs + " ok = ok and im(v(7), v(3)) == 1 and im(v(-7), v(3)) == 2 and im(v(7), v(-3)) == 1", + " ok = ok and im(v(-7), v(-3)) == -4 and im(v(0), v(-5)) == 0", + // reals: a divisor of at least 1, below 1, negative, at runtime + " ok = ok and near(rm(w(7.5), 2.), 1.5) and near(rm(w(-7.5), 2.), 0.5) and near(rm(w(0.), 360.), 0.)", + " ok = ok and near(rm(w(-370.), 360.), 350.) and near(rm(w(-0.75), 0.5), 0.25)", + " ok = ok and near(rm(w(7.5), -2.), 1.5) and near(rm(w(-7.5), -2.), -3.5)", + " ok = ok and near(rm(w(-7.5), w(2.)), 0.5) and near(rm(w(-7.5), w(0.5)), 0.)", + // -0.0 is a zero, not the divisor + " ok = ok and rm(negZero, 360.) == 0. and rm(negZero, 360.) < 1. and rm(negZero, w(360.)) < 1.", + " if ok", + " testSuccess()" + }; + + /** + * Wurst mod is Blizzard.j's ModuloInteger/ModuloReal: the truncated remainder, plus the divisor + * when it is negative. Lua's % agrees for an int divisor above 0 and a real one of at least 1, + * which the helper tests at run time and the optimiser folds for a literal. Against Jass and the + * interpreter, once with the helper called and once inlined and folded. + */ + @Test + public void modAgreesWithJassForEveryDivisorSign() { + test().testLua(true).luaOnly(false).executeProg().lines(MOD_SEMANTICS_PROG); + } + + @Test + public void modAgreesWithJassForEveryDivisorSignWhenFolded() { + test().testLua(true).luaOnly(false).inline().localOptimizations().executeProg().lines(MOD_SEMANTICS_PROG); + } + + /** -0.0 stays -0.0 under Lua's % as under fmod and its correction: 1 / r is minus infinity. */ + @Test + public void modKeepsTheSignOfANegativeZeroOnLua() throws IOException { + test().testLua(true).inline().localOptimizations().executeProg().lines( + "package Test", + "native testSuccess()", + "real zero = 0.", + "function rm(real a, real b) returns real", + " return a % b", + "@noinline function w(real x) returns real", + " return x", + "init", + " let negZero = zero * -1.", + " if 1. / rm(negZero, 360.) < 0. and 1. / rm(negZero, w(360.)) < 0. and 1. / rm(negZero, w(0.5)) < 0.", + " testSuccess()"); + String compiled = topLevelFunctionBodyWithPrefix(compiledLua("modKeepsTheSignOfANegativeZeroOnLua"), + "init_Test"); + assertTrue(compiled, compiled.contains(" % 360.)")); + } } From d0f96fc96bb66dbc690c933da348f4544f902fc4 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 01:31:27 +0200 Subject: [PATCH 06/12] Lua: drop the or "" guard of a concatenation operand which cannot be nil The guard is decided before inlining, where int.toString() is a call that might answer nil; once inlined it is tostring(x), and a parameter may have become a literal. The backend now asks neverNil again of the operand the guard finally holds, so tostring results, literals and concatenations are joined as they are. Every other operand keeps its guard. --- .../imtranslation/LuaNativeLowering.java | 10 ++++-- .../lua/translation/ExprTranslation.java | 11 ++++-- .../tests/LuaBackendAuditTests.java | 34 +++++++++++++++++++ 3 files changed, 51 insertions(+), 4 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 5facbace4..d235f999c 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 @@ -408,15 +408,21 @@ private static ImExpr concatenation(ImOperatorCall call, ImTranslator translator */ private static final Set STRING_NATIVES_NEVER_NIL = Set.of("I2S"); - private static boolean neverNil(ImExpr e, ImTranslator translator) { + /** + * Whether the string {@code e} cannot be nil on Lua. Asked here, before the optimiser, and again + * by the backend of the operand a {@code luaRawOrEmptyFunc} guard finally holds: inlining turns a + * call such as {@code int.toString()} into the {@code I2S} it returns, which needs no guard. + */ + public static boolean neverNil(ImExpr e, ImTranslator translator) { if (e instanceof ImStringVal) { return true; } if (e instanceof ImFunctionCall imFunctionCall) { ImFunction f = imFunctionCall.getFunc(); - // a lowered concatenation always has two strings to join + // a lowered concatenation always has two strings to join; tostring answers a string return f == translator.luaRawConcatFunc || f == translator.luaRawOrEmptyFunc + || f == translator.luaRawToStringFunc || (f.isNative() && STRING_NATIVES_NEVER_NIL.contains(f.getName())); } return false; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java index bc5c77b77..f56fa6dc9 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java @@ -8,6 +8,7 @@ import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; import de.peeeq.wurstscript.translation.imtranslation.LuaKeyedMap; import de.peeeq.wurstscript.translation.imtranslation.LuaMethodCallLowering; +import de.peeeq.wurstscript.translation.imtranslation.LuaNativeLowering; import de.peeeq.wurstscript.types.TypesHelper; import java.util.Optional; @@ -122,8 +123,14 @@ public static LuaExpr translate(ImFunctionCall e, LuaTranslator tr) { unaryIntrinsic.substring(dot + 1)), argument); } if (e.getFunc() == tr.imTr.luaRawOrEmptyFunc && e.getArguments().size() == 1) { - return LuaAst.LuaExprBinary(e.getArguments().get(0).translateToLua(tr), LuaAst.LuaOpOr(), - LuaAst.LuaExprStringVal("")); + ImExpr operand = e.getArguments().get(0); + LuaExpr translated = operand.translateToLua(tr); + // The guard was placed before inlining; the operand it finally holds may be one that + // cannot be nil (a literal, I2S, a concatenation), which then needs no guard. + if (LuaNativeLowering.neverNil(operand, tr.imTr)) { + return translated; + } + return LuaAst.LuaExprBinary(translated, LuaAst.LuaOpOr(), LuaAst.LuaExprStringVal("")); } if (e.getFunc() == tr.imTr.luaRawR2IFunc && e.getArguments().size() == 1) { LuaExpr x = e.getArguments().get(0).translateToLua(tr); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 00b5cb276..9a227a2c5 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -7557,6 +7557,40 @@ public void modAgreesWithJassForEveryDivisorSignWhenFolded() { test().testLua(true).luaOnly(false).inline().localOptimizations().executeProg().lines(MOD_SEMANTICS_PROG); } + /** + * The {@code or ""} guard of a concatenation operand is placed before inlining, where + * {@code x.str()} is a call that might answer nil. Inlined it is the {@code tostring} of I2S, and + * a parameter may become a literal; neither can be nil, so neither keeps the guard. A variable + * which may be nil keeps it. + */ + @Test + public void concatenationOperandsWhichCannotBeNilLoseTheirGuardOnceInlined() throws IOException { + test().testLua(true).inline().localOptimizations().executeProg().lines( + "package Test", + "native testSuccess()", + "native I2S(int i) returns string", + "function int.str() returns string", + " return I2S(this)", + "function greet(string name) returns string", + " return \"hi \" + name", + "@noinline function label(int x) returns string", + " return \"x=\" + x.str() + \"!\"", + "@noinline function fixedGreeting() returns string", + " return greet(\"bob\")", + "@noinline function greetAnyone(string name) returns string", + " return \"hi \" + name", + "init", + " if label(-3) == \"x=-3!\" and fixedGreeting() == \"hi bob\" and greetAnyone(\"al\") == \"hi al\"", + " testSuccess()"); + String compiled = compiledLua("concatenationOperandsWhichCannotBeNilLoseTheirGuardOnceInlined"); + String label = topLevelFunctionBodyWithPrefix(compiled, "label"); + assertTrue(label, label.contains("(\"x=\" .. tostring(x) .. \"!\")")); + String fixedGreeting = topLevelFunctionBodyWithPrefix(compiled, "fixedGreeting"); + assertTrue(fixedGreeting, fixedGreeting.contains("(\"hi \" .. \"bob\")")); + String greetAnyone = topLevelFunctionBodyWithPrefix(compiled, "greetAnyone"); + assertTrue("a variable keeps its guard:\n" + greetAnyone, greetAnyone.contains("(name or \"\")")); + } + /** -0.0 stays -0.0 under Lua's % as under fmod and its correction: 1 / r is minus infinity. */ @Test public void modKeepsTheSignOfANegativeZeroOnLua() throws IOException { From 0bd234c05151e5fc21bd6bc8e9d03a35ed2d1160 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 01:42:34 +0200 Subject: [PATCH 07/12] Lua: no int normalisation in front of __wurst_objectFromIndex An old-generics value read back as a handle was passed through ensureInt (tonumber, math.tointeger) before the TypeCasting fromIndex, which Lua prints as __wurst_objectFromIndex. That helper answers nil for nil, for 0 and for any number it never handed out, and indexes its table with the number itself, so for what a slot can hold (nil, 0, an index) the result is the same without it. Other fromIndex conversions keep their ensureInt. --- .../imtranslation/ExprTranslation.java | 8 +++- .../lua/translation/ExprTranslation.java | 22 +++++++++-- .../tests/LuaBackendAuditTests.java | 39 +++++++++++++++++++ 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java index c0b3bdef4..01a23e4a4 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java @@ -155,8 +155,9 @@ private static ImExpr wrapTranslation(Element trace, ImTranslator t, ImExpr tran boolean indexContext) { ImFunction toIndex = null; ImFunction fromIndex = null; + FuncDef fromIndexFunc = null; if (actualType instanceof WurstTypeBoundTypeParam wtb) { - FuncDef fromIndexFunc = wtb.getFromIndex(); + fromIndexFunc = wtb.getFromIndex(); if (fromIndexFunc != null) { fromIndex = t.getFuncFor(fromIndexFunc); } @@ -178,7 +179,10 @@ private static ImExpr wrapTranslation(Element trace, ImTranslator t, ImExpr tran return wrapLua(trace, t, translated, actualType); } else if (fromIndex != null) { // System.out.println(" --> fromIndex"); - if(t.isLuaTarget()) { + // Lua normalises the erased value to an int first, except for the conversions which + // answer the same for every value a slot can hold (nil, 0, an index) without it. + if (t.isLuaTarget() && !de.peeeq.wurstscript.translation.lua.translation.ExprTranslation + .printsAsObjectFromIndex(fromIndexFunc)) { translated = ImFunctionCall(trace, t.ensureIntFunc, ImTypeArguments(), JassIm.ImExprs(translated), false, CallType.NORMAL); } // no ensure type necessary here, because the fromIndex function is already type safe diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java index f56fa6dc9..3f6865495 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/ExprTranslation.java @@ -1,6 +1,8 @@ package de.peeeq.wurstscript.translation.lua.translation; import de.peeeq.wurstscript.WurstOperator; +import de.peeeq.wurstscript.ast.FuncDef; +import de.peeeq.wurstscript.ast.WPackage; import de.peeeq.wurstscript.attributes.CompileError; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.luaAst.*; @@ -92,15 +94,13 @@ public static LuaExpr translate(ImFunctionCall e, LuaTranslator tr) { return classFromIndex(arg, tr); } else if (tcFunc.equals("objectToIndex")) { return LuaAst.LuaExprFunctionCall(tr.toIndexFunction, LuaAst.LuaExprlist(arg)); - } else if (tcFunc.equals("objectFromIndex")) { - return LuaAst.LuaExprFunctionCall(tr.fromIndexFunction, LuaAst.LuaExprlist(arg)); } else if (tcFunc.equals("stringToIndex")) { return LuaAst.LuaExprFunctionCall(tr.stringToIndexFunction, LuaAst.LuaExprlist(arg)); } else if (tcFunc.equals("stringFromIndex")) { return LuaAst.LuaExprFunctionCall(tr.stringFromIndexFunction, LuaAst.LuaExprlist(arg)); } else if (LUA_HANDLE_TO_INDEX.contains(tcFunc)) { return LuaAst.LuaExprFunctionCall(tr.toIndexFunction, LuaAst.LuaExprlist(arg)); - } else if (LUA_HANDLE_FROM_INDEX.contains(tcFunc)) { + } else if (isObjectFromIndexName(tcFunc)) { return LuaAst.LuaExprFunctionCall(tr.fromIndexFunction, LuaAst.LuaExprlist(arg)); } } @@ -185,6 +185,22 @@ public static LuaExpr translate(ImFunctionCall e, LuaTranslator tr) { return LuaAst.LuaExprFunctionCall(f, tr.translateExprList(e.getArguments())); } + private static boolean isObjectFromIndexName(String typeCastingFunction) { + return "objectFromIndex".equals(typeCastingFunction) || LUA_HANDLE_FROM_INDEX.contains(typeCastingFunction); + } + + /** + * Whether a call of {@code fromIndex}, an old-generics conversion back to a handle, is printed as + * {@code __wurst_objectFromIndex}. That helper answers nil for nil, for 0 and for every number it + * never handed out, and indexes its table with the number it is given (Lua reads 3.0 as the key 3), + * so it needs no int normalisation in front of it: an old-generics slot holds nil or the index the + * matching toIndex gave. + */ + public static boolean printsAsObjectFromIndex(FuncDef fromIndex) { + return fromIndex.attrNearestPackage() instanceof WPackage p && "TypeCasting".equals(p.getName()) + && isObjectFromIndexName(fromIndex.getName()); + } + /** * Compiler-synthesised natives which the backend prints as an operator or a table store instead * of a call, so they have no definition. Recognised by node identity: an ordinary function of the diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 9a227a2c5..d930eb59b 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -7591,6 +7591,45 @@ public void concatenationOperandsWhichCannotBeNilLoseTheirGuardOnceInlined() thr assertTrue("a variable keeps its guard:\n" + greetAnyone, greetAnyone.contains("(name or \"\")")); } + /** + * An old-generics value read back as a handle goes through the TypeCasting fromIndex, which Lua + * prints as __wurst_objectFromIndex. That maps nil, 0 and an unknown index to nil and indexes its + * table with the integer it is given, so an int normalisation in front of it (tonumber, + * math.tointeger) answers the same and is not emitted. Jass has no such normalisation. + */ + @Test + public void oldGenericHandleReadsNeedNoIntNormalisation() throws IOException { + test().withStdLib().testLua(true).executeProg().lines( + "package Test", + "class Box", + " T elem", + " construct(T e)", + " elem = e", + " function get() returns T", + " return elem", + "class Unset", + " T elem", + "init", + " let t = CreateTimer()", + " let other = CreateTimer()", + " let full = new Box(t)", + " let empty = new Box(null)", + " let unset = new Unset()", + " if full.get() != t", + " testFail(\"the handle did not come back\")", + " if full.get() == other", + " testFail(\"another handle came back\")", + " if empty.get() != null", + " testFail(\"null (index 0) did not come back\")", + " if unset.elem != null", + " testFail(\"an unset slot (nil) did not read as null\")", + " testSuccess()"); + String init = topLevelFunctionBodyWithPrefix(compiledLua("oldGenericHandleReadsNeedNoIntNormalisation"), + "init_Test"); + assertTrue(init, init.contains("__wurst_objectFromIndex(") || init.contains("timerFromIndex(")); + assertFalse(init, init.contains("math.tointeger") || init.contains("tonumber") || init.contains("__wurst_ensureInt")); + } + /** -0.0 stays -0.0 under Lua's % as under fmod and its correction: 1 / r is minus infinity. */ @Test public void modKeepsTheSignOfANegativeZeroOnLua() throws IOException { From ad80a51d831a3a207a1fc00c317db75f38425372 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 02:19:49 +0200 Subject: [PATCH 08/12] RedundantFieldStores: a deallocation is a barrier, and only new objects count A deallocation fails on a double free and stops the thread, so a write before it stays: the first value would be what remains. And the first write is removed only for an object allocated in the same statement list: any other object may be null, where that write would fail itself and the statements after it would not run. The pass exists for the defaults written after an allocation, so it loses nothing there (castle fight: 2 of the removed writes stay). --- .../optimizer/RedundantFieldStores.java | 37 +++++++++++------ .../wurstscript/tests/OptimizerTests.java | 41 +++++++++++++++++++ 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java index fc7f25726..cc78768a5 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java @@ -9,14 +9,16 @@ import java.util.*; /** - * Removes a write of a constant to a field which the same statement list writes again before anything can read - * it. + * Removes a write of a constant to a field of an object allocated in the same statement list, which the list writes + * again before anything can read it. *

* Between the two writes there must be no read of the field through any object (so an alias cannot see the first * value), no call (a callee can read anything, a native can run Wurst code through an event), nothing which can stop - * the thread or leave the list ({@code exitwhen}, {@code return}: the first value would then be what remains) and no - * statement with statement lists of its own (whose paths are not followed). The object is told by value: a local copied from another names the same object until - * either is assigned again. An allocation reads no field and runs no Wurst code, so it is not in the way. + * the thread or leave the list (a division, a deallocation, which fails on a double free, {@code exitwhen}, + * {@code return}: the first value would then be what remains) and no statement with statement lists of its own + * (whose paths are not followed). The object is told by value: a local copied from another names the same object + * until either is assigned again. It must have been allocated in the list, so it is not null and the first write + * cannot fail either. An allocation reads no field and runs no Wurst code, so it is not in the way. *

* This removes the defaults an allocation writes on Lua where the constructor sets the field * ({@code LuaFieldDefaults}). Fields are only still fields on Lua: on Jass the class elimination has turned them into @@ -51,6 +53,7 @@ private record Slot(ImVar field, int object) { private void optimizeStatements(ImStmts stmts) { Map objectOf = new IdentityHashMap<>(); int[] nextObject = {0}; + Set allocated = new HashSet<>(); Map pending = new HashMap<>(); Set dead = Collections.newSetFromMap(new IdentityHashMap<>()); for (ImStmt s : stmts) { @@ -58,6 +61,7 @@ private void optimizeStatements(ImStmts stmts) { // Its paths can assign the locals, so their values are told apart afresh after it. pending.clear(); objectOf.clear(); + allocated.clear(); s.accept(new Element.DefaultVisitor() { @Override public void visit(ImStmts nested) { @@ -74,11 +78,11 @@ public void visit(ImStmts nested) { if (set.getLeft() instanceof ImMemberAccess target) { forgetReadFields(target.getReceiver(), pending); forgetReadFields(target.getIndexes(), pending); - if (target.getIndexes().isEmpty() + Integer object = target.getIndexes().isEmpty() && target.getReceiver() instanceof ImVarAccess receiver - && !receiver.getVar().isGlobal()) { - Slot slot = new Slot(trans.canonical(target.getVar()), - objectOf.computeIfAbsent(receiver.getVar(), v -> nextObject[0]++)); + && !receiver.getVar().isGlobal() ? objectOf.get(receiver.getVar()) : null; + if (object != null && allocated.contains(object)) { + Slot slot = new Slot(trans.canonical(target.getVar()), object); ImSet earlier = pending.remove(slot); if (earlier != null) { dead.add(earlier); @@ -94,7 +98,11 @@ public void visit(ImStmts nested) { objectOf.put(assigned.getVar(), objectOf.computeIfAbsent(copied.getVar(), v -> nextObject[0]++)); } else { - objectOf.put(assigned.getVar(), nextObject[0]++); + int object = nextObject[0]++; + objectOf.put(assigned.getVar(), object); + if (set.getRight() instanceof ImAlloc) { + allocated.add(object); + } } } } @@ -113,8 +121,8 @@ private static boolean hasStatementLists(ImStmt s) { } /** - * Whether running {@code s} can read any field or leave the list with the first value in place: a call, an - * operation which can stop the thread, or an {@code exitwhen} or {@code return}. + * Whether running {@code s} can read any field or leave the list with the first value in place: a call, a + * deallocation or a division, which can stop the thread, or an {@code exitwhen} or {@code return}. */ private static boolean isBarrier(ImStmt s) { if (s instanceof ImExitwhen || s instanceof ImReturn) { @@ -132,6 +140,11 @@ public void visit(ImMethodCall call) { barrier[0] = true; } + @Override + public void visit(ImDealloc dealloc) { + barrier[0] = true; + } + @Override public void visit(ImOperatorCall call) { super.visit(call); diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 797bf4f48..f702a8f64 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -4505,4 +4505,45 @@ public void inlinedReturnLoweringMatchesTheCalledFunction() { " if failures == 0", " testSuccess()"); } + + /** + * The pass behind the Lua field defaults, on hand-made IM: of {@code o.f = 0; ...; o.f = 2} the first write goes + * only when o was allocated in the same list and nothing between the writes can stop the thread. A deallocation + * can (a double free fails), and an object which was not allocated there may be null, so its first write could + * fail itself. + */ + @Test + public void aFieldWriteGoesOnlyForANewObjectWithNothingBetweenWhichCanStopTheThread() { + assertEquals(redundantFieldStoresLeave(true, false), 2, "o = alloc; o.f = 2"); + assertEquals(redundantFieldStoresLeave(true, true), 4, "a deallocation between keeps the first write"); + assertEquals(redundantFieldStoresLeave(false, false), 2, "an object not allocated here keeps it: o.f = 0; o.f = 2"); + } + + /** The statements left of {@code [o = alloc C;] o.f = 0; [dealloc o;] o.f = 2} after the pass. */ + private int redundantFieldStoresLeave(boolean allocate, boolean deallocate) { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, new RunArgs()); + ImVar field = JassIm.ImVar(model, TypesHelper.imInt(), "f", false); + ImClass c = JassIm.ImClass(model, "C", JassIm.ImTypeVars(), JassIm.ImVars(field), JassIm.ImMethods(), + JassIm.ImFunctions(), new ArrayList<>()); + translator.getImProg().getClasses().add(c); + ImVar o = JassIm.ImVar(model, JassIm.ImClassType(c, JassIm.ImTypeArguments()), "o", false); + ImStmts body = JassIm.ImStmts(); + if (allocate) { + body.add(JassIm.ImSet(model, JassIm.ImVarAccess(o), + JassIm.ImAlloc(model, JassIm.ImClassType(c, JassIm.ImTypeArguments())))); + } + body.add(JassIm.ImSet(model, JassIm.ImMemberAccess(model, JassIm.ImVarAccess(o), JassIm.ImTypeArguments(), + field, JassIm.ImExprs()), JassIm.ImIntVal(0))); + if (deallocate) { + body.add(JassIm.ImDealloc(model, JassIm.ImClassType(c, JassIm.ImTypeArguments()), JassIm.ImVarAccess(o))); + } + body.add(JassIm.ImSet(model, JassIm.ImMemberAccess(model, JassIm.ImVarAccess(o), JassIm.ImTypeArguments(), + field, JassIm.ImExprs()), JassIm.ImIntVal(2))); + ImFunction f = JassIm.ImFunction(model, "f", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImVoid(), + JassIm.ImVars(o), body, Collections.emptyList()); + translator.getImProg().getFunctions().add(f); + new de.peeeq.wurstscript.intermediatelang.optimizer.RedundantFieldStores().optimize(translator); + return f.getBody().size(); + } } From ae820357f3a0d930ad1fc6b46cfc07c547276304 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 02:38:47 +0200 Subject: [PATCH 09/12] Lua: one answer to what can raise, for the field-default pass and keyed writes Lua raises where Jass reads a default through a null or freed object: an array field is storage[o][i], which indexes nil, a field read gives nil, on which arithmetic and orderings raise, and a write under a nil key fails. LuaTraps answers whether an evaluation can raise (a call, a deallocation, a division, an array field read, arithmetic or an ordering on a field read or on a local assigned one), and both places which move or skip an evaluation use it: - RedundantFieldStores keeps a write when anything between it and the next write can raise, including a write through an object not allocated in the list or an array write under an index which may be nil. - A keyed-map store evaluates a value or table operand which can raise before the nil-key test, as the stub's argument was, instead of skipping it with a nil key. --- .../optimizer/RedundantFieldStores.java | 70 ++++++++-------- .../translation/imtranslation/LuaTraps.java | 80 +++++++++++++++++++ .../lua/translation/StmtTranslation.java | 6 ++ .../wurstscript/tests/LuaKeyedMapTests.java | 40 ++++++++++ .../wurstscript/tests/OptimizerTests.java | 74 ++++++++++++----- 5 files changed, 214 insertions(+), 56 deletions(-) create mode 100644 de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java index cc78768a5..81641d6c7 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java @@ -2,9 +2,9 @@ import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.translation.imoptimizer.OptimizerPass; -import de.peeeq.wurstscript.translation.imtranslation.Flatten; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; +import de.peeeq.wurstscript.translation.imtranslation.LuaTraps; import java.util.*; @@ -14,9 +14,9 @@ *

* Between the two writes there must be no read of the field through any object (so an alias cannot see the first * value), no call (a callee can read anything, a native can run Wurst code through an event), nothing which can stop - * the thread or leave the list (a division, a deallocation, which fails on a double free, {@code exitwhen}, - * {@code return}: the first value would then be what remains) and no statement with statement lists of its own - * (whose paths are not followed). The object is told by value: a local copied from another names the same object + * the thread or leave the list ({@link LuaTraps#mayRaise}, such as a division, a deallocation or arithmetic on a + * field read through a null object; {@code exitwhen}, {@code return}: the first value would then be what remains) + * and no statement with statement lists of its own (whose paths are not followed). The object is told by value: a local copied from another names the same object * until either is assigned again. It must have been allocated in the list, so it is not null and the first write * cannot fail either. An allocation reads no field and runs no Wurst code, so it is not in the way. *

@@ -54,6 +54,8 @@ private void optimizeStatements(ImStmts stmts) { Map objectOf = new IdentityHashMap<>(); int[] nextObject = {0}; Set allocated = new HashSet<>(); + // Locals assigned a field read in this list: nil when the object was null, and arithmetic on nil raises. + Set maybeNil = Collections.newSetFromMap(new IdentityHashMap<>()); Map pending = new HashMap<>(); Set dead = Collections.newSetFromMap(new IdentityHashMap<>()); for (ImStmt s : stmts) { @@ -62,6 +64,7 @@ private void optimizeStatements(ImStmts stmts) { pending.clear(); objectOf.clear(); allocated.clear(); + maybeNil.clear(); s.accept(new Element.DefaultVisitor() { @Override public void visit(ImStmts nested) { @@ -70,7 +73,7 @@ public void visit(ImStmts nested) { }); continue; } - if (isBarrier(s)) { + if (isBarrier(s, maybeNil)) { pending.clear(); } if (s instanceof ImSet set) { @@ -81,7 +84,11 @@ public void visit(ImStmts nested) { Integer object = target.getIndexes().isEmpty() && target.getReceiver() instanceof ImVarAccess receiver && !receiver.getVar().isGlobal() ? objectOf.get(receiver.getVar()) : null; - if (object != null && allocated.contains(object)) { + if (object == null || !allocated.contains(object)) { + // A write through an object which may be null raises (a nil table key), like a read of an + // array field through one. + pending.clear(); + } else { Slot slot = new Slot(trans.canonical(target.getVar()), object); ImSet earlier = pending.remove(slot); if (earlier != null) { @@ -93,7 +100,16 @@ public void visit(ImStmts nested) { } } else { forgetReadFields(set.getLeft(), pending); + if (set.getLeft() instanceof ImVarArrayAccess target && writesUnderAMaybeNilIndex(target, maybeNil)) { + pending.clear(); + } if (set.getLeft() instanceof ImVarAccess assigned && !assigned.getVar().isGlobal()) { + if (LuaTraps.mayBeNil(set.getRight()) || (set.getRight() instanceof ImVarAccess copiedVar + && maybeNil.contains(copiedVar.getVar()))) { + maybeNil.add(assigned.getVar()); + } else { + maybeNil.remove(assigned.getVar()); + } if (set.getRight() instanceof ImVarAccess copied && !copied.getVar().isGlobal()) { objectOf.put(assigned.getVar(), objectOf.computeIfAbsent(copied.getVar(), v -> nextObject[0]++)); @@ -121,39 +137,21 @@ private static boolean hasStatementLists(ImStmt s) { } /** - * Whether running {@code s} can read any field or leave the list with the first value in place: a call, a - * deallocation or a division, which can stop the thread, or an {@code exitwhen} or {@code return}. + * Whether running {@code s} can read any field or leave the list with the first value in place: anything which can + * raise ({@link LuaTraps#mayRaise}, a call among them), or an {@code exitwhen} or {@code return}. */ - private static boolean isBarrier(ImStmt s) { - if (s instanceof ImExitwhen || s instanceof ImReturn) { - return true; - } - boolean[] barrier = {false}; - s.accept(new Element.DefaultVisitor() { - @Override - public void visit(ImFunctionCall call) { - barrier[0] = true; - } - - @Override - public void visit(ImMethodCall call) { - barrier[0] = true; - } - - @Override - public void visit(ImDealloc dealloc) { - barrier[0] = true; - } + private static boolean isBarrier(ImStmt s, Set maybeNil) { + return s instanceof ImExitwhen || s instanceof ImReturn || LuaTraps.mayRaise(s, maybeNil::contains); + } - @Override - public void visit(ImOperatorCall call) { - super.visit(call); - if (Flatten.mayStopTheThread(call)) { - barrier[0] = true; - } + /** Whether an index of the array write {@code target} may be nil, which raises: a field read or such a local. */ + private static boolean writesUnderAMaybeNilIndex(ImVarArrayAccess target, Set maybeNil) { + for (ImExpr index : target.getIndexes()) { + if (LuaTraps.mayBeNil(index) || (index instanceof ImVarAccess v && maybeNil.contains(v.getVar()))) { + return true; } - }); - return barrier[0]; + } + return false; } /** Forgets the pending writes of every field {@code e} reads, through whatever object. */ diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java new file mode 100644 index 000000000..be0527d92 --- /dev/null +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java @@ -0,0 +1,80 @@ +package de.peeeq.wurstscript.translation.imtranslation; + +import de.peeeq.wurstscript.WurstOperator; +import de.peeeq.wurstscript.jassIm.*; + +import java.util.function.Predicate; + +/** + * What can raise a Lua error when it is evaluated, and so stop the thread at that point. + *

+ * A pass which removes or moves an evaluation must not move it past such a point: the code after it would then run + * where it did not, or a value would be left which the thread never saw replaced. Beyond what stops the thread on Jass + * too ({@link Flatten#mayStopTheThread}), Lua raises where Jass reads a default through an object which is null or + * freed: an array field is {@code storage[o][i]}, which indexes nil, and a field read gives nil, on which arithmetic + * and orderings raise. + */ +public final class LuaTraps { + + private LuaTraps() { + } + + /** + * Whether evaluating {@code e} can raise: it contains a call (the callee can), a deallocation (a double free + * fails), an operation which stops the thread on Jass too, a read of an array field, or arithmetic or an ordering + * on a value which may be nil: a field read, or a local which {@code maybeNil} says holds one. + */ + public static boolean mayRaise(Element e, Predicate maybeNil) { + boolean[] raises = {false}; + e.accept(new Element.DefaultVisitor() { + @Override + public void visit(ImFunctionCall call) { + raises[0] = true; + } + + @Override + public void visit(ImMethodCall call) { + raises[0] = true; + } + + @Override + public void visit(ImDealloc dealloc) { + raises[0] = true; + } + + @Override + public void visit(ImMemberAccess access) { + super.visit(access); + if (!access.getIndexes().isEmpty()) { + raises[0] = true; + } + } + + @Override + public void visit(ImOperatorCall call) { + super.visit(call); + if (Flatten.mayStopTheThread(call)) { + raises[0] = true; + } else if (!neverRaisesOnNil(call.getOp())) { + for (ImExpr operand : call.getArguments()) { + if (operand instanceof ImMemberAccess + || (operand instanceof ImVarAccess v && maybeNil.test(v.getVar()))) { + raises[0] = true; + } + } + } + } + }); + return raises[0]; + } + + /** Whether the expression {@code e} reads a value which is nil when its object is null: a field read. */ + public static boolean mayBeNil(ImExpr e) { + return e instanceof ImMemberAccess; + } + + private static boolean neverRaisesOnNil(WurstOperator op) { + return op == WurstOperator.EQ || op == WurstOperator.NOTEQ || op == WurstOperator.AND + || op == WurstOperator.OR || op == WurstOperator.NOT; + } +} diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java index c7ca3a43d..7a3d6d8f5 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java @@ -4,6 +4,7 @@ import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.luaAst.*; import de.peeeq.wurstscript.translation.imtranslation.LuaKeyedMap; +import de.peeeq.wurstscript.translation.imtranslation.LuaTraps; import de.peeeq.wurstscript.translation.lua.printing.LuaPrinter; import de.peeeq.wurstscript.types.TypesHelper; @@ -48,6 +49,11 @@ private static WriteOperand writeOperand(ImExpr e) { || e instanceof ImBoolVal || e instanceof ImNull) { return WriteOperand.STABLE; } + if (LuaTraps.mayRaise(e, v -> false)) { + // It must be evaluated even when the key is nil, as the stub's argument was: an array field read + // through a null object raises. + return WriteOperand.EFFECT; + } if (e instanceof ImVarAccess access) { return access.getVar().isGlobal() ? WriteOperand.READ : WriteOperand.STABLE; } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java index 49453c8aa..470a56336 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java @@ -1174,6 +1174,46 @@ public void keyedMapWritesEvaluateEachOperandOnceAndInOrder() throws IOException init.contains("__wurst_keyedMapPut") || init.contains("__wurst_keyedMapRemove")); } + /** + * A value which can raise, such as an array field read through an object which may be null, is evaluated before + * the nil-key test, as the stub's argument was: with a nil key it still runs. + */ + @Test + public void aKeyedMapWriteEvaluatesAValueWhichCanRaiseEvenForANilKey() throws IOException { + test().testLua(true).luaOnly(false).executeProg(true).withStdLib().lines( + "package KeyedMap", + "import Table", + "@compilerintrinsic public function keyedMapCreate() returns int", + " return (new Table()) castTo int", + "@compilerintrinsic public function keyedMapPut(int tbl, timer key, int value)", + " (tbl castTo Table).saveInt(GetHandleId(key), value)", + "@compilerintrinsic public function keyedMapGetInt(int tbl, timer key) returns int", + " return (tbl castTo Table).loadInt(GetHandleId(key))", + "endpackage", + "package Test", + "import KeyedMap", + "class Holder", + " int array[2] arr", + "init", + " let theMap = keyedMapCreate()", + " let h = new Holder()", + " h.arr[0] = 5", + " timer none = null", + " keyedMapPut(theMap, none, h.arr[0])", + " let key = CreateTimer()", + " keyedMapPut(theMap, key, h.arr[0])", + " if keyedMapGetInt(theMap, key) != 5", + " testFail(\"put did not store\")", + " testSuccess()", + "endpackage"); + + String init = getFunctionBody(compiled("aKeyedMapWriteEvaluatesAValueWhichCanRaiseEvenForANilKey"), "init_Test"); + java.util.regex.Matcher read = java.util.regex.Pattern + .compile("local (\\w+) = Holder_arr_storage\\[\\w+\\]\\[0\\]\\s*\\n\\s*if \\(\\w+ ~= nil\\) then") + .matcher(init); + assertTrue("the array field read is evaluated before the nil-key test: " + init, read.find()); + } + @Test public void keyedMapDestroyClearsLuaStoreWhileAliasRemains() { test().testLua(true).luaOnly(true).executeProg().withStdLib().lines( diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index f702a8f64..8c4908660 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -4507,43 +4507,77 @@ public void inlinedReturnLoweringMatchesTheCalledFunction() { } /** - * The pass behind the Lua field defaults, on hand-made IM: of {@code o.f = 0; ...; o.f = 2} the first write goes - * only when o was allocated in the same list and nothing between the writes can stop the thread. A deallocation - * can (a double free fails), and an object which was not allocated there may be null, so its first write could - * fail itself. + * The pass behind the Lua field defaults, on hand-made IM: of {@code o = alloc C; o.f = 0; ...; o.f = 2} the + * first write goes only when nothing between the writes can raise. A deallocation can (a double free), and so can + * reading an array field through an object which may be null ({@code storage[p][0]} indexes nil), arithmetic on a + * field read or on a local assigned one (nil), and a write through such an object (a nil table key). An object + * which was not allocated in the list may be null itself, so its first write could raise. */ @Test - public void aFieldWriteGoesOnlyForANewObjectWithNothingBetweenWhichCanStopTheThread() { - assertEquals(redundantFieldStoresLeave(true, false), 2, "o = alloc; o.f = 2"); - assertEquals(redundantFieldStoresLeave(true, true), 4, "a deallocation between keeps the first write"); - assertEquals(redundantFieldStoresLeave(false, false), 2, "an object not allocated here keeps it: o.f = 0; o.f = 2"); + public void aFieldWriteGoesOnlyForANewObjectWithNothingBetweenWhichCanRaise() { + assertEquals(redundantFieldStoresLeave(true, "nothing"), 2, "o = alloc; o.f = 2"); + assertEquals(redundantFieldStoresLeave(true, "arithmetic on locals"), 3, "a local sum does not raise"); + assertEquals(redundantFieldStoresLeave(true, "dealloc"), 4, "a deallocation between keeps the first write"); + assertEquals(redundantFieldStoresLeave(true, "array field read"), 4, "p.arr[0] can index nil"); + assertEquals(redundantFieldStoresLeave(true, "arithmetic on a field read"), 4, "p.g + 1 can add nil"); + assertEquals(redundantFieldStoresLeave(true, "arithmetic on a local read from a field"), 5, + "x = p.g; x + 1 can add nil"); + assertEquals(redundantFieldStoresLeave(true, "write through another object"), 4, "p.g = 1 can use a nil key"); + assertEquals(redundantFieldStoresLeave(false, "nothing"), 2, "an object not allocated here keeps it"); } - /** The statements left of {@code [o = alloc C;] o.f = 0; [dealloc o;] o.f = 2} after the pass. */ - private int redundantFieldStoresLeave(boolean allocate, boolean deallocate) { + /** The statements left of {@code [o = alloc C;] o.f = 0; ; o.f = 2} after the pass. */ + private int redundantFieldStoresLeave(boolean allocate, String middle) { WurstModel model = Ast.WurstModel(); ImTranslator translator = new ImTranslator(model, false, new RunArgs()); ImVar field = JassIm.ImVar(model, TypesHelper.imInt(), "f", false); - ImClass c = JassIm.ImClass(model, "C", JassIm.ImTypeVars(), JassIm.ImVars(field), JassIm.ImMethods(), - JassIm.ImFunctions(), new ArrayList<>()); + ImVar other = JassIm.ImVar(model, TypesHelper.imInt(), "g", false); + ImVar array = JassIm.ImVar(model, JassIm.ImArrayType(TypesHelper.imInt()), "arr", false); + ImClass c = JassIm.ImClass(model, "C", JassIm.ImTypeVars(), JassIm.ImVars(field, other, array), + JassIm.ImMethods(), JassIm.ImFunctions(), new ArrayList<>()); translator.getImProg().getClasses().add(c); ImVar o = JassIm.ImVar(model, JassIm.ImClassType(c, JassIm.ImTypeArguments()), "o", false); + ImVar p = JassIm.ImVar(model, JassIm.ImClassType(c, JassIm.ImTypeArguments()), "p", false); + ImVar x = JassIm.ImVar(model, TypesHelper.imInt(), "x", false); ImStmts body = JassIm.ImStmts(); if (allocate) { body.add(JassIm.ImSet(model, JassIm.ImVarAccess(o), JassIm.ImAlloc(model, JassIm.ImClassType(c, JassIm.ImTypeArguments())))); } - body.add(JassIm.ImSet(model, JassIm.ImMemberAccess(model, JassIm.ImVarAccess(o), JassIm.ImTypeArguments(), - field, JassIm.ImExprs()), JassIm.ImIntVal(0))); - if (deallocate) { - body.add(JassIm.ImDealloc(model, JassIm.ImClassType(c, JassIm.ImTypeArguments()), JassIm.ImVarAccess(o))); + body.add(JassIm.ImSet(model, member(model, o, field), JassIm.ImIntVal(0))); + switch (middle) { + case "nothing" -> { + } + case "arithmetic on locals" -> body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), plusOne(JassIm.ImVarAccess(x)))); + case "dealloc" -> body.add(JassIm.ImDealloc(model, JassIm.ImClassType(c, JassIm.ImTypeArguments()), + JassIm.ImVarAccess(o))); + case "array field read" -> body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), + JassIm.ImMemberAccess(model, JassIm.ImVarAccess(p), JassIm.ImTypeArguments(), array, + JassIm.ImExprs(JassIm.ImIntVal(0))))); + case "arithmetic on a field read" -> body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), + plusOne(member(model, p, other)))); + case "arithmetic on a local read from a field" -> { + body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), member(model, p, other))); + body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), plusOne(JassIm.ImVarAccess(x)))); + } + case "write through another object" -> body.add(JassIm.ImSet(model, member(model, p, other), + JassIm.ImIntVal(1))); + default -> throw new IllegalArgumentException(middle); } - body.add(JassIm.ImSet(model, JassIm.ImMemberAccess(model, JassIm.ImVarAccess(o), JassIm.ImTypeArguments(), - field, JassIm.ImExprs()), JassIm.ImIntVal(2))); - ImFunction f = JassIm.ImFunction(model, "f", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImVoid(), - JassIm.ImVars(o), body, Collections.emptyList()); + body.add(JassIm.ImSet(model, member(model, o, field), JassIm.ImIntVal(2))); + ImFunction f = JassIm.ImFunction(model, "f", JassIm.ImTypeVars(), JassIm.ImVars(p), JassIm.ImVoid(), + JassIm.ImVars(o, x), body, Collections.emptyList()); translator.getImProg().getFunctions().add(f); new de.peeeq.wurstscript.intermediatelang.optimizer.RedundantFieldStores().optimize(translator); return f.getBody().size(); } + + private static ImMemberAccess member(WurstModel model, ImVar receiver, ImVar field) { + return JassIm.ImMemberAccess(model, JassIm.ImVarAccess(receiver), JassIm.ImTypeArguments(), field, + JassIm.ImExprs()); + } + + private static ImExpr plusOne(ImExpr e) { + return JassIm.ImOperatorCall(de.peeeq.wurstscript.WurstOperator.PLUS, JassIm.ImExprs(e, JassIm.ImIntVal(1))); + } } From ac9f17c154b60a2e8e786c7b40394cf3af8ef46b Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 02:54:34 +0200 Subject: [PATCH 10/12] LuaTraps: decide by the kind of every node, a kind not named can raise mayRaise listed what raises, so each kind it did not know was taken as safe; an object's type id (`__wurst_objectClass[p].__typeId__`, nil for a null or freed p) was one. It now lists what cannot raise (constants, variable reads, field reads without an index, array reads, allocations, a class's type id, instanceof, tuples, and operators other than a division and arithmetic or an ordering on a value which may be nil) and takes every other kind as raising: a call, a deallocation, an object's type id, a cast, an array field read. --- .../translation/imtranslation/LuaTraps.java | 87 ++++++++++--------- .../wurstscript/tests/OptimizerTests.java | 9 +- 2 files changed, 53 insertions(+), 43 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java index be0527d92..77ac325de 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java @@ -11,8 +11,11 @@ * A pass which removes or moves an evaluation must not move it past such a point: the code after it would then run * where it did not, or a value would be left which the thread never saw replaced. Beyond what stops the thread on Jass * too ({@link Flatten#mayStopTheThread}), Lua raises where Jass reads a default through an object which is null or - * freed: an array field is {@code storage[o][i]}, which indexes nil, and a field read gives nil, on which arithmetic - * and orderings raise. + * freed: an array field is {@code storage[o][i]}, which indexes nil, an object's type id is read from its class + * descriptor, which is nil, and a field read gives nil, on which arithmetic and orderings raise. + *

+ * The answer is decided by the kind of each node, and a kind not named here can raise, so a new one is safe until it + * is shown not to. */ public final class LuaTraps { @@ -20,52 +23,54 @@ private LuaTraps() { } /** - * Whether evaluating {@code e} can raise: it contains a call (the callee can), a deallocation (a double free - * fails), an operation which stops the thread on Jass too, a read of an array field, or arithmetic or an ordering - * on a value which may be nil: a field read, or a local which {@code maybeNil} says holds one. + * Whether evaluating {@code e}, an expression or a statement which is not a nested block, can raise. It cannot + * when it consists only of constants, variable reads, field reads without an index, array reads, allocations, type + * ids of a class, instanceof tests and tuples, and of operators other than a division, and arithmetic or an + * ordering on a value which may be nil: a field read, or a local which {@code maybeNil} says holds one. Anything + * else can: a call, a deallocation (a double free fails), a type id read through an object, a cast, an array field + * read. */ public static boolean mayRaise(Element e, Predicate maybeNil) { - boolean[] raises = {false}; - e.accept(new Element.DefaultVisitor() { - @Override - public void visit(ImFunctionCall call) { - raises[0] = true; - } - - @Override - public void visit(ImMethodCall call) { - raises[0] = true; - } - - @Override - public void visit(ImDealloc dealloc) { - raises[0] = true; + if (e instanceof ImIntVal || e instanceof ImRealVal || e instanceof ImStringVal || e instanceof ImBoolVal + || e instanceof ImFuncRef || e instanceof ImNull || e instanceof ImVarAccess || e instanceof ImAlloc + || e instanceof ImTypeIdOfClass) { + return false; + } else if (e instanceof ImMemberAccess access) { + return !access.getIndexes().isEmpty() || mayRaise(access.getReceiver(), maybeNil); + } else if (e instanceof ImVarArrayAccess access) { + return anyMayRaise(access.getIndexes(), maybeNil); + } else if (e instanceof ImInstanceof instanceOf) { + // isInstanceOf answers false for an object without a class descriptor. + return mayRaise(instanceOf.getObj(), maybeNil); + } else if (e instanceof ImTupleExpr tuple) { + return anyMayRaise(tuple.getExprs(), maybeNil); + } else if (e instanceof ImTupleSelection selection) { + return mayRaise(selection.getTupleExpr(), maybeNil); + } else if (e instanceof ImOperatorCall call) { + if (Flatten.mayStopTheThread(call)) { + return true; } - - @Override - public void visit(ImMemberAccess access) { - super.visit(access); - if (!access.getIndexes().isEmpty()) { - raises[0] = true; + if (!neverRaisesOnNil(call.getOp())) { + for (ImExpr operand : call.getArguments()) { + if (mayBeNil(operand) || (operand instanceof ImVarAccess v && maybeNil.test(v.getVar()))) { + return true; + } } } + return anyMayRaise(call.getArguments(), maybeNil); + } else if (e instanceof ImSet set) { + return mayRaise(set.getLeft(), maybeNil) || mayRaise(set.getRight(), maybeNil); + } + return true; + } - @Override - public void visit(ImOperatorCall call) { - super.visit(call); - if (Flatten.mayStopTheThread(call)) { - raises[0] = true; - } else if (!neverRaisesOnNil(call.getOp())) { - for (ImExpr operand : call.getArguments()) { - if (operand instanceof ImMemberAccess - || (operand instanceof ImVarAccess v && maybeNil.test(v.getVar()))) { - raises[0] = true; - } - } - } + private static boolean anyMayRaise(ImExprs exprs, Predicate maybeNil) { + for (ImExpr e : exprs) { + if (mayRaise(e, maybeNil)) { + return true; } - }); - return raises[0]; + } + return false; } /** Whether the expression {@code e} reads a value which is nil when its object is null: a field read. */ diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 8c4908660..668a9cc94 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -4509,8 +4509,9 @@ public void inlinedReturnLoweringMatchesTheCalledFunction() { /** * The pass behind the Lua field defaults, on hand-made IM: of {@code o = alloc C; o.f = 0; ...; o.f = 2} the * first write goes only when nothing between the writes can raise. A deallocation can (a double free), and so can - * reading an array field through an object which may be null ({@code storage[p][0]} indexes nil), arithmetic on a - * field read or on a local assigned one (nil), and a write through such an object (a nil table key). An object + * reading an array field or the type id through an object which may be null ({@code storage[p][0]} and the class + * descriptor of p index nil), arithmetic on a field read or on a local assigned one (nil), and a write through such + * an object (a nil table key). An object * which was not allocated in the list may be null itself, so its first write could raise. */ @Test @@ -4523,6 +4524,8 @@ public void aFieldWriteGoesOnlyForANewObjectWithNothingBetweenWhichCanRaise() { assertEquals(redundantFieldStoresLeave(true, "arithmetic on a local read from a field"), 5, "x = p.g; x + 1 can add nil"); assertEquals(redundantFieldStoresLeave(true, "write through another object"), 4, "p.g = 1 can use a nil key"); + assertEquals(redundantFieldStoresLeave(true, "type id read through another object"), 4, + "p.typeId reads a nil class descriptor"); assertEquals(redundantFieldStoresLeave(false, "nothing"), 2, "an object not allocated here keeps it"); } @@ -4562,6 +4565,8 @@ private int redundantFieldStoresLeave(boolean allocate, String middle) { } case "write through another object" -> body.add(JassIm.ImSet(model, member(model, p, other), JassIm.ImIntVal(1))); + case "type id read through another object" -> body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), + JassIm.ImTypeIdOfObj(JassIm.ImVarAccess(p), JassIm.ImClassType(c, JassIm.ImTypeArguments())))); default -> throw new IllegalArgumentException(middle); } body.add(JassIm.ImSet(model, member(model, o, field), JassIm.ImIntVal(2))); From 5c3a45de025d9ea188b6068b3ee95d1b4862481b Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 03:07:34 +0200 Subject: [PATCH 11/12] LuaTraps: do not trace nil, take arithmetic on any variable as raising A nil read through a null object can reach an operand through any variable, a local, a global or a parameter, so tracing which ones hold one was never complete. Arithmetic, an ordering and an array write now count as raising unless their operands are literals; nothing is traced. It costs the real maps almost nothing: castle fight keeps 7 more writes (23,526, master 28,011), zombie defense 5, and DamageInstance's allocation still writes no field. --- .../optimizer/RedundantFieldStores.java | 34 +++--------- .../translation/imtranslation/LuaTraps.java | 53 ++++++++++--------- .../lua/translation/StmtTranslation.java | 2 +- .../tests/LuaBackendAuditTests.java | 9 ++-- .../wurstscript/tests/OptimizerTests.java | 18 +++++-- 5 files changed, 54 insertions(+), 62 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java index 81641d6c7..767406c9d 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/RedundantFieldStores.java @@ -14,9 +14,9 @@ *

* Between the two writes there must be no read of the field through any object (so an alias cannot see the first * value), no call (a callee can read anything, a native can run Wurst code through an event), nothing which can stop - * the thread or leave the list ({@link LuaTraps#mayRaise}, such as a division, a deallocation or arithmetic on a - * field read through a null object; {@code exitwhen}, {@code return}: the first value would then be what remains) - * and no statement with statement lists of its own (whose paths are not followed). The object is told by value: a local copied from another names the same object + * the thread or leave the list ({@link LuaTraps#mayRaise}, such as a division, a deallocation, arithmetic on a value + * which may be nil, or a write through an object which may be null; {@code exitwhen}, {@code return}: the first value + * would then be what remains) and no statement with statement lists of its own (whose paths are not followed). The object is told by value: a local copied from another names the same object * until either is assigned again. It must have been allocated in the list, so it is not null and the first write * cannot fail either. An allocation reads no field and runs no Wurst code, so it is not in the way. *

@@ -54,8 +54,6 @@ private void optimizeStatements(ImStmts stmts) { Map objectOf = new IdentityHashMap<>(); int[] nextObject = {0}; Set allocated = new HashSet<>(); - // Locals assigned a field read in this list: nil when the object was null, and arithmetic on nil raises. - Set maybeNil = Collections.newSetFromMap(new IdentityHashMap<>()); Map pending = new HashMap<>(); Set dead = Collections.newSetFromMap(new IdentityHashMap<>()); for (ImStmt s : stmts) { @@ -64,7 +62,6 @@ private void optimizeStatements(ImStmts stmts) { pending.clear(); objectOf.clear(); allocated.clear(); - maybeNil.clear(); s.accept(new Element.DefaultVisitor() { @Override public void visit(ImStmts nested) { @@ -73,7 +70,7 @@ public void visit(ImStmts nested) { }); continue; } - if (isBarrier(s, maybeNil)) { + if (isBarrier(s)) { pending.clear(); } if (s instanceof ImSet set) { @@ -100,16 +97,7 @@ public void visit(ImStmts nested) { } } else { forgetReadFields(set.getLeft(), pending); - if (set.getLeft() instanceof ImVarArrayAccess target && writesUnderAMaybeNilIndex(target, maybeNil)) { - pending.clear(); - } if (set.getLeft() instanceof ImVarAccess assigned && !assigned.getVar().isGlobal()) { - if (LuaTraps.mayBeNil(set.getRight()) || (set.getRight() instanceof ImVarAccess copiedVar - && maybeNil.contains(copiedVar.getVar()))) { - maybeNil.add(assigned.getVar()); - } else { - maybeNil.remove(assigned.getVar()); - } if (set.getRight() instanceof ImVarAccess copied && !copied.getVar().isGlobal()) { objectOf.put(assigned.getVar(), objectOf.computeIfAbsent(copied.getVar(), v -> nextObject[0]++)); @@ -140,18 +128,8 @@ private static boolean hasStatementLists(ImStmt s) { * Whether running {@code s} can read any field or leave the list with the first value in place: anything which can * raise ({@link LuaTraps#mayRaise}, a call among them), or an {@code exitwhen} or {@code return}. */ - private static boolean isBarrier(ImStmt s, Set maybeNil) { - return s instanceof ImExitwhen || s instanceof ImReturn || LuaTraps.mayRaise(s, maybeNil::contains); - } - - /** Whether an index of the array write {@code target} may be nil, which raises: a field read or such a local. */ - private static boolean writesUnderAMaybeNilIndex(ImVarArrayAccess target, Set maybeNil) { - for (ImExpr index : target.getIndexes()) { - if (LuaTraps.mayBeNil(index) || (index instanceof ImVarAccess v && maybeNil.contains(v.getVar()))) { - return true; - } - } - return false; + private static boolean isBarrier(ImStmt s) { + return s instanceof ImExitwhen || s instanceof ImReturn || LuaTraps.mayRaise(s); } /** Forgets the pending writes of every field {@code e} reads, through whatever object. */ diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java index 77ac325de..3458c6e54 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaTraps.java @@ -3,8 +3,6 @@ import de.peeeq.wurstscript.WurstOperator; import de.peeeq.wurstscript.jassIm.*; -import java.util.function.Predicate; - /** * What can raise a Lua error when it is evaluated, and so stop the thread at that point. *

@@ -12,10 +10,12 @@ * where it did not, or a value would be left which the thread never saw replaced. Beyond what stops the thread on Jass * too ({@link Flatten#mayStopTheThread}), Lua raises where Jass reads a default through an object which is null or * freed: an array field is {@code storage[o][i]}, which indexes nil, an object's type id is read from its class - * descriptor, which is nil, and a field read gives nil, on which arithmetic and orderings raise. + * descriptor, which is nil, and a field read gives nil, on which arithmetic and orderings raise, as does a table + * write under a nil key. A nil read can reach an operand through any variable, so whether an operand is nil is not + * traced: arithmetic, an ordering and an array write count as raising unless their operands are literals. *

- * The answer is decided by the kind of each node, and a kind not named here can raise, so a new one is safe until it - * is shown not to. + * The answer is decided by the kind of each node, and a kind not named here counts as raising until it is shown not + * to. */ public final class LuaTraps { @@ -25,59 +25,62 @@ private LuaTraps() { /** * Whether evaluating {@code e}, an expression or a statement which is not a nested block, can raise. It cannot * when it consists only of constants, variable reads, field reads without an index, array reads, allocations, type - * ids of a class, instanceof tests and tuples, and of operators other than a division, and arithmetic or an - * ordering on a value which may be nil: a field read, or a local which {@code maybeNil} says holds one. Anything - * else can: a call, a deallocation (a double free fails), a type id read through an object, a cast, an array field - * read. + * ids of a class, instanceof tests and tuples, of {@code ==}, {@code !=}, {@code and}, {@code or} and {@code not}, + * of arithmetic and orderings on literals, and of assignments to a variable or a field. Anything else can: a call, + * a deallocation (a double free fails), a division, arithmetic or an ordering on a value which is not a literal (it + * may be nil), an array write under an index which is not a literal, a type id read through an object, a cast, an + * array field read. A field write through an object which may be null raises too; that is for the caller to tell. */ - public static boolean mayRaise(Element e, Predicate maybeNil) { + public static boolean mayRaise(Element e) { if (e instanceof ImIntVal || e instanceof ImRealVal || e instanceof ImStringVal || e instanceof ImBoolVal || e instanceof ImFuncRef || e instanceof ImNull || e instanceof ImVarAccess || e instanceof ImAlloc || e instanceof ImTypeIdOfClass) { return false; } else if (e instanceof ImMemberAccess access) { - return !access.getIndexes().isEmpty() || mayRaise(access.getReceiver(), maybeNil); + return !access.getIndexes().isEmpty() || mayRaise(access.getReceiver()); } else if (e instanceof ImVarArrayAccess access) { - return anyMayRaise(access.getIndexes(), maybeNil); + return anyMayRaise(access.getIndexes()); } else if (e instanceof ImInstanceof instanceOf) { // isInstanceOf answers false for an object without a class descriptor. - return mayRaise(instanceOf.getObj(), maybeNil); + return mayRaise(instanceOf.getObj()); } else if (e instanceof ImTupleExpr tuple) { - return anyMayRaise(tuple.getExprs(), maybeNil); + return anyMayRaise(tuple.getExprs()); } else if (e instanceof ImTupleSelection selection) { - return mayRaise(selection.getTupleExpr(), maybeNil); + return mayRaise(selection.getTupleExpr()); } else if (e instanceof ImOperatorCall call) { if (Flatten.mayStopTheThread(call)) { return true; } if (!neverRaisesOnNil(call.getOp())) { for (ImExpr operand : call.getArguments()) { - if (mayBeNil(operand) || (operand instanceof ImVarAccess v && maybeNil.test(v.getVar()))) { + if (!(operand instanceof ImConst)) { return true; } } } - return anyMayRaise(call.getArguments(), maybeNil); + return anyMayRaise(call.getArguments()); } else if (e instanceof ImSet set) { - return mayRaise(set.getLeft(), maybeNil) || mayRaise(set.getRight(), maybeNil); + if (set.getLeft() instanceof ImVarArrayAccess target) { + for (ImExpr index : target.getIndexes()) { + if (!(index instanceof ImConst)) { + return true; + } + } + } + return mayRaise(set.getLeft()) || mayRaise(set.getRight()); } return true; } - private static boolean anyMayRaise(ImExprs exprs, Predicate maybeNil) { + private static boolean anyMayRaise(ImExprs exprs) { for (ImExpr e : exprs) { - if (mayRaise(e, maybeNil)) { + if (mayRaise(e)) { return true; } } return false; } - /** Whether the expression {@code e} reads a value which is nil when its object is null: a field read. */ - public static boolean mayBeNil(ImExpr e) { - return e instanceof ImMemberAccess; - } - private static boolean neverRaisesOnNil(WurstOperator op) { return op == WurstOperator.EQ || op == WurstOperator.NOTEQ || op == WurstOperator.AND || op == WurstOperator.OR || op == WurstOperator.NOT; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java index 7a3d6d8f5..1dae44bcc 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java @@ -49,7 +49,7 @@ private static WriteOperand writeOperand(ImExpr e) { || e instanceof ImBoolVal || e instanceof ImNull) { return WriteOperand.STABLE; } - if (LuaTraps.mayRaise(e, v -> false)) { + if (LuaTraps.mayRaise(e)) { // It must be evaluated even when the key is nil, as the stub's argument was: an array field read // through a null object raises. return WriteOperand.EFFECT; diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index d930eb59b..a57f6ae52 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -7326,7 +7326,8 @@ public void closureMethodsWithUnderscoresKeepTheirSemanticNames() throws IOExcep /** * A new object's field defaults are IM writes after its allocation, so where the constructor sets a field the - * default is gone: the allocation writes no field, and each field is written once where the object is made. + * default is gone: the allocation writes no field, and each field is written once where the object is made. (A + * value computed between them, which may raise on nil, would keep the defaults after it: OptimizerTests.) */ @Test public void aConstructedFieldIsWrittenOnceNotFirstWithItsDefault() throws IOException { @@ -7339,14 +7340,14 @@ public void aConstructedFieldIsWrittenOnceNotFirstWithItsDefault() throws IOExce " boolean crit", " string label", " Hit next", - " construct(int amount, real factor)", + " construct(int amount, real factor, boolean crit)", " this.amount = amount", " this.factor = factor", - " this.crit = amount > 10", + " this.crit = crit", " this.label = \"hit\"", " this.next = null", "@noinline function make(int amount) returns Hit", - " return new Hit(amount, 1.5)", + " return new Hit(amount, 1.5, amount > 10)", "init", " let h = make(12)", " if h.amount == 12 and h.factor == 1.5 and h.crit and h.label == \"hit\" and h.next == null", diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 668a9cc94..dd8215c1e 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -4510,19 +4510,22 @@ public void inlinedReturnLoweringMatchesTheCalledFunction() { * The pass behind the Lua field defaults, on hand-made IM: of {@code o = alloc C; o.f = 0; ...; o.f = 2} the * first write goes only when nothing between the writes can raise. A deallocation can (a double free), and so can * reading an array field or the type id through an object which may be null ({@code storage[p][0]} and the class - * descriptor of p index nil), arithmetic on a field read or on a local assigned one (nil), and a write through such - * an object (a nil table key). An object + * descriptor of p index nil), arithmetic on anything but literals (a nil read can reach any variable), and a write + * through such an object (a nil table key). An object * which was not allocated in the list may be null itself, so its first write could raise. */ @Test public void aFieldWriteGoesOnlyForANewObjectWithNothingBetweenWhichCanRaise() { assertEquals(redundantFieldStoresLeave(true, "nothing"), 2, "o = alloc; o.f = 2"); - assertEquals(redundantFieldStoresLeave(true, "arithmetic on locals"), 3, "a local sum does not raise"); + assertEquals(redundantFieldStoresLeave(true, "arithmetic on literals"), 3, "1 + 2 does not raise"); + assertEquals(redundantFieldStoresLeave(true, "arithmetic on a local"), 4, "x + 1 can add nil: x may hold one"); assertEquals(redundantFieldStoresLeave(true, "dealloc"), 4, "a deallocation between keeps the first write"); assertEquals(redundantFieldStoresLeave(true, "array field read"), 4, "p.arr[0] can index nil"); assertEquals(redundantFieldStoresLeave(true, "arithmetic on a field read"), 4, "p.g + 1 can add nil"); assertEquals(redundantFieldStoresLeave(true, "arithmetic on a local read from a field"), 5, "x = p.g; x + 1 can add nil"); + assertEquals(redundantFieldStoresLeave(true, "arithmetic on a global read from a field"), 5, + "G = p.g; G + 1 can add nil"); assertEquals(redundantFieldStoresLeave(true, "write through another object"), 4, "p.g = 1 can use a nil key"); assertEquals(redundantFieldStoresLeave(true, "type id read through another object"), 4, "p.typeId reads a nil class descriptor"); @@ -4551,7 +4554,14 @@ private int redundantFieldStoresLeave(boolean allocate, String middle) { switch (middle) { case "nothing" -> { } - case "arithmetic on locals" -> body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), plusOne(JassIm.ImVarAccess(x)))); + case "arithmetic on literals" -> body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), plusOne(JassIm.ImIntVal(1)))); + case "arithmetic on a local" -> body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), plusOne(JassIm.ImVarAccess(x)))); + case "arithmetic on a global read from a field" -> { + ImVar g = JassIm.ImVar(model, TypesHelper.imInt(), "G", false); + translator.getImProg().getGlobals().add(g); + body.add(JassIm.ImSet(model, JassIm.ImVarAccess(g), member(model, p, other))); + body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), plusOne(JassIm.ImVarAccess(g)))); + } case "dealloc" -> body.add(JassIm.ImDealloc(model, JassIm.ImClassType(c, JassIm.ImTypeArguments()), JassIm.ImVarAccess(o))); case "array field read" -> body.add(JassIm.ImSet(model, JassIm.ImVarAccess(x), From dec198a86d17b96fb4bac1683aa608e86781c4b8 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 10 Oct 2026 03:21:44 +0200 Subject: [PATCH 12/12] Lua keyed writes: only a literal key goes without the nil test An int key read through a null object is nil on Lua, so an unguarded store raised where the stub stored nothing. As in LuaTraps, a nil read can reach any variable, so every key but a literal keeps the test; the store is still inline, a comparison instead of a call. --- .../lua/translation/StmtTranslation.java | 12 +++---- .../wurstscript/tests/LuaKeyedMapTests.java | 31 +++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java index 1dae44bcc..f0cde2434 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/StmtTranslation.java @@ -6,7 +6,6 @@ import de.peeeq.wurstscript.translation.imtranslation.LuaKeyedMap; import de.peeeq.wurstscript.translation.imtranslation.LuaTraps; import de.peeeq.wurstscript.translation.lua.printing.LuaPrinter; -import de.peeeq.wurstscript.types.TypesHelper; import java.util.ArrayList; import java.util.List; @@ -79,9 +78,10 @@ private static boolean readsOnly(ImExprs exprs) { /** * A keyed-map put or remove as the table store it stands for, {@code t[k] = v} or * {@code t[k] = nil}, instead of a call to a stub with that body. Storing under a nil key is an - * error in Lua, where reading one only answers nil, so a key which may be nil (a handle, a - * string, an erased element) is tested first: a null element stores nothing and reads as absent. - * An int key or a literal is never nil and is stored without the test. + * error in Lua, where reading one only answers nil, so a key which may be nil is tested first: a + * null element stores nothing and reads as absent. Any key but a literal may be nil, an int too + * when it is a field read through an object which is null, so only a literal is stored without + * the test. * *

Each operand is evaluated once and in the order of the call. A plain store does that by * itself. The test reads the key twice and evaluates the table and the value only when the key @@ -98,8 +98,8 @@ private static void translateKeyedMapWrite(ImFunctionCall call, String stub, Lis "Lua backend: " + stub + " expects " + arity + " arguments, got " + args.size() + "."); } ImExpr key = args.get(1); - boolean keyNeverNil = TypesHelper.isIntType(key.attrTyp()) - || key instanceof ImIntVal || key instanceof ImRealVal || key instanceof ImStringVal + // Only a literal: a key of any type can hold nil, read through an object which is null (LuaTraps). + boolean keyNeverNil = key instanceof ImIntVal || key instanceof ImRealVal || key instanceof ImStringVal || key instanceof ImBoolVal; List operands = new ArrayList<>(); for (ImExpr arg : args) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java index 470a56336..f1a0668de 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaKeyedMapTests.java @@ -1214,6 +1214,37 @@ public void aKeyedMapWriteEvaluatesAValueWhichCanRaiseEvenForANilKey() throws IO assertTrue("the array field read is evaluated before the nil-key test: " + init, read.find()); } + /** + * An int key read through a null object is nil on Lua, so an int key which is not a literal keeps the nil test: + * the store does nothing, as the stub did, instead of raising on a nil table key. (Jass reads the field as 0 and + * stores under 0; Lua only.) + */ + @Test + public void aKeyedMapWriteUnderAnIntKeyReadThroughANullObjectStoresNothing() { + test().testLua(true).executeProg(true).withStdLib().lines( + "package KeyedMap", + "import Table", + "@compilerintrinsic public function keyedMapCreate() returns int", + " return (new Table()) castTo int", + "@compilerintrinsic public function keyedMapPut(int tbl, int key, int value)", + " (tbl castTo Table).saveInt(key, value)", + "@compilerintrinsic public function keyedMapGetInt(int tbl, int key) returns int", + " return (tbl castTo Table).loadInt(key)", + "endpackage", + "package Test", + "import KeyedMap", + "class Holder", + " int id", + "init", + " let theMap = keyedMapCreate()", + " Holder none = null", + " keyedMapPut(theMap, none.id, 7)", + " keyedMapPut(theMap, 3, 4)", + " if keyedMapGetInt(theMap, 3) == 4 and keyedMapGetInt(theMap, 0) == 0", + " testSuccess()", + "endpackage"); + } + @Test public void keyedMapDestroyClearsLuaStoreWhileAliasRemains() { test().testLua(true).luaOnly(true).executeProg().withStdLib().lines(