From 1290de5a8ed5f9748e13477b22b2abfb7154d5d3 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 09:16:01 +0200 Subject: [PATCH 1/4] Project a keyed set's element to an integer key on Jass. --- .../peeeq/wurstio/WurstCompilerJassImpl.java | 7 ++ .../imtranslation/JassKeyOfLowering.java | 113 ++++++++++++++++++ .../wurstscript/tests/KeyedTableTests.java | 111 +++++++++++++++++ 3 files changed, 231 insertions(+) create mode 100644 de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java create mode 100644 de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.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 0d1efb8f9..57edf33d2 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 @@ -409,6 +409,13 @@ public JassProg transformProgToJass() { printDebugImProg("./test-output/im " + stage++ + "_classesEliminated.im"); timeTaker.endPhase(); + // Generic elimination has made each specialisation's element type concrete and classes are + // integers by now, so the integer key a Jass keyed set needs follows from the type. Before + // inlining, so every call site agrees on one body. + beginPhase(2, "lower keyed-set key projection"); + JassKeyOfLowering.transform(imProg2); + timeTaker.endPhase(); + if (!runArgs.isNoDebugMessages() && runArgs.isIncludeStacktraces()) { beginPhase(4, "add stack traces"); new StackTraceInjector2(imProg2, imTranslator2).transform(timeTaker); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java new file mode 100644 index 000000000..adba84991 --- /dev/null +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java @@ -0,0 +1,113 @@ +package de.peeeq.wurstscript.translation.imtranslation; + +import de.peeeq.wurstscript.CompilerIntrinsics; +import de.peeeq.wurstscript.ast.FuncDef; +import de.peeeq.wurstscript.attributes.CompileError; +import de.peeeq.wurstscript.jassIm.*; +import de.peeeq.wurstscript.types.TypesHelper; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Gives {@code wurstKeyOf} a body on Jass, so a keyed set works on both backends. + * + *

Jass has no hashing, so a keyed structure needs an integer key. A {@code T:} type parameter + * cannot be projected to one in source - that is why SparseSet asks its caller for a + * {@code SparseSetKey} - but after generic elimination each specialisation has a concrete + * parameter type, and the projection follows from it. + * + *

The projections are ordinary Wurst functions in the intrinsic's own package rather than + * natives synthesised here. A synthesised IS_NATIVE stub would be emitted as a {@code native} + * declaration by ImToJassTranslator unless flagged BJ or extern, redeclaring a common.j native and + * failing pjass. Functions the library already references are in the IM, declared correctly, and + * emitted by the normal path - so this pass only has to pick one. + * + *

