From 5970878a12743eb3fa9b7211b3733ce7b676c214 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sun, 16 Aug 2026 20:46:48 +0200 Subject: [PATCH 1/3] Verify the junk dispatch slot and record why it stays Confirmed dead: in the Lua for FastHashMap the slot is assigned once, bound to the alphabetically first method, and never called - the only method calls in that script are the two constructors. Confirmed harder to remove than the note implied. addDirectAliases was changed to drop a composed name which more than one method of the class produces, on the grounds that a name meaning one of these arbitrarily is worse than a name meaning nothing, and the slot was still emitted: collectDispatchSlotNames composes it independently from the cross product of hierarchy class names and the group's semantic names, and being called per group it cannot see the sibling collision without class-level knowledge it is not given. So a fix needs a shared notion of ambiguity across both composers, which is the coordinated change the note weighed against the benefit, and the benefit is still one unused table key per specialised class. Reverted, and the item now carries what was verified rather than what was assumed. --- BACKLOG.md | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 86165e337..a9ef91269 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -107,23 +107,36 @@ itself, and one gap in what the suite can see. `simplifyClasses` nor `addMemberTypeArguments`, so the type variables are never lifted there and a super call has nothing to carry. Closing it is item 23. -15. **One junk dispatch slot per specialised class.** `addDirectAliases` and - `LuaTranslator.collectDispatchSlotNames` both compose `owner.getName() + "_" + - semanticNameFromMethodName(name)`, and for a specialised method that trailing segment is the - type argument — so every method of `FastHashMap` claims one shared - `FastHashMap_specialized_integer__integer` slot and the alphabetically first wins it. - Nothing calls it, so it is dead weight rather than a wrong result. - - Tried using the declared name instead and reverted it: overloads share a declared name, so - `setup(int)` and `setup(string)` collapse into one slot, which is what +15. **One junk dispatch slot per specialised class.** Confirmed dead weight, and confirmed harder to + remove than it looks. Leave it unless it stops being dead. + + In the Lua for `FastHashMap` the slot + `FastHashMap_specialized_integer__integer.FastHashMap_specialized_integer__integer_integer` is + assigned once, bound to `FastHashMap_get_specialized` — the alphabetically first method — and + never called: the only `:` calls in that script are `:create(` and `:create1(`. The name is the + owner's plus the segment after the last underscore of the method's, and for a specialised method + that segment is the type argument, so every method of the class composes the same one. + + **Two composers emit it, and suppressing it in one is not enough.** `addDirectAliases` was made + to drop a composed name which more than one method of the class produces — reasoning that a name + meaning "one of these, arbitrarily" is worse than a name meaning nothing — and the slot was still + emitted, because `LuaTranslator.collectDispatchSlotNames` composes it independently, from the + cross product of the class names in the hierarchy and the semantic names of the group. That + method is called per group of same-named methods, so it cannot see that a sibling group composes + the same name without being given class-level knowledge it does not currently have. + + So a fix means a shared notion of ambiguity across both composers, which is the coordinated + change the earlier note weighed against the benefit. The benefit is still one unused table key + per specialised class. Reverted. + + Also tried and reverted earlier: using the declared name instead. Overloads share a declared + name, so `setup(int)` and `setup(string)` collapse into one slot, which `LuaTranslationTests.overloadedMethodsDoNotAliasInLuaDispatchTables` and `moduleProvidedOverloadedOverrideDoesNotCollapseLuaSlots` exist to prevent. Both sources of a - semantic name are wrong, in opposite directions: the mangled trailing segment collides across - the siblings of one specialisation, the declared name collides across overloads. A fix needs a - name separating both — the declared name together with the dispatch signature key would, since - that is already what distinguishes overloads elsewhere in the same file. Worth doing only if - this stops being dead weight, because the cost of getting it wrong is a real mis-binding while - the cost of leaving it is one unused table key per specialised class. + semantic name are wrong in opposite directions: the mangled trailing segment collides across the + siblings of one specialisation, the declared name across overloads. A name separating both — the + declared name together with the dispatch signature key — is what a real fix needs, in both + composers. 24. **`luaOutputIsDeterministicForGenericOverrideSlots` fails intermittently.** It failed once on Windows CI and passed on a re-run of the same commit, having blocked an unrelated pull request From dea459b45694284e025c0f83d8c07e50f72c331d Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 11:54:13 +0200 Subject: [PATCH 2/3] Stop composing a dispatch slot which names no method A slot's name is the owner's plus the segment after the last underscore of the method's, and for a specialised method that segment is the type argument - so every method of one specialisation composed the same name and whichever was bound first claimed it. Nothing called it, which is why it went unnoticed. Both composers now leave such a name uncomposed. What tells this apart from ordinary dispatch is the name in the source: a method and its overrides all declare the same one and must share a slot, while the siblings of one specialisation declare different ones and merely end up composing the same segment. Counting methods rather than declared names breaks the override chain tests, which is how the distinction was found. everySlotOnASpecialisedClassNamesAMethod pins it: every slot on a specialised class carries a method name, and the shared one is no longer emitted. --- .../imtranslation/LuaDispatchPreparation.java | 62 +++++++++++++++++-- .../lua/translation/LuaTranslator.java | 41 ++++++++++++ .../wurstscript/tests/FastHashMapTests.java | 42 +++++++++++++ 3 files changed, 139 insertions(+), 6 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 86b95b25c..0257e388a 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 @@ -18,6 +18,8 @@ import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import org.eclipse.jdt.annotation.Nullable; + import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -114,9 +116,11 @@ 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); + addDirectAliases(method, aliases, ambiguousDirectAliases); addHierarchyAliases(method, aliases, sortedMethodsByClass); addClosureFamilyAliases(prog, method, aliases, sortedMethodsByClass, closureFamilyAnchorsCache, closureFamilyClassesByAnchor); method.setLuaMethodDispatchAliases(new ArrayList<>(aliases)); @@ -146,7 +150,52 @@ private static String uniqueName(String name, Set usedNames) { return result; } - private static void addDirectAliases(ImMethod method, Set aliases) { + /** + * The composed names which more than one method of the same class produces. + *

+ * The name is the owner's plus the segment after the last underscore of the method's. For a + * specialised class that segment is the type argument, so every method of + * {@code FastHashMap} composes the same one, which then names no method in particular. + * Whichever is bound first would claim it, so it is left unbound: a name meaning "one of these, + * 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) { + Map claimedBy = new LinkedHashMap<>(); + Set ambiguous = new HashSet<>(); + 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. What tells the two cases apart is the + // name in the source: overrides all declare the same one, while 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 rather than a method name. + String identity = declaredName(method); + String previous = claimedBy.put(composed, identity); + if (previous != null && !previous.equals(identity)) { + ambiguous.add(composed); + } + } + return ambiguous; + } + + private static @Nullable String directAliasFor(ImMethod method) { + if (method == null) { + return null; + } + ImClass owner = method.attrClass(); + String semanticName = semanticNameFromMethodName(method.getName()); + if (owner == null || semanticName.isEmpty()) { + return null; + } + return owner.getName() + "_" + semanticName; + } + + private static void addDirectAliases(ImMethod method, Set aliases, + Set ambiguousDirectAliases) { if (method == null) { return; } @@ -155,9 +204,9 @@ private static void addDirectAliases(ImMethod method, Set aliases) { aliases.add(methodName); } ImClass owner = method.attrClass(); - String semanticName = semanticNameFromMethodName(methodName); - if (owner != null && !semanticName.isEmpty()) { - aliases.add(owner.getName() + "_" + semanticName); + String composed = directAliasFor(method); + if (composed != null && !ambiguousDirectAliases.contains(composed)) { + aliases.add(composed); } String sourceSemanticName = sourceSemanticName(method); if (owner != null && isClosureGeneratedClass(owner) && !sourceSemanticName.isEmpty()) { @@ -274,7 +323,8 @@ private static boolean sharesSemanticName(ImMethod method, Set semanticN } /** The name the method was written with, or empty when there is no declaration to ask. */ - private static String declaredName(ImMethod method) { + /** 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) { return ""; } 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 ea86cbfc1..4b3a42a8b 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.LuaDispatchPreparation; import de.peeeq.wurstscript.translation.imtranslation.LuaNativeLowering; import de.peeeq.wurstscript.types.TypesHelper; import de.peeeq.wurstscript.utils.Lazy; @@ -1029,10 +1030,19 @@ private Set 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)); } } @@ -1040,6 +1050,37 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List + * A method and its overrides share a semantic name and must share a slot: that is dispatch. 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 a slot composed from it would be claimed by whichever is bound first, then + * never called. So the test is how many distinct declared names produce it. + */ + private Set ambiguousSemanticNames(ImClass c) { + Map> declaredNames = new TreeMap<>(); + for (ImMethod m : collectMethodsInHierarchy(c)) { + if (m == null) { + continue; + } + String semanticName = semanticNameFromMethodName(m.getName()); + if (semanticName.isEmpty()) { + continue; + } + declaredNames.computeIfAbsent(semanticName, name -> new TreeSet<>()) + .add(LuaDispatchPreparation.declaredName(m)); + } + Set ambiguous = new TreeSet<>(); + declaredNames.forEach((name, names) -> { + if (names.size() > 1) { + ambiguous.add(name); + } + }); + return ambiguous; + } + private void collectClassNamesInHierarchy(ImClass c, Set out, Set visited) { if (c == null || !visited.add(c)) { return; 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 446610d5b..074d47b85 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 @@ -474,6 +474,48 @@ public void aFieldNamedLikeAMethodKeepsOneKeyAcrossSpecialisation() throws IOExc compiledLua("aFieldNamedLikeAMethodKeepsOneKeyAcrossSpecialisation")); } + /** + * Every dispatch slot on a specialised class names one of its methods. + *

+ * A slot's name is composed from the owner's and the segment after the last underscore of the + * method's, and for a specialised method that segment is the type argument - so every method of + * one specialisation used to compose the same name and the first bound claimed it. Nothing called + * it, which is why it went unnoticed; it is not emitted now, and this says so rather than leaving + * the next reader to wonder what it was. + *

+ * A method and its overrides do share a slot, which is dispatch rather than a collision. They + * share a declared name, which is what tells the two cases apart. + */ + @Test + public void everySlotOnASpecialisedClassNamesAMethod() throws IOException { + test().testLua(true).executeProg().lines(program(fastHashMap(), INT_INSTANCE, USE_WITH_COLLISION)); + String lua = compiledLua("everySlotOnASpecialisedClassNamesAMethod"); + + 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; + } + // A real slot carries a method name; the junk one was the class name and the type + // argument with no method name anywhere in it. + 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 2d15413b03d24c154c36b9e970cfc81b13a20792 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 12:31:04 +0200 Subject: [PATCH 3/3] Cache the ambiguity check, and say what it does not cover Three things from review. The dispatch group key would separate overloads, which the declared name does not, so a specialised class holding only overloads of one method keeps one dead key. It cannot be used: the key embeds the signature, and a generic override chain's signatures differ by each class's type variable, so overrides read as unrelated and lose the slot they must share - the override chain tests fail exactly that way. Both composers say so where the decision is made, and the backlog item records the residual rather than claiming the slot is gone everywhere. The check is computed once per receiver class now. createMethods asks twice per dispatch group and each ask rebuilt and sorted the whole inherited method list. Item 15 said to leave the slot alone and that a fix had been reverted, which is the opposite of what this branch does. It now describes what landed and what is left. --- BACKLOG.md | 51 +++++++--------- .../imtranslation/LuaDispatchPreparation.java | 12 ++-- .../lua/translation/LuaTranslator.java | 60 ++++++++++++------- 3 files changed, 66 insertions(+), 57 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index a9ef91269..643205d5e 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -107,36 +107,27 @@ itself, and one gap in what the suite can see. `simplifyClasses` nor `addMemberTypeArguments`, so the type variables are never lifted there and a super call has nothing to carry. Closing it is item 23. -15. **One junk dispatch slot per specialised class.** Confirmed dead weight, and confirmed harder to - remove than it looks. Leave it unless it stops being dead. - - In the Lua for `FastHashMap` the slot - `FastHashMap_specialized_integer__integer.FastHashMap_specialized_integer__integer_integer` is - assigned once, bound to `FastHashMap_get_specialized` — the alphabetically first method — and - never called: the only `:` calls in that script are `:create(` and `:create1(`. The name is the - owner's plus the segment after the last underscore of the method's, and for a specialised method - that segment is the type argument, so every method of the class composes the same one. - - **Two composers emit it, and suppressing it in one is not enough.** `addDirectAliases` was made - to drop a composed name which more than one method of the class produces — reasoning that a name - meaning "one of these, arbitrarily" is worse than a name meaning nothing — and the slot was still - emitted, because `LuaTranslator.collectDispatchSlotNames` composes it independently, from the - cross product of the class names in the hierarchy and the semantic names of the group. That - method is called per group of same-named methods, so it cannot see that a sibling group composes - the same name without being given class-level knowledge it does not currently have. - - So a fix means a shared notion of ambiguity across both composers, which is the coordinated - change the earlier note weighed against the benefit. The benefit is still one unused table key - per specialised class. Reverted. - - Also tried and reverted earlier: using the declared name instead. Overloads share a declared - name, so `setup(int)` and `setup(string)` collapse into one slot, which - `LuaTranslationTests.overloadedMethodsDoNotAliasInLuaDispatchTables` and - `moduleProvidedOverloadedOverrideDoesNotCollapseLuaSlots` exist to prevent. Both sources of a - semantic name are wrong in opposite directions: the mangled trailing segment collides across the - siblings of one specialisation, the declared name across overloads. A name separating both — the - declared name together with the dispatch signature key — is what a real fix needs, in both - composers. +15. **A dead dispatch slot survives for overloads inside a specialised class.** What is left of the + junk slot, which is otherwise gone. + + A slot's name is the owner's plus the segment after the last underscore of the method's, and for a + specialised method that segment is the type argument, so every method of one specialisation + composes the same name. Both composers now leave such a name uncomposed, deciding by how many + distinct declared names produce it: a method and its overrides declare one name and must share a + slot, while siblings declare different ones. + + That leaves overloads. Two overloads of one source method share a declared name, so a specialised + class holding only overloads still composes one shared name and binds it to whichever is reached + first. Dead weight as before - nothing calls it - but no longer true of the general case. + + The sharper identity is the dispatch group key, which separates overloads by signature. It cannot + be used here: the signature embeds each class's type variable, so a generic override chain reads as + `void|T192,real` against `void|T636,real` and the overrides look unrelated, which drops the slot + they must share. `LuaTranslationTests.genericOverrideChainBindsRootSlotToMostSpecificImplInLua` + fails exactly that way, and is how this was found rather than shipped. + + A fix needs an identity which treats a chain's differing type variables as the same signature while + still separating real parameter differences. Worth doing only if this stops being dead weight. 24. **`luaOutputIsDeterministicForGenericOverrideSlots` fails intermittently.** It failed once on Windows CI and passed on a re-run of the same commit, having blocked an unrelated pull request 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 0257e388a..4891947a2 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 @@ -169,10 +169,14 @@ 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. What tells the two cases apart is the - // name in the source: overrides all declare the same one, while 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 rather than a method name. + // 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); String previous = claimedBy.put(composed, identity); if (previous != null && !previous.equals(identity)) { 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 4b3a42a8b..d9f6ba2dd 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 @@ -1051,34 +1051,48 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List - * A method and its overrides share a semantic name and must share a slot: that is dispatch. 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 a slot composed from it would be claimed by whichever is bound first, then - * never called. So the test is how many distinct declared names produce it. + * 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. + *

+ * 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) { - Map> declaredNames = new TreeMap<>(); - for (ImMethod m : collectMethodsInHierarchy(c)) { - if (m == null) { - continue; - } - String semanticName = semanticNameFromMethodName(m.getName()); - if (semanticName.isEmpty()) { - continue; - } - declaredNames.computeIfAbsent(semanticName, name -> new TreeSet<>()) - .add(LuaDispatchPreparation.declaredName(m)); - } - Set ambiguous = new TreeSet<>(); - declaredNames.forEach((name, names) -> { - if (names.size() > 1) { - ambiguous.add(name); + return ambiguousSemanticNamesByClass.computeIfAbsent(c, owner -> { + Map> claimants = new TreeMap<>(); + for (ImMethod m : collectMethodsInHierarchy(owner)) { + if (m == null) { + continue; + } + String semanticName = semanticNameFromMethodName(m.getName()); + if (semanticName.isEmpty()) { + continue; + } + claimants.computeIfAbsent(semanticName, name -> new TreeSet<>()) + .add(LuaDispatchPreparation.declaredName(m)); } + Set ambiguous = new TreeSet<>(); + claimants.forEach((name, keys) -> { + if (keys.size() > 1) { + ambiguous.add(name); + } + }); + return ambiguous; }); - return ambiguous; } private void collectClassNamesInHierarchy(ImClass c, Set out, Set visited) {