From 29ffce9d8e214c0833e9ed64447ddb873d9e1db6 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 14:19:27 +0200 Subject: [PATCH 1/6] Decide slot ambiguity by the dispatch family, not by a declared name A slot name shared by several methods is only a collision when those methods are not one dispatchable thing. assignDispatchGroupKeys already computes that: it unions a method with its submethods, which is a method together with its overrides. The key it stores is taken after the union is split by signature, and that split separates the members of a generic override chain, since their signatures differ by each class's own type variable - which is why using the group key broke the chain tests in #1247 and why the declared name was used instead. The family key is the union root, recorded before the split. Overrides share it, so their shared slot survives; overloads and unrelated siblings do not, which the declared name could not express. Note on what this does not demonstrate. #1247 recorded a residual - overloads in a specialised class keeping a dead key because they share a declared name - and a test written for it passes under the declared name too: two overloads compose different names rather than colliding. So this is a cleanup which removes a documented gap in principle, not a fix with a failing case behind it. The item is corrected rather than closed with a claim I cannot back. --- .../parserspec/jass_im.parseq | 5 ++ .../imtranslation/ClosureTranslator.java | 2 +- .../imtranslation/ImTranslator.java | 4 +- .../imtranslation/LuaDispatchPreparation.java | 24 +++++---- .../imtranslation/OverrideUtils.java | 2 +- .../lua/translation/LuaTranslator.java | 22 ++++----- .../wurstscript/tests/FastHashMapTests.java | 49 +++++++++++++++++++ 7 files changed, 82 insertions(+), 26 deletions(-) diff --git a/de.peeeq.wurstscript/parserspec/jass_im.parseq b/de.peeeq.wurstscript/parserspec/jass_im.parseq index c8d4eadae..7d8f9497b 100644 --- a/de.peeeq.wurstscript/parserspec/jass_im.parseq +++ b/de.peeeq.wurstscript/parserspec/jass_im.parseq @@ -70,6 +70,11 @@ ImMethod(@ignoreForEquality de.peeeq.wurstscript.ast.Element trace, java.util.List subMethods, java.util.List luaMethodDispatchAliases, String luaDispatchGroupKey, + // A method and its overrides, before the group is split by signature. Overrides of one method + // share this; overloads and unrelated methods do not. The group key cannot answer that, because + // it embeds the signature and a generic override chain's signatures differ by each class's own + // type variable. + String luaDispatchFamilyKey, boolean isAbstract) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ClosureTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ClosureTranslator.java index d540afdc7..39d81f154 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ClosureTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ClosureTranslator.java @@ -195,7 +195,7 @@ private ImClass createClass() { tr.getImProg().getFunctions().remove(impl); c.getFunctions().add(impl); ImClassType methodClass = JassIm.ImClassType(c, JassIm.ImTypeArguments()); - ImMethod m = JassIm.ImMethod(e, methodClass, superMethod.getName(), impl, JassIm.ImMethods(), Lists.newArrayList(), "", false); + ImMethod m = JassIm.ImMethod(e, methodClass, superMethod.getName(), impl, JassIm.ImMethods(), Lists.newArrayList(), "", "", false); c.getMethods().add(m); OverrideUtils.addOverrideClosure(tr, superMethod, m, e); 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 455aedeb5..aa5dba940 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 @@ -889,7 +889,7 @@ public ImFunction initFor(StructureDef classDef) { public ImMethod initFor(StructureDef classDef) { ImFunction impl = destroyFunc.getFor(classDef); ImMethod m = JassIm.ImMethod(classDef, selfType(classDef), "destroy" + classDef.getName(), - impl, Lists.newArrayList(), Lists.newArrayList(), "", false); + impl, Lists.newArrayList(), Lists.newArrayList(), "", "", false); return m; } }; @@ -2278,7 +2278,7 @@ public ImMethod getMethodFor(FuncDef f) { // otherwise EliminateClasses dispatch lookup can fail. String methodName = imFunc.getName(); WLogger.trace(() -> "[GENCAP] getMethodFor " + elementNameWithPath(f) + " -> methodName=" + methodName); - m = JassIm.ImMethod(f, selfType(f), methodName, imFunc, Lists.newArrayList(), Lists.newArrayList(), "", false); + m = JassIm.ImMethod(f, selfType(f), methodName, imFunc, Lists.newArrayList(), Lists.newArrayList(), "", "", false); methodForFuncDef.put(f, m); } return m; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java index 4891947a2..af46568b7 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java @@ -68,6 +68,16 @@ private static void assignDispatchGroupKeys(List allMethods) { grouped.computeIfAbsent(root, ignored -> new ArrayList<>()).add(method); } + // The union above is a method together with its overrides, which is one dispatchable thing. + // Recorded before the group is split by signature, because that split separates the members of + // a generic override chain - their signatures differ by each class's own type variable. + for (Map.Entry> family : grouped.entrySet()) { + String familyKey = methodSortKey(family.getKey()); + for (ImMethod method : family.getValue()) { + method.setLuaDispatchFamilyKey(familyKey); + } + } + for (List group : grouped.values()) { Map> partitions = new LinkedHashMap<>(); group.sort(Comparator.comparing(LuaDispatchPreparation::methodSortKey)); @@ -169,15 +179,11 @@ private static Set ambiguousDirectAliases(List allMethods) { continue; } // A method and its overrides are one dispatchable thing and must share a slot - that is - // what dispatch is - so they are not a collision, and they all declare the same name in - // the source. The siblings of one specialisation declare different ones and merely end up - // composing the same segment, because for them that segment is the type argument. - // - // The dispatch group key would separate overloads too, but it embeds the signature, and a - // generic override chain's signatures differ by each class's type variable - so overrides - // would read as unrelated and lose the slot they must share. Backlog item 15 records what - // that leaves: overloads inside a specialised class keep one dead key. - String identity = declaredName(method); + // what dispatch is - so they are not a collision. The family key says exactly that: the + // union of a method with its overrides, before the split by signature which would have + // separated the members of a generic chain. Unlike the declared name it also separates + // overloads, which are not one dispatchable thing however they are spelled. + String identity = method.getLuaDispatchFamilyKey(); String previous = claimedBy.put(composed, identity); if (previous != null && !previous.equals(identity)) { ambiguous.add(composed); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java index 07aa1a4f4..43e4659bc 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java @@ -119,7 +119,7 @@ public static void addOverride( ImFunction implementation = JassIm.ImFunction(e, subMethod.getName() + "_wrapper", JassIm.ImTypeVars(), parameters, rType, locals, body, flags); tr.getImProg().getFunctions().add(implementation); - ImMethod wrapperMethod = JassIm.ImMethod(e, subMethod.getMethodClass(), subMethod.getName() + "_wrapper", implementation, JassIm.ImMethods(), new ArrayList<>(), "", false); + ImMethod wrapperMethod = JassIm.ImMethod(e, subMethod.getMethodClass(), subMethod.getName() + "_wrapper", implementation, JassIm.ImMethods(), new ArrayList<>(), "", "", false); subClass.getMethods().add(wrapperMethod); superMethodIm.getSubMethods().add(wrapperMethod); } 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 05915f337..1f3167bba 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 @@ -1053,18 +1053,13 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List - * A method and its overrides share a semantic name and must share a slot: that is dispatch, and - * they all declare the same name in the source. The siblings of one specialisation declare - * different names and still compose the same segment, because for a specialised method that - * segment is the type argument - and the slot composed from it is claimed by whichever is bound - * first, then never called. - *

- * The dispatch group key would be a sharper identity but cannot be used: it embeds the signature, - * and a generic override chain's signatures differ by the type variable of each class in it - * ({@code void|T192,real} against {@code void|T636,real}), so overrides would read as unrelated - * and their shared slot would be dropped. What that leaves uncovered is recorded in backlog - * item 15: overloads of one source method inside a specialised class share a declared name, so - * their composed name is not seen as ambiguous and one dead key survives there. + * A method and its overrides share a semantic name and must share a slot: that is dispatch. The + * family key says which methods are that one thing - the union of a method with its overrides, + * taken before the split by signature which would separate the members of a generic chain, since + * their signatures differ by each class's own type variable. The siblings of one specialisation + * belong to different families and still compose the same segment, because for a specialised + * method that segment is the type argument, and the slot composed from it is claimed by whichever + * is bound first and then never called. *

* Cached because {@code createMethods} asks twice per dispatch group and each ask would otherwise * rebuild and sort the whole inherited method list. @@ -1082,8 +1077,9 @@ private Set ambiguousSemanticNames(ImClass c) { if (semanticName.isEmpty()) { continue; } + String family = m.getLuaDispatchFamilyKey(); claimants.computeIfAbsent(semanticName, name -> new TreeSet<>()) - .add(LuaDispatchPreparation.declaredName(m)); + .add(family == null ? "" : family); } Set ambiguous = new TreeSet<>(); claimants.forEach((name, keys) -> { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java index 074d47b85..a61e603de 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java @@ -516,6 +516,55 @@ public void everySlotOnASpecialisedClassNamesAMethod() throws IOException { } } + /** + * Overloads of one method inside a specialised class, which nothing else covers. + *

+ * It passes under the previous identity as well, so it does not pin the dispatch family: the two + * overloads compose different names rather than colliding, which means the residual backlog item + * 15 described is narrower than it claimed, or not reachable this way. Kept for the shape. + */ + @Test + public void overloadsInASpecialisedClassLeaveNoDeadSlot() throws IOException { + String[] withOverload = program(fastHashMap( + " function get(K key, V fallback) returns V", + " let s = slotFor(key)", + " if s < base or not used[s]", + " return fallback", + " return values[s]" + ), INT_INSTANCE, USE_WITH_COLLISION); + + test().testLua(true).executeProg().lines(withOverload); + assertSpecialisedClassesAllocateTheirFields( + compiledLua("overloadsInASpecialisedClassLeaveNoDeadSlot")); + + assertEverySlotNamesAMethod(compiledLua("overloadsInASpecialisedClassLeaveNoDeadSlot")); + } + + /** Every slot assigned on a specialised class table carries one of the container's method names. */ + private static void assertEverySlotNamesAMethod(String lua) { + Matcher table = Pattern.compile("(FastHashMap_specialized\\w*)\\.(\\w+)\\s*=").matcher(lua); + java.util.List unnamed = new java.util.ArrayList<>(); + while (table.find()) { + String slot = table.group(2); + if (slot.startsWith("__")) { + continue; + } + boolean namesAMethod = false; + for (String method : METHOD_NAMES) { + if (slot.contains(method)) { + namesAMethod = true; + break; + } + } + if (!namesAMethod) { + unnamed.add(slot); + } + } + if (!unnamed.isEmpty()) { + throw new AssertionError("these slots name no method: " + unnamed + "\n" + lua); + } + } + private String compiledJass(String testName) throws IOException { return Files.toString(new File(TEST_OUTPUT_PATH, "FastHashMapTests_" + testName + ".j"), Charsets.UTF_8); } From cb3c9da68d1fba51cb427f6aab9807b96761d844 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 15:04:47 +0200 Subject: [PATCH 2/6] Decide a junk slot by whether the name names a method The dispatch family key is withdrawn. A conversion wrapper is linked to the method it overrides but not to the override it calls, so it lands in a different family from that override, and their shared slot was suppressed as a collision - which is what the review found. Linking the wrapper to the override, as suggested, is not safe: the wrapper's dispatch then reaches the override directly, skipping the conversion the wrapper exists to perform, and the implicit conversion and generic overload tests fail. That is noted where the wrapper is built, so the next reader does not try it. The test which replaces it needs no families. A composed name is junk when it names no method: for a specialised method the trailing segment is the type argument, which nobody declared, so every method of the specialisation composes it and the slot is claimed arbitrarily. A method and its overrides compose the name they were declared with and must share that slot - a conversion wrapper included, since it carries the same declared name. That also covers the overload case item 15 recorded, which the declared name alone could not: two overloads in a specialised class both compose the type argument, which is neither of their declared names. --- de.peeeq.wurstscript/parserspec/jass_im.parseq | 5 ----- .../imtranslation/ClosureTranslator.java | 2 +- .../imtranslation/ImTranslator.java | 4 ++-- .../imtranslation/LuaDispatchPreparation.java | 18 +++++++----------- .../imtranslation/OverrideUtils.java | 7 ++++++- .../lua/translation/LuaTranslator.java | 9 +++++++-- 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/de.peeeq.wurstscript/parserspec/jass_im.parseq b/de.peeeq.wurstscript/parserspec/jass_im.parseq index 7d8f9497b..c8d4eadae 100644 --- a/de.peeeq.wurstscript/parserspec/jass_im.parseq +++ b/de.peeeq.wurstscript/parserspec/jass_im.parseq @@ -70,11 +70,6 @@ ImMethod(@ignoreForEquality de.peeeq.wurstscript.ast.Element trace, java.util.List subMethods, java.util.List luaMethodDispatchAliases, String luaDispatchGroupKey, - // A method and its overrides, before the group is split by signature. Overrides of one method - // share this; overloads and unrelated methods do not. The group key cannot answer that, because - // it embeds the signature and a generic override chain's signatures differ by each class's own - // type variable. - String luaDispatchFamilyKey, boolean isAbstract) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ClosureTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ClosureTranslator.java index 39d81f154..d540afdc7 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ClosureTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ClosureTranslator.java @@ -195,7 +195,7 @@ private ImClass createClass() { tr.getImProg().getFunctions().remove(impl); c.getFunctions().add(impl); ImClassType methodClass = JassIm.ImClassType(c, JassIm.ImTypeArguments()); - ImMethod m = JassIm.ImMethod(e, methodClass, superMethod.getName(), impl, JassIm.ImMethods(), Lists.newArrayList(), "", "", false); + ImMethod m = JassIm.ImMethod(e, methodClass, superMethod.getName(), impl, JassIm.ImMethods(), Lists.newArrayList(), "", false); c.getMethods().add(m); OverrideUtils.addOverrideClosure(tr, superMethod, m, e); 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 aa5dba940..455aedeb5 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 @@ -889,7 +889,7 @@ public ImFunction initFor(StructureDef classDef) { public ImMethod initFor(StructureDef classDef) { ImFunction impl = destroyFunc.getFor(classDef); ImMethod m = JassIm.ImMethod(classDef, selfType(classDef), "destroy" + classDef.getName(), - impl, Lists.newArrayList(), Lists.newArrayList(), "", "", false); + impl, Lists.newArrayList(), Lists.newArrayList(), "", false); return m; } }; @@ -2278,7 +2278,7 @@ public ImMethod getMethodFor(FuncDef f) { // otherwise EliminateClasses dispatch lookup can fail. String methodName = imFunc.getName(); WLogger.trace(() -> "[GENCAP] getMethodFor " + elementNameWithPath(f) + " -> methodName=" + methodName); - m = JassIm.ImMethod(f, selfType(f), methodName, imFunc, Lists.newArrayList(), Lists.newArrayList(), "", "", false); + m = JassIm.ImMethod(f, selfType(f), methodName, imFunc, Lists.newArrayList(), Lists.newArrayList(), "", false); methodForFuncDef.put(f, m); } return m; diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java index af46568b7..d9946cf5e 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java @@ -68,16 +68,6 @@ private static void assignDispatchGroupKeys(List allMethods) { grouped.computeIfAbsent(root, ignored -> new ArrayList<>()).add(method); } - // The union above is a method together with its overrides, which is one dispatchable thing. - // Recorded before the group is split by signature, because that split separates the members of - // a generic override chain - their signatures differ by each class's own type variable. - for (Map.Entry> family : grouped.entrySet()) { - String familyKey = methodSortKey(family.getKey()); - for (ImMethod method : family.getValue()) { - method.setLuaDispatchFamilyKey(familyKey); - } - } - for (List group : grouped.values()) { Map> partitions = new LinkedHashMap<>(); group.sort(Comparator.comparing(LuaDispatchPreparation::methodSortKey)); @@ -183,7 +173,13 @@ private static Set ambiguousDirectAliases(List allMethods) { // union of a method with its overrides, before the split by signature which would have // separated the members of a generic chain. Unlike the declared name it also separates // overloads, which are not one dispatchable thing however they are spelled. - String identity = method.getLuaDispatchFamilyKey(); + // Junk when the name names no method: the segment it was composed from is the type + // argument rather than anything anyone wrote. A method and its overrides compose the name + // they were declared with and share that slot, which is dispatch - including a conversion + // wrapper, which carries the same declared name as the method it wraps. + String identity = semanticNameFromMethodName(method.getName()).equals(declaredName(method)) + ? "declared:" + declaredName(method) + : "mangled:" + System.identityHashCode(method); String previous = claimedBy.put(composed, identity); if (previous != null && !previous.equals(identity)) { ambiguous.add(composed); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java index 43e4659bc..4f43e38e0 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java @@ -119,9 +119,14 @@ public static void addOverride( ImFunction implementation = JassIm.ImFunction(e, subMethod.getName() + "_wrapper", JassIm.ImTypeVars(), parameters, rType, locals, body, flags); tr.getImProg().getFunctions().add(implementation); - ImMethod wrapperMethod = JassIm.ImMethod(e, subMethod.getMethodClass(), subMethod.getName() + "_wrapper", implementation, JassIm.ImMethods(), new ArrayList<>(), "", "", false); + ImMethod wrapperMethod = JassIm.ImMethod(e, subMethod.getMethodClass(), subMethod.getName() + "_wrapper", implementation, JassIm.ImMethods(), new ArrayList<>(), "", false); subClass.getMethods().add(wrapperMethod); superMethodIm.getSubMethods().add(wrapperMethod); + // Deliberately not linking wrapperMethod to subMethod as a submethod, though the wrapper does + // call it. Doing so makes the wrapper's dispatch reach the override directly, without the + // conversion the wrapper exists to perform, and fails the implicit conversion and generic + // overload tests. The relation is real but this is not the way to state it; the family below + // is derived without needing it. } 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 1f3167bba..a3d245f68 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 @@ -1077,9 +1077,14 @@ private Set ambiguousSemanticNames(ImClass c) { if (semanticName.isEmpty()) { continue; } - String family = m.getLuaDispatchFamilyKey(); + // A name which is the method's own declared name belongs to it and to its overrides, + // a conversion wrapper included, since the wrapper carries that name too. One built + // from a mangled segment belongs to none of them, and counting each such method + // separately is what makes the name read as claimed by several and left uncomposed. + String declared = LuaDispatchPreparation.declaredName(m); claimants.computeIfAbsent(semanticName, name -> new TreeSet<>()) - .add(family == null ? "" : family); + .add(semanticName.equals(declared) ? "declared:" + declared + : "mangled:" + System.identityHashCode(m)); } Set ambiguous = new TreeSet<>(); claimants.forEach((name, keys) -> { From 01781f4452b85841d7c7830568721b93710405e2 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 15:15:52 +0200 Subject: [PATCH 3/6] Count claimants by identity, not by a stringified identity hash Two distinct methods can share a 32-bit identity hash, and the hash depends on where the JVM allocated them. Using it as a claimant key made two methods read as one, so a slot could appear or disappear for the same input between runs - which is the kind of nondeterminism another test in this repository exists to catch, introduced by the change meant to make naming structural. Counted by object identity now, through a set backed by an IdentityHashMap, with the decision stated outright rather than encoded in keys: a name is junk when nobody declared it and more than one method composes it. --- .../imtranslation/LuaDispatchPreparation.java | 40 +++++++++++-------- .../lua/translation/LuaTranslator.java | 22 +++++----- 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java index d9946cf5e..624af9d66 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java @@ -20,6 +20,8 @@ import java.util.HashSet; import org.eclipse.jdt.annotation.Nullable; +import java.util.Collections; +import java.util.IdentityHashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -161,30 +163,34 @@ private static String uniqueName(String name, Set usedNames) { * matching slot for the same reason. */ private static Set ambiguousDirectAliases(List allMethods) { - Map claimedBy = new LinkedHashMap<>(); - Set ambiguous = new HashSet<>(); + // A composed name is junk when it names no method: for a specialised method the segment it was + // built from is the type argument, which nobody declared, so every method of the specialisation + // composes it and the slot would be claimed by whichever is bound first. A method and its + // overrides compose the name they were declared with and must share that slot, which is + // dispatch - a conversion wrapper included, since it carries the same declared name. + Map declaredByAnyone = new LinkedHashMap<>(); + Map> composedBy = new LinkedHashMap<>(); for (ImMethod method : allMethods) { String composed = directAliasFor(method); if (composed == null) { continue; } - // A method and its overrides are one dispatchable thing and must share a slot - that is - // what dispatch is - so they are not a collision. The family key says exactly that: the - // union of a method with its overrides, before the split by signature which would have - // separated the members of a generic chain. Unlike the declared name it also separates - // overloads, which are not one dispatchable thing however they are spelled. - // Junk when the name names no method: the segment it was composed from is the type - // argument rather than anything anyone wrote. A method and its overrides compose the name - // they were declared with and share that slot, which is dispatch - including a conversion - // wrapper, which carries the same declared name as the method it wraps. - String identity = semanticNameFromMethodName(method.getName()).equals(declaredName(method)) - ? "declared:" + declaredName(method) - : "mangled:" + System.identityHashCode(method); - String previous = claimedBy.put(composed, identity); - if (previous != null && !previous.equals(identity)) { + boolean namesThisMethod = semanticNameFromMethodName(method.getName()) + .equals(declaredName(method)); + declaredByAnyone.merge(composed, namesThisMethod, (a, b) -> a || b); + // Counted by object identity: two methods are two claimants, and nothing about how many + // there are may depend on where the JVM happened to allocate them. + composedBy.computeIfAbsent(composed, + name -> Collections.newSetFromMap(new IdentityHashMap<>())) + .add(method); + } + + Set ambiguous = new HashSet<>(); + composedBy.forEach((composed, methods) -> { + if (!declaredByAnyone.getOrDefault(composed, false) && methods.size() > 1) { ambiguous.add(composed); } - } + }); return ambiguous; } 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 a3d245f68..76da71b25 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 @@ -1068,7 +1068,8 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List ambiguousSemanticNames(ImClass c) { return ambiguousSemanticNamesByClass.computeIfAbsent(c, owner -> { - Map> claimants = new TreeMap<>(); + Map declaredByAnyone = new TreeMap<>(); + Map> composedBy = new TreeMap<>(); for (ImMethod m : collectMethodsInHierarchy(owner)) { if (m == null) { continue; @@ -1077,18 +1078,17 @@ private Set ambiguousSemanticNames(ImClass c) { if (semanticName.isEmpty()) { continue; } - // A name which is the method's own declared name belongs to it and to its overrides, - // a conversion wrapper included, since the wrapper carries that name too. One built - // from a mangled segment belongs to none of them, and counting each such method - // separately is what makes the name read as claimed by several and left uncomposed. - String declared = LuaDispatchPreparation.declaredName(m); - claimants.computeIfAbsent(semanticName, name -> new TreeSet<>()) - .add(semanticName.equals(declared) ? "declared:" + declared - : "mangled:" + System.identityHashCode(m)); + boolean namesThisMethod = semanticName.equals(LuaDispatchPreparation.declaredName(m)); + declaredByAnyone.merge(semanticName, namesThisMethod, (a, b) -> a || b); + // By object identity, so the count cannot depend on where the JVM allocated them. + composedBy.computeIfAbsent(semanticName, + name -> Collections.newSetFromMap(new IdentityHashMap<>())) + .add(m); } + Set ambiguous = new TreeSet<>(); - claimants.forEach((name, keys) -> { - if (keys.size() > 1) { + composedBy.forEach((name, methods) -> { + if (!declaredByAnyone.getOrDefault(name, false) && methods.size() > 1) { ambiguous.add(name); } }); From f4c03ed7ead731de27a5daef67e354c4d8088571 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 15:26:00 +0200 Subject: [PATCH 4/6] Let a method compose only the name it was declared with Deciding per name rather than per method gave one declaration ownership of a whole bucket: with a method declared "integer" in a class specialised over int, every unrelated method of that specialisation composes the same segment and was allowed to claim the slot that method owns - so a call through it could reach another method entirely. The test is per method now. A method composes the class-qualified name only when the segment it would be built from is its own declared name; a specialised method's segment is the type argument, which names no method, so it composes nothing. Overrides and a conversion wrapper carry the declared name and keep the slot they share. That removes the claimant counting rather than correcting it - no buckets, no identity sets, no counts. The decision is local to one method, which is why it cannot be swayed by what another method happens to be called. --- .../imtranslation/LuaDispatchPreparation.java | 46 +++------------ .../lua/translation/LuaTranslator.java | 56 ++----------------- 2 files changed, 13 insertions(+), 89 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java index 624af9d66..6f9ee47e4 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java @@ -118,11 +118,9 @@ private static void assignDispatchAliases(ImProg prog, List allMethods Map> closureFamilyAnchorsCache = new HashMap<>(); Map> closureFamilyClassesByAnchor = new HashMap<>(); - Set ambiguousDirectAliases = ambiguousDirectAliases(allMethods); - for (ImMethod method : allMethods) { TreeSet aliases = new TreeSet<>(); - addDirectAliases(method, aliases, ambiguousDirectAliases); + addDirectAliases(method, aliases); addHierarchyAliases(method, aliases, sortedMethodsByClass); addClosureFamilyAliases(prog, method, aliases, sortedMethodsByClass, closureFamilyAnchorsCache, closureFamilyClassesByAnchor); method.setLuaMethodDispatchAliases(new ArrayList<>(aliases)); @@ -162,38 +160,6 @@ private static String uniqueName(String name, Set usedNames) { * arbitrarily" is worse than a name meaning nothing. {@code LuaTranslator} skips composing the * matching slot for the same reason. */ - private static Set ambiguousDirectAliases(List allMethods) { - // A composed name is junk when it names no method: for a specialised method the segment it was - // built from is the type argument, which nobody declared, so every method of the specialisation - // composes it and the slot would be claimed by whichever is bound first. A method and its - // overrides compose the name they were declared with and must share that slot, which is - // dispatch - a conversion wrapper included, since it carries the same declared name. - Map declaredByAnyone = new LinkedHashMap<>(); - Map> composedBy = new LinkedHashMap<>(); - for (ImMethod method : allMethods) { - String composed = directAliasFor(method); - if (composed == null) { - continue; - } - boolean namesThisMethod = semanticNameFromMethodName(method.getName()) - .equals(declaredName(method)); - declaredByAnyone.merge(composed, namesThisMethod, (a, b) -> a || b); - // Counted by object identity: two methods are two claimants, and nothing about how many - // there are may depend on where the JVM happened to allocate them. - composedBy.computeIfAbsent(composed, - name -> Collections.newSetFromMap(new IdentityHashMap<>())) - .add(method); - } - - Set ambiguous = new HashSet<>(); - composedBy.forEach((composed, methods) -> { - if (!declaredByAnyone.getOrDefault(composed, false) && methods.size() > 1) { - ambiguous.add(composed); - } - }); - return ambiguous; - } - private static @Nullable String directAliasFor(ImMethod method) { if (method == null) { return null; @@ -206,8 +172,7 @@ private static Set ambiguousDirectAliases(List allMethods) { return owner.getName() + "_" + semanticName; } - private static void addDirectAliases(ImMethod method, Set aliases, - Set ambiguousDirectAliases) { + private static void addDirectAliases(ImMethod method, Set aliases) { if (method == null) { return; } @@ -216,8 +181,13 @@ private static void addDirectAliases(ImMethod method, Set aliases, aliases.add(methodName); } ImClass owner = method.attrClass(); + // A method composes the class-qualified name only when the segment it would be built from is + // this method's own declared name. For a specialised method that segment is the type argument + // instead, which names no method - and one method happening to be declared with the same word + // as the type argument does not entitle the others to the slot it owns. String composed = directAliasFor(method); - if (composed != null && !ambiguousDirectAliases.contains(composed)) { + if (composed != null + && semanticNameFromMethodName(method.getName()).equals(declaredName(method))) { aliases.add(composed); } String sourceSemanticName = sourceSemanticName(method); 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 76da71b25..4041b691e 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 @@ -1020,8 +1020,12 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List collectDispatchSlotNames(ImClass receiverClass, List ambiguous = ambiguousSemanticNames(receiverClass); Set classNames = new TreeSet<>(); collectClassNamesInHierarchy(receiverClass, classNames, new HashSet<>()); for (String className : classNames) { for (String semanticName : semanticNames) { - if (ambiguous.contains(semanticName)) { - continue; - } slotNames.add(dispatchSlotName(className + "_" + semanticName)); } } @@ -1050,52 +1050,6 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List - * A method and its overrides share a semantic name and must share a slot: that is dispatch. The - * family key says which methods are that one thing - the union of a method with its overrides, - * taken before the split by signature which would separate the members of a generic chain, since - * their signatures differ by each class's own type variable. The siblings of one specialisation - * belong to different families and still compose the same segment, because for a specialised - * method that segment is the type argument, and the slot composed from it is claimed by whichever - * is bound first and then never called. - *

- * Cached because {@code createMethods} asks twice per dispatch group and each ask would otherwise - * rebuild and sort the whole inherited method list. - */ - private final Map> ambiguousSemanticNamesByClass = new LinkedHashMap<>(); - - private Set ambiguousSemanticNames(ImClass c) { - return ambiguousSemanticNamesByClass.computeIfAbsent(c, owner -> { - Map declaredByAnyone = new TreeMap<>(); - Map> composedBy = new TreeMap<>(); - for (ImMethod m : collectMethodsInHierarchy(owner)) { - if (m == null) { - continue; - } - String semanticName = semanticNameFromMethodName(m.getName()); - if (semanticName.isEmpty()) { - continue; - } - boolean namesThisMethod = semanticName.equals(LuaDispatchPreparation.declaredName(m)); - declaredByAnyone.merge(semanticName, namesThisMethod, (a, b) -> a || b); - // By object identity, so the count cannot depend on where the JVM allocated them. - composedBy.computeIfAbsent(semanticName, - name -> Collections.newSetFromMap(new IdentityHashMap<>())) - .add(m); - } - - Set ambiguous = new TreeSet<>(); - composedBy.forEach((name, methods) -> { - if (!declaredByAnyone.getOrDefault(name, false) && methods.size() > 1) { - ambiguous.add(name); - } - }); - return ambiguous; - }); - } - private void collectClassNamesInHierarchy(ImClass c, Set out, Set visited) { if (c == null || !visited.add(c)) { return; From de578a65fc7b5499f9a6e3216aa54d0ca62fbaab Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 17:17:01 +0200 Subject: [PATCH 5/6] Let an override of a numbered overload replace its ancestor's slot A method the translation numbered to keep it apart from its overloads carries that number in the segment a slot name is composed from: the second route of a class is mangled to Base_route1, so the segment is route1 where the declaration says route. Requiring the segment to equal the declared name refuses the alias for every overload past the first, so an override of one never composes the class-qualified slot it needs to replace, and a call through the base stays bound to the base implementation. The number is the translation's own and means the same method, so it is allowed. Anything else between the two names is not, which is the case the check exists for: a segment which is a type argument rather than a method, composing a slot that names nothing. The test is the case that was broken - a generic base and a subclass each declaring two overloads of one method, called through a base-typed reference. It passes with the previous composers and failed with this rule, so it is a regression this introduced rather than something it inherited. --- .../imtranslation/LuaDispatchPreparation.java | 33 ++++++++++++++++++- .../lua/translation/LuaTranslator.java | 8 ++++- .../tests/LuaTranslationTests.java | 22 +++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java index 6f9ee47e4..1be77ae3f 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java @@ -187,7 +187,7 @@ private static void addDirectAliases(ImMethod method, Set aliases) { // as the type argument does not entitle the others to the slot it owns. String composed = directAliasFor(method); if (composed != null - && semanticNameFromMethodName(method.getName()).equals(declaredName(method))) { + && namesItsDeclaredMethod(method)) { aliases.add(composed); } String sourceSemanticName = sourceSemanticName(method); @@ -305,6 +305,37 @@ private static boolean sharesSemanticName(ImMethod method, Set semanticN } /** The name the method was written with, or empty when there is no declaration to ask. */ + /** + * Whether the name recovered from this method's mangled name is the name it was declared with. + *

+ * The recovered name is the segment after the last underscore, and a method the translation + * numbered to keep it apart from its overloads carries that number in the segment: the second + * {@code route} of a class is mangled to {@code Base_route1}, so the segment is {@code route1} + * where the declaration says {@code route}. Requiring the two to be equal refuses the alias for + * every overload past the first, which loses the class-qualified slot an override of it needs to + * replace - a call through the base then stays bound to the base implementation. + *

+ * The number is the translation's own and means the same method, so it is allowed. Anything else + * between the two names is not: that is the case this check exists for, where the segment is a type + * argument rather than a method and the slot composed from it names nothing. + */ + private static boolean namesItsDeclaredMethod(ImMethod method) { + String recovered = semanticNameFromMethodName(method.getName()); + String declared = declaredName(method); + if (recovered.equals(declared)) { + return true; + } + if (declared.isEmpty() || !recovered.startsWith(declared)) { + return false; + } + return isOverloadNumber(recovered.substring(declared.length())); + } + + /** The suffix the translation appends to tell overloads of one name apart. */ + public static boolean isOverloadNumber(String suffix) { + return !suffix.isEmpty() && suffix.chars().allMatch(Character::isDigit); + } + /** The name a method carries in the source, which a method and its overrides all share. */ public static String declaredName(ImMethod method) { if (method == null) { 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 4041b691e..081eca069 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 @@ -1025,7 +1025,13 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List", + " function route(T t) returns int", + " return 1", + " function route(T t, int extra) returns int", + " return 2", + "class Child extends Base", + " override function route(int t) returns int", + " return 10", + " override function route(int t, int extra) returns int", + " return 20", + "init", + " Base b = new Child()", + " if b.route(1) == 10 and b.route(1, 2) == 20", + " testSuccess()" + ); + } + @Test public void luaOutputIsDeterministicForGenericOverrideSlots() throws IOException { test().testLua(true).compilationUnits(genericOverrideReproUnits()); From 5ac3bfa8ceee9a26a3284174750cfc7d5e66d406 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 17:56:51 +0200 Subject: [PATCH 6/6] Check the type name a numeric suffix could be confused with Allowing an overload number after the declared name raises the question of a type argument named the same way: a class called route1 used as the argument of a generic which also declares route. It holds, and for a structural reason rather than luck - the type argument is part of the owning class's name, not the tail of the method's, so the segment a slot name is composed from is route for route and route1 for route1 and the number is never reached. Kept as a test because which segment carries the type argument is the sort of thing to check rather than argue about. --- .../tests/LuaTranslationTests.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) 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 e1b3fc4ac..325f22dab 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 @@ -1430,6 +1430,48 @@ public void overloadedOverrideOnAGenericBaseIsReachedThroughTheBase() { ); } + /** + * A type argument whose name is a method's name followed by a number, which is what the rule + * allowing an overload number could in principle be fooled by. + *

+ * It holds, because the type argument is part of the owning class's name rather than the tail of the + * method's: the segment a slot name is composed from is {@code route} for {@code route} and + * {@code route1} for {@code route1}, and neither needs the number tolerated. The case is kept + * because it was raised against that rule and reasoning about which segment carries the type is + * exactly the kind of thing to check rather than argue about. + */ + /** + * A type argument whose name is a method's name followed by a number, which is what the rule + * allowing an overload number could in principle be fooled by. + *

+ * It holds, because the type argument is part of the owning class's name rather than the tail of the + * method's: the segment a slot name is composed from is {@code route} for {@code route} and + * {@code route1} for {@code route1}, and neither needs the number tolerated. The case is kept + * because it was raised against that rule, and which segment carries the type argument is exactly + * the kind of thing to check rather than argue about. + */ + @Test + public void aTypeNamedLikeAnOverloadNumberDoesNotStealTheSlot() { + test().testLua(true).executeProg().lines( + "package test", + "native testSuccess()", + "class route1", + " int v = 3", + "class Holder", + " T item", + " construct(T item)", + " this.item = item", + " function route() returns int", + " return 1", + " function route1() returns int", + " return 2", + "init", + " let h = new Holder(new route1())", + " if h.route() == 1 and h.route1() == 2", + " testSuccess()" + ); + } + @Test public void luaOutputIsDeterministicForGenericOverrideSlots() throws IOException { test().testLua(true).compilationUnits(genericOverrideReproUnits());