On Lua this never runs: {@link LuaKeyedTable} replaces the keyed-table operations wholesale + * before the inliner, and the element is its own key there, so no projection exists to make. + */ +public final class JassKeyOfLowering { + + /** The intrinsic being given a body. */ + private static final String KEY_OF = "wurstKeyOf"; + + /** Projections it can be rewritten to, each an ordinary function in the same package. */ + private static final String KEY_OF_INT = "keyOfInt"; + private static final String KEY_OF_HANDLE = "keyOfHandle"; + private static final String KEY_OF_STRING = "keyOfString"; + + private JassKeyOfLowering() { + } + + public static void transform(ImProg prog) { + Map projections = new LinkedHashMap<>(); + for (ImFunction f : prog.getFunctions()) { + String name = annotatedName(f); + if (KEY_OF_INT.equals(name) || KEY_OF_HANDLE.equals(name) || KEY_OF_STRING.equals(name)) { + projections.put(name, f); + } + } + + for (ImFunction f : prog.getFunctions()) { + if (!KEY_OF.equals(annotatedName(f)) || f.getParameters().size() != 1) { + continue; + } + ImVar value = f.getParameters().get(0); + ImFunction projection = projections.get(projectionFor(value.getType(), f)); + if (projection == null) { + // The library is expected to declare all three next to the intrinsic; without them + // there is nothing to call, and silently leaving the original body would ship a + // keyed set that does not key on anything. + throw new CompileError(f.attrTrace().attrErrorPos(), + "The KeyedTable package must declare keyOfInt, keyOfHandle and keyOfString " + + "alongside " + KEY_OF + "."); + } + f.getBody().clear(); + f.getLocals().clear(); + f.getBody().add(JassIm.ImReturn(f.attrTrace(), JassIm.ImFunctionCall( + f.attrTrace(), projection, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImVarAccess(value)), + false, CallType.NORMAL))); + } + } + + /** Which projection a concrete element type needs. */ + private static String projectionFor(ImType t, ImFunction f) { + if (TypesHelper.isRealType(t) || TypesHelper.isBoolType(t)) { + throw new CompileError(f.attrTrace().attrErrorPos(), + "A keyed set cannot use " + typeNameOf(t) + " as its element type on Jass: it has " + + "no stable integer key. Use int, string, a handle or a class, or restrict " + + "the set to Lua."); + } + if (isCodeType(t)) { + throw new CompileError(f.attrTrace().attrErrorPos(), + "A keyed set cannot use code as its element type: function references have no " + + "stable identity to key on."); + } + if (TypesHelper.isStringType(t)) { + return KEY_OF_STRING; + } + // Class instances are integers by this point, and so is int itself. + if (TypesHelper.isIntType(t) || t instanceof ImClassType) { + return KEY_OF_INT; + } + // Everything left is a handle type, whose id is its key. + return KEY_OF_HANDLE; + } + + private static boolean isCodeType(ImType t) { + return t instanceof ImSimpleType st && "code".equals(st.getTypename()); + } + + private static String typeNameOf(ImType t) { + return t instanceof ImSimpleType st ? st.getTypename() : t.toString(); + } + + /** The source name of {@code f} if it is a compiler intrinsic declaration, else null. */ + private static String annotatedName(ImFunction f) { + return f.attrTrace() instanceof FuncDef fd + && fd.attrHasAnnotation(CompilerIntrinsics.ANNOTATION) + ? fd.getName() + : null; + } +} diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java new file mode 100644 index 000000000..4136ac68f --- /dev/null +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java @@ -0,0 +1,111 @@ +package tests.wurstscript.tests; + +import org.testng.annotations.Test; + +/** + * The Jass side of the keyed-table intrinsics. + * + *

On Lua the element is its own table key and there is nothing to project. Jass has no hashing, + * so a keyed structure needs an integer, and a {@code T:} parameter cannot be projected to one in + * source. JassKeyOfLowering fills that in after generic elimination, when each specialisation's + * element type is concrete. + */ +public class KeyedTableTests extends WurstScriptTest { + + /** The intrinsic and the projections it is rewritten to, declared as the library would. */ + private static String[] withKeyedTable(String... usage) { + java.util.List lines = new java.util.ArrayList<>(java.util.Arrays.asList( + "package KeyedTable", + "@compilerintrinsic public function wurstKeyOf(T value) returns int", + " return 0", + "@compilerintrinsic public function keyOfInt(int v) returns int", + " return v", + "@compilerintrinsic public function keyOfHandle(handle h) returns int", + " return GetHandleId(h)", + "@compilerintrinsic public function keyOfString(string s) returns int", + " return StringHash(s)", + "endpackage")); + lines.addAll(java.util.Arrays.asList(usage)); + return lines.toArray(new String[0]); + } + + /** An int is its own key. */ + @Test + public void intKeyIsIdentity() { + test().executeProg(true).executeProgOnlyAfterTransforms().withStdLib().lines(withKeyedTable( + "package Test", + "import KeyedTable", + "init", + " wurstKeyOf(7).assertEquals(7)", + " wurstKeyOf(-3).assertEquals(-3)", + " testSuccess()", + "endpackage")); + } + + /** A handle is keyed by its id, which is what makes unit membership work on Jass. */ + @Test + public void handleKeyIsItsHandleId() { + test().executeProg(true).executeProgOnlyAfterTransforms().withStdLib().lines(withKeyedTable( + "package Test", + "import KeyedTable", + "init", + " let u = CreateUnit(Player(0), 'hfoo', 0., 0., 0.)", + " wurstKeyOf(u).assertEquals(GetHandleId(u))", + " let v = CreateUnit(Player(0), 'hfoo', 0., 0., 0.)", + " (wurstKeyOf(u) != wurstKeyOf(v)).assertTrue()", + " testSuccess()", + "endpackage")); + } + + /** A class instance is already an integer by the time the projection is chosen. */ + @Test + public void classInstanceKeyIsStable() { + test().executeProg(true).executeProgOnlyAfterTransforms().withStdLib().lines(withKeyedTable( + "package Test", + "import KeyedTable", + "class Marker", + "init", + " let a = new Marker()", + " let b = new Marker()", + " wurstKeyOf(a).assertEquals(wurstKeyOf(a))", + " (wurstKeyOf(a) != wurstKeyOf(b)).assertTrue()", + " testSuccess()", + "endpackage")); + } + + @Test + public void stringKeyIsItsHash() { + test().executeProg(true).executeProgOnlyAfterTransforms().withStdLib().lines(withKeyedTable( + "package Test", + "import KeyedTable", + "init", + " wurstKeyOf(\"abc\").assertEquals(StringHash(\"abc\"))", + " testSuccess()", + "endpackage")); + } + + /** real has no stable integer key, and saying so beats keying on a truncation. */ + @Test + public void realElementIsRejected() { + testAssertErrorsLinesWithStdLib(false, "cannot use real as its element type", withKeyedTable( + "package Test", + "import KeyedTable", + "init", + " let k = wurstKeyOf(1.5)", + " if k > 0", + " skip", + "endpackage")); + } + + @Test + public void booleanElementIsRejected() { + testAssertErrorsLinesWithStdLib(false, "cannot use boolean as its element type", withKeyedTable( + "package Test", + "import KeyedTable", + "init", + " let k = wurstKeyOf(true)", + " if k > 0", + " skip", + "endpackage")); + } +} From eec0ee41abcc51e4f3ef727be7bab573b395a508 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 10:11:42 +0200 Subject: [PATCH 2/4] Reject string and tuple keys on Jass, and resolve projections within the declaring package. --- .../imtranslation/JassKeyOfLowering.java | 78 +++++++++++++------ .../wurstscript/tests/KeyedTableTests.java | 54 +++++++++++-- 2 files changed, 102 insertions(+), 30 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java index adba84991..fe3984204 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java @@ -2,13 +2,11 @@ import de.peeeq.wurstscript.CompilerIntrinsics; 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.types.TypesHelper; -import java.util.LinkedHashMap; -import java.util.Map; - /** * Gives {@code wurstKeyOf} a body on Jass, so a keyed set works on both backends. * @@ -34,34 +32,18 @@ public final class JassKeyOfLowering { /** Projections it can be rewritten to, each an ordinary function in the same package. */ private static final String KEY_OF_INT = "keyOfInt"; private static final String KEY_OF_HANDLE = "keyOfHandle"; - private static final String KEY_OF_STRING = "keyOfString"; private JassKeyOfLowering() { } public static void transform(ImProg prog) { - Map projections = new LinkedHashMap<>(); - for (ImFunction f : prog.getFunctions()) { - String name = annotatedName(f); - if (KEY_OF_INT.equals(name) || KEY_OF_HANDLE.equals(name) || KEY_OF_STRING.equals(name)) { - projections.put(name, f); - } - } - for (ImFunction f : prog.getFunctions()) { if (!KEY_OF.equals(annotatedName(f)) || f.getParameters().size() != 1) { continue; } ImVar value = f.getParameters().get(0); - ImFunction projection = projections.get(projectionFor(value.getType(), f)); - if (projection == null) { - // The library is expected to declare all three next to the intrinsic; without them - // there is nothing to call, and silently leaving the original body would ship a - // keyed set that does not key on anything. - throw new CompileError(f.attrTrace().attrErrorPos(), - "The KeyedTable package must declare keyOfInt, keyOfHandle and keyOfString " - + "alongside " + KEY_OF + "."); - } + ImFunction projection = + findProjection(prog, packageOf(f), projectionFor(value.getType(), f), f); f.getBody().clear(); f.getLocals().clear(); f.getBody().add(JassIm.ImReturn(f.attrTrace(), JassIm.ImFunctionCall( @@ -71,13 +53,47 @@ public static void transform(ImProg prog) { } } + /** + * The projection of that name declared beside the intrinsic itself. + * + *

Scoped to the declaring package rather than matched by name across the program: the name + * is not identity, and a same-named annotated function in another package would otherwise win + * or lose by traversal order and silently key every set on something else. The signature is + * checked for the same reason - a helper of the wrong shape produces malformed IM rather than + * an error anyone can read. + */ + private static ImFunction findProjection(ImProg prog, WPackage owner, String name, ImFunction f) { + if (owner != null) { + for (ImFunction candidate : prog.getFunctions()) { + if (!name.equals(annotatedName(candidate)) || packageOf(candidate) != owner) { + continue; + } + if (candidate.getParameters().size() != 1 + || !TypesHelper.isIntType(candidate.getReturnType())) { + throw new CompileError(candidate.attrTrace().attrErrorPos(), + name + " must take exactly one parameter and return int."); + } + return candidate; + } + } + throw new CompileError(f.attrTrace().attrErrorPos(), + "The package declaring " + KEY_OF + " must also declare " + name + "."); + } + + /** The package a compiler-intrinsic declaration belongs to, or null if it has no trace. */ + private static WPackage packageOf(ImFunction f) { + return f.attrTrace() instanceof FuncDef fd && fd.attrNearestPackage() instanceof WPackage p + ? p + : null; + } + /** Which projection a concrete element type needs. */ private static String projectionFor(ImType t, ImFunction f) { if (TypesHelper.isRealType(t) || TypesHelper.isBoolType(t)) { throw new CompileError(f.attrTrace().attrErrorPos(), "A keyed set cannot use " + typeNameOf(t) + " as its element type on Jass: it has " - + "no stable integer key. Use int, string, a handle or a class, or restrict " - + "the set to Lua."); + + "no stable integer key. Use int, a handle or a class, or restrict the set " + + "to Lua."); } if (isCodeType(t)) { throw new CompileError(f.attrTrace().attrErrorPos(), @@ -85,7 +101,21 @@ private static String projectionFor(ImType t, ImFunction f) { + "stable identity to key on."); } if (TypesHelper.isStringType(t)) { - return KEY_OF_STRING; + // StringHash is not identity. This repository's own MultibyteDiagnostics records that + // it collapses whole classes of strings to one marker hash and that its behaviour has + // changed between game versions - so membership would be wrong for ordinary inputs, + // and wrong differently per patch, while Lua keyed on the string itself would be + // right. A set that disagrees with itself across backends is worse than one that says + // no. + throw new CompileError(f.attrTrace().attrErrorPos(), + "A keyed set cannot use string as its element type on Jass: StringHash is lossy " + + "and patch-dependent, so membership would not match the Lua backend. " + + "Restrict the set to Lua, or key on an int derived from the string."); + } + if (t instanceof ImTupleType) { + throw new CompileError(f.attrTrace().attrErrorPos(), + "A keyed set cannot use a tuple as its element type: tuple elimination expands the " + + "argument, so there is no single value to key on."); } // Class instances are integers by this point, and so is int itself. if (TypesHelper.isIntType(t) || t instanceof ImClassType) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java index 4136ac68f..8650d91ab 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java @@ -22,8 +22,6 @@ private static String[] withKeyedTable(String... usage) { " return v", "@compilerintrinsic public function keyOfHandle(handle h) returns int", " return GetHandleId(h)", - "@compilerintrinsic public function keyOfString(string s) returns int", - " return StringHash(s)", "endpackage")); lines.addAll(java.util.Arrays.asList(usage)); return lines.toArray(new String[0]); @@ -73,17 +71,61 @@ public void classInstanceKeyIsStable() { "endpackage")); } + /** + * StringHash is not identity - this repo's MultibyteDiagnostics records that it collapses + * whole classes of strings and has changed between patches - so Jass membership would + * disagree with Lua, which keys on the string itself. + */ @Test - public void stringKeyIsItsHash() { - test().executeProg(true).executeProgOnlyAfterTransforms().withStdLib().lines(withKeyedTable( + public void stringElementIsRejected() { + testAssertErrorsLinesWithStdLib(false, "cannot use string as its element type", withKeyedTable( "package Test", "import KeyedTable", "init", - " wurstKeyOf(\"abc\").assertEquals(StringHash(\"abc\"))", - " testSuccess()", + " let k = wurstKeyOf(\"abc\")", + " if k > 0", + " skip", + "endpackage")); + } + + /** Tuple elimination expands the argument, so there is no single value to key on. */ + @Test + public void tupleElementIsRejected() { + testAssertErrorsLinesWithStdLib(false, "cannot use a tuple as its element type", withKeyedTable( + "package Test", + "import KeyedTable", + "tuple pair(int a, int b)", + "init", + " let k = wurstKeyOf(pair(1, 2))", + " if k > 0", + " skip", "endpackage")); } + /** A same-named helper in another package must not be picked up. */ + @Test + public void helperFromAnotherPackageIsNotUsed() { + testAssertErrorsLinesWithStdLib(false, "must also declare keyOfInt", + "package KeyedTable", + "@compilerintrinsic public function wurstKeyOf(T value) returns int", + " return 0", + "@compilerintrinsic public function keyOfHandle(handle h) returns int", + " return GetHandleId(h)", + "endpackage", + "package Impostor", + "@compilerintrinsic public function keyOfInt(int v) returns int", + " return v + 1", + "endpackage", + "package Test", + "import KeyedTable", + "import Impostor", + "init", + " let k = wurstKeyOf(7)", + " if k > 0", + " skip", + "endpackage"); + } + /** real has no stable integer key, and saying so beats keying on a truncation. */ @Test public void realElementIsRejected() { From dbd43af57ca3815190c48c2b6114843915ddedf8 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 10:39:49 +0200 Subject: [PATCH 3/4] Give the key projection a meaning in the interpreter. JassKeyOfLowering runs inside transformProgToJass, so compiletime evaluation and -runTests executed the intrinsic's placeholder source body and gave every element the same key. ILInterpreter now projects the value the same way the pass compiles the call to: an int and a class instance are their own key, a handle is keyed by its id. Scoped to the still-generic function, so a monomorphised copy runs its lowered body and both paths stay covered - the positive tests drop executeProgOnlyAfterTransforms and now run either way. Also validate a projection helper's parameter type, not just its arity and return type: the emitted call assumes it, so a helper of the wrong shape produced malformed IM and a pjass failure instead of a readable error. --- .../interpreter/ILInterpreter.java | 38 +++++++++++++ .../imtranslation/JassKeyOfLowering.java | 54 +++++++++++++++---- .../wurstscript/tests/KeyedTableTests.java | 31 +++++++++-- 3 files changed, 111 insertions(+), 12 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/interpreter/ILInterpreter.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/interpreter/ILInterpreter.java index 56d0b5f06..6e48cb01a 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/interpreter/ILInterpreter.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/interpreter/ILInterpreter.java @@ -18,6 +18,7 @@ import de.peeeq.wurstscript.parser.WPos; import de.peeeq.wurstscript.translation.imtranslation.FunctionFlagEnum; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; +import de.peeeq.wurstscript.translation.imtranslation.JassKeyOfLowering; import de.peeeq.wurstscript.validation.GlobalCaches; import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import org.eclipse.jdt.annotation.Nullable; @@ -215,6 +216,15 @@ public static LocalState runFunc(ProgramState globalState, ImFunction f, @Nullab return runBuiltinFunction(globalState, f, args); } + // --- key projection intrinsic --- + // Its source body is a placeholder: a `T:` parameter cannot be projected to an integer + // in Wurst, which is why JassKeyOfLowering supplies one after generic elimination. That + // pass runs inside transformProgToJass, so running the body here would give compiletime + // evaluation and -runTests a constant key for every element. + if (JassKeyOfLowering.isUnloweredKeyOf(f)) { + return new LocalState(keyOfValue(globalState, args[0])); + } + // --- local state & bind parameters --- LocalState localState = new LocalState(); for (int i = 0; i < f.getParameters().size(); i++) { @@ -470,6 +480,34 @@ private static boolean isTypeReal(ImType t) { private static final LocalState EMPTY_LOCAL_STATE = new LocalState(); + /** + * The integer key of a runtime value, matching what JassKeyOfLowering compiles the intrinsic + * to: an int and a class instance are their own key, and a handle is keyed by its id. + */ + private static ILconst keyOfValue(ProgramState globalState, ILconst value) { + if (value instanceof ILconstInt) { + return value; + } + if (value instanceof ILconstObject obj) { + return ILconstInt.create(obj.getObjectId()); + } + if (value instanceof ILconstNull) { + // A null class instance is integer 0 on Jass, and so is GetHandleId of a null handle. + return ILconstInt.create(0); + } + if (value instanceof IlConstHandle) { + for (NativesProvider natives : globalState.getNativeProviders()) { + try { + return natives.invoke("GetHandleId", new ILconst[]{value}); + } catch (NoSuchNativeException e) { + // Not this provider's native - the next one may have it. + } + } + } + throw new InterpreterException(globalState, "Cannot compute a keyed-set key for " + + value.print() + ": only int, class instances and handles have a stable integer key."); + } + private static LocalState runBuiltinFunction(ProgramState globalState, ImFunction f, ILconst... args) { // Delegate to the array overload to avoid double-allocations. return runBuiltinFunction(globalState, f, args, /*isVarargs*/ true); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java index fe3984204..37711d7f8 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java @@ -38,7 +38,7 @@ private JassKeyOfLowering() { public static void transform(ImProg prog) { for (ImFunction f : prog.getFunctions()) { - if (!KEY_OF.equals(annotatedName(f)) || f.getParameters().size() != 1) { + if (!isKeyOf(f)) { continue; } ImVar value = f.getParameters().get(0); @@ -53,14 +53,39 @@ public static void transform(ImProg prog) { } } + /** + * Whether {@code f} is the key projection intrinsic still carrying its placeholder body. + * + *

The interpreter asks: it runs on the IM before transformProgToJass, so compiletime + * evaluation and {@code -runTests} would otherwise execute that placeholder and give every + * element the same key. ILInterpreter supplies the meaning this pass compiles the call to. + * + *

Still generic is what makes it unlowered: generic elimination gives each specialisation a + * concrete parameter type, and only then can this pass pick a projection. A monomorphised copy + * therefore runs its real body, so the post-transform interpreter run still exercises it. + */ + public static boolean isUnloweredKeyOf(ImFunction f) { + // The interpreter asks this on every call it makes, so the list checks come before the + // trace and annotation lookups. Having type variables rules out almost everything. + return !f.getTypeVariables().isEmpty() && isKeyOf(f); + } + + private static boolean isKeyOf(ImFunction f) { + return f.getParameters().size() == 1 + && f.attrTrace() instanceof FuncDef fd + && KEY_OF.equals(fd.getName()) + && fd.attrHasAnnotation(CompilerIntrinsics.ANNOTATION); + } + /** * The projection of that name declared beside the intrinsic itself. * *

Scoped to the declaring package rather than matched by name across the program: the name * is not identity, and a same-named annotated function in another package would otherwise win - * or lose by traversal order and silently key every set on something else. The signature is - * checked for the same reason - a helper of the wrong shape produces malformed IM rather than - * an error anyone can read. + * or lose by traversal order and silently key every set on something else. The whole signature + * is checked for the same reason - the call emitted below assumes the parameter type, so a + * helper of the wrong shape produces malformed IM and a pjass failure rather than an error + * anyone can read. */ private static ImFunction findProjection(ImProg prog, WPackage owner, String name, ImFunction f) { if (owner != null) { @@ -69,9 +94,11 @@ private static ImFunction findProjection(ImProg prog, WPackage owner, String nam continue; } if (candidate.getParameters().size() != 1 - || !TypesHelper.isIntType(candidate.getReturnType())) { + || !TypesHelper.isIntType(candidate.getReturnType()) + || !takesExpectedParam(name, candidate.getParameters().get(0).getType())) { throw new CompileError(candidate.attrTrace().attrErrorPos(), - name + " must take exactly one parameter and return int."); + name + " must take exactly one " + expectedParam(name) + + " parameter and return int."); } return candidate; } @@ -95,7 +122,7 @@ private static String projectionFor(ImType t, ImFunction f) { + "no stable integer key. Use int, a handle or a class, or restrict the set " + "to Lua."); } - if (isCodeType(t)) { + if (isSimpleType(t, "code")) { throw new CompileError(f.attrTrace().attrErrorPos(), "A keyed set cannot use code as its element type: function references have no " + "stable identity to key on."); @@ -125,8 +152,17 @@ private static String projectionFor(ImType t, ImFunction f) { return KEY_OF_HANDLE; } - private static boolean isCodeType(ImType t) { - return t instanceof ImSimpleType st && "code".equals(st.getTypename()); + /** The parameter type a projection must take, since the emitted call assumes it. */ + private static boolean takesExpectedParam(String name, ImType t) { + return KEY_OF_INT.equals(name) ? TypesHelper.isIntType(t) : isSimpleType(t, "handle"); + } + + private static String expectedParam(String name) { + return KEY_OF_INT.equals(name) ? "int" : "handle"; + } + + private static boolean isSimpleType(ImType t, String name) { + return t instanceof ImSimpleType st && name.equals(st.getTypename()); } private static String typeNameOf(ImType t) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java index 8650d91ab..b8def71cc 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java @@ -9,6 +9,10 @@ * so a keyed structure needs an integer, and a {@code T:} parameter cannot be projected to one in * source. JassKeyOfLowering fills that in after generic elimination, when each specialisation's * element type is concrete. + * + *

The positive tests deliberately run the interpreter both before and after that pass: before + * it the intrinsic is still generic and ILInterpreter supplies the projection, after it the + * lowered body runs. Both must agree, or compiletime state would disagree with the final Jass. */ public class KeyedTableTests extends WurstScriptTest { @@ -30,7 +34,7 @@ private static String[] withKeyedTable(String... usage) { /** An int is its own key. */ @Test public void intKeyIsIdentity() { - test().executeProg(true).executeProgOnlyAfterTransforms().withStdLib().lines(withKeyedTable( + test().executeProg(true).withStdLib().lines(withKeyedTable( "package Test", "import KeyedTable", "init", @@ -43,7 +47,7 @@ public void intKeyIsIdentity() { /** A handle is keyed by its id, which is what makes unit membership work on Jass. */ @Test public void handleKeyIsItsHandleId() { - test().executeProg(true).executeProgOnlyAfterTransforms().withStdLib().lines(withKeyedTable( + test().executeProg(true).withStdLib().lines(withKeyedTable( "package Test", "import KeyedTable", "init", @@ -58,7 +62,7 @@ public void handleKeyIsItsHandleId() { /** A class instance is already an integer by the time the projection is chosen. */ @Test public void classInstanceKeyIsStable() { - test().executeProg(true).executeProgOnlyAfterTransforms().withStdLib().lines(withKeyedTable( + test().executeProg(true).withStdLib().lines(withKeyedTable( "package Test", "import KeyedTable", "class Marker", @@ -126,6 +130,27 @@ public void helperFromAnotherPackageIsNotUsed() { "endpackage"); } + /** The emitted call assumes the projection's parameter type, so a wrong one is a contract error. */ + @Test + public void projectionWithWrongParameterTypeIsRejected() { + testAssertErrorsLinesWithStdLib(false, "keyOfInt must take exactly one int parameter", + "package KeyedTable", + "@compilerintrinsic public function wurstKeyOf(T value) returns int", + " return 0", + "@compilerintrinsic public function keyOfInt(string v) returns int", + " return 1", + "@compilerintrinsic public function keyOfHandle(handle h) returns int", + " return GetHandleId(h)", + "endpackage", + "package Test", + "import KeyedTable", + "init", + " let k = wurstKeyOf(7)", + " if k > 0", + " skip", + "endpackage"); + } + /** real has no stable integer key, and saying so beats keying on a truncation. */ @Test public void realElementIsRejected() { From fc6684293769755057df91c72325826e8266beba Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 11 Sep 2026 11:03:30 +0200 Subject: [PATCH 4/4] Keep a null element's key distinct from every live handle. The interpreter handed out handle ids from 0, so a null element and the first handle created both keyed to 0 and a keyed set reported the first unit as present when asked about null. The game reserves 0 for a null handle, so ids now start at 1 and GetHandleId answers 0 for null instead of failing the reflective invoke - natives dispatch on name and arity, so a null argument used to reach the handle overload and throw. Also check the intrinsic's own signature before rewriting it: the new body is an integer projection, so a declaration of another shape would leave the return type disagreeing with the body it was given. --- .../providers/HandleProvider.java | 21 ++++++++-- .../imtranslation/JassKeyOfLowering.java | 17 ++++++-- .../wurstscript/tests/KeyedTableTests.java | 39 +++++++++++++++++++ 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/jassinterpreter/providers/HandleProvider.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/jassinterpreter/providers/HandleProvider.java index da51dbe79..466a9dd18 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/jassinterpreter/providers/HandleProvider.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/jassinterpreter/providers/HandleProvider.java @@ -1,21 +1,36 @@ package de.peeeq.wurstio.jassinterpreter.providers; +import de.peeeq.wurstio.jassinterpreter.InterpreterException; +import de.peeeq.wurstscript.intermediatelang.ILconst; import de.peeeq.wurstscript.intermediatelang.ILconstInt; +import de.peeeq.wurstscript.intermediatelang.ILconstNull; import de.peeeq.wurstscript.intermediatelang.IlConstHandle; import de.peeeq.wurstscript.intermediatelang.interpreter.AbstractInterpreter; import java.util.LinkedHashMap; public class HandleProvider extends Provider { - private int handleCounter = 0; + /** Ids start at 1: the game reserves 0 for a null handle, so nothing may be given it. */ + private int handleCounter = 1; private final LinkedHashMap handleMap = new LinkedHashMap<>(); public HandleProvider(AbstractInterpreter interpreter) { super(interpreter); } - public ILconstInt GetHandleId(IlConstHandle handle) { - return handleMap.computeIfAbsent(handle,(_key) -> ILconstInt.create(handleCounter++)); + /** + * Takes ILconst rather than IlConstHandle so a null handle reaches us: natives are dispatched + * by name and arity, so a null argument used to fail the reflective invoke with an Error. The + * game answers 0 for one, which is why no real handle is given that id. + */ + public ILconstInt GetHandleId(ILconst handle) { + if (handle instanceof ILconstNull) { + return ILconstInt.create(0); + } + if (!(handle instanceof IlConstHandle h)) { + throw new InterpreterException("GetHandleId expects a handle, got " + handle.print() + "."); + } + return handleMap.computeIfAbsent(h, (_key) -> ILconstInt.create(handleCounter++)); } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java index 37711d7f8..9aefd1083 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/JassKeyOfLowering.java @@ -38,9 +38,15 @@ private JassKeyOfLowering() { public static void transform(ImProg prog) { for (ImFunction f : prog.getFunctions()) { - if (!isKeyOf(f)) { + if (!isNamedKeyOf(f)) { continue; } + if (!isKeyOf(f)) { + // The rewrite below returns an integer projection, so a declaration of another + // shape would leave the return type disagreeing with the body it now has. + throw new CompileError(f.attrTrace().attrErrorPos(), + KEY_OF + " must take exactly one parameter and return int."); + } ImVar value = f.getParameters().get(0); ImFunction projection = findProjection(prog, packageOf(f), projectionFor(value.getType(), f), f); @@ -71,8 +77,13 @@ public static boolean isUnloweredKeyOf(ImFunction f) { } private static boolean isKeyOf(ImFunction f) { - return f.getParameters().size() == 1 - && f.attrTrace() instanceof FuncDef fd + return isNamedKeyOf(f) + && f.getParameters().size() == 1 + && TypesHelper.isIntType(f.getReturnType()); + } + + private static boolean isNamedKeyOf(ImFunction f) { + return f.attrTrace() instanceof FuncDef fd && KEY_OF.equals(fd.getName()) && fd.attrHasAnnotation(CompilerIntrinsics.ANNOTATION); } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java index b8def71cc..e9f9079bf 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/KeyedTableTests.java @@ -75,6 +75,45 @@ public void classInstanceKeyIsStable() { "endpackage")); } + /** + * A null element keys to 0, the id the game gives a null handle and the value of a null class + * instance. No live handle may share it, which is why interpreter ids start at 1. + */ + @Test + public void nullElementKeysToZeroAndDoesNotCollide() { + test().executeProg(true).withStdLib().lines(withKeyedTable( + "package Test", + "import KeyedTable", + "init", + " unit noUnit = null", + " let u = CreateUnit(Player(0), 'hfoo', 0., 0., 0.)", + " wurstKeyOf(noUnit).assertEquals(0)", + " (wurstKeyOf(u) != wurstKeyOf(noUnit)).assertTrue()", + " testSuccess()", + "endpackage")); + } + + /** The rewrite returns an integer projection, so the intrinsic itself must return int. */ + @Test + public void intrinsicWithWrongReturnTypeIsRejected() { + testAssertErrorsLinesWithStdLib(false, "wurstKeyOf must take exactly one parameter and return int", + "package KeyedTable", + "@compilerintrinsic public function wurstKeyOf(T value) returns real", + " return 0.", + "@compilerintrinsic public function keyOfInt(int v) returns int", + " return v", + "@compilerintrinsic public function keyOfHandle(handle h) returns int", + " return GetHandleId(h)", + "endpackage", + "package Test", + "import KeyedTable", + "init", + " let k = wurstKeyOf(7)", + " if k > 0.", + " skip", + "endpackage"); + } + /** * StringHash is not identity - this repo's MultibyteDiagnostics records that it collapses * whole classes of strings and has changed between patches - so Jass membership would