From 8416af05188f5ea251c04311f679e96a07b9410f Mon Sep 17 00:00:00 2001 From: Frotty Date: Sun, 16 Aug 2026 18:22:12 +0200 Subject: [PATCH 1/5] Make the determinism failure say what differed It compiled the repro twice and compared the two whole scripts with assertEquals, so its one failure on CI printed an unreadable dump, left nothing behind, and cost a re-run to dismiss without a diagnosis. The assertion is unchanged - still byte for byte on two compiles in one run - but a failure now writes both scripts beside the test output and names the first differing lines with their numbers. Not reproduced while doing this: 250 compiles of that repro in one JVM, caches cleared between each, came out identical, and the hash-ordered iteration in the emission path is sorted where it reaches the output. Whatever differs is rarer than that or depends on something a local run does not vary. The backlog item records both the negative result and where not to look again. --- BACKLOG.md | 31 +++++++++------- .../tests/LuaTranslationTests.java | 37 ++++++++++++++++++- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index 86165e337..9ddf44f20 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -125,22 +125,27 @@ itself, and one gap in what the suite can see. 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. -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 +24. **`luaOutputIsDeterministicForGenericOverrideSlots` failed once and has not since.** It failed + on Windows CI and passed on a re-run of the same commit, having blocked an unrelated pull request in between. Do not weaken the assertion. It compiles one repro twice and compares the output byte for byte, - so an intermittent mismatch is evidence of intermittent nondeterminism in Lua emission, which is - exactly what it exists to catch — a re-run passing says the nondeterminism is intermittent, not - that the test is at fault. Calling it flaky was too quick. - - Diagnose it instead: capture both outputs on a failing run and diff them, and rule out harness - interference rather than assuming it. The two compiles do start from the same cache state, but - that comes from two separate resets: `WurstScriptTest`'s `@BeforeMethod` clears before the - first, and the explicit `GlobalCaches.clearAll()` between the compilations inside the test - clears before the second. Both are load bearing — remove either and the comparison stops being - between equal starting states, which would invalidate the conclusion rather than explain the - failure. + so a mismatch is evidence of nondeterminism in Lua emission, which is what it exists to catch. + + **Not reproduced.** 250 compiles of that repro in one JVM, caches cleared between each, came out + byte-identical. Sources of hash-ordered iteration in the emission path were read rather than + guessed at: `TypeId.calculate` sorts by name and package, `createMethods` groups through a + `TreeMap`, `assignDispatchAliases` collects into a `TreeSet` and iterates a list, and + `collectSuperClasses` uses its set only to mark what it has seen. None of those can vary. + + So whatever differs is either rarer than one in 250, or comes from something the local run does + not vary — a different core count changing the fork layout, memory pressure, or the interpreter + build on that runner. + + What changed meanwhile is that the failure now carries evidence: both scripts are written beside + the test output and the first differing lines are named with their numbers. The one occurrence so + far produced nothing to work from, which is why it cost a re-run and no diagnosis. The next one + will say what differed. 12. **Standing item, never finished.** When nothing above is left, find the next thing worth doing and add it here rather than stopping. Good sources, in order: a test that would have 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 014a3d9ce..c20dfeb3a 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 @@ -1418,7 +1418,42 @@ public void luaOutputIsDeterministicForGenericOverrideSlots() throws IOException test().testLua(true).compilationUnits(genericOverrideReproUnits()); String second = Files.toString(new File("test-output/lua/LuaTranslationTests_luaOutputIsDeterministicForGenericOverrideSlots.lua"), Charsets.UTF_8); - assertEquals(first, second); + if (!first.equals(second)) { + // This has failed once on CI and not since, and 250 compiles in one JVM did not + // reproduce it. A bare "expected X but got Y" over two whole scripts is unreadable and + // the run's output is gone by the time anyone looks, so the failure carries what it + // takes to act on: both scripts kept beside the test output, and the differing lines + // named. Without this the next occurrence is as unactionable as the first. + File firstFile = new File(TEST_OUTPUT_PATH, "determinism-first.lua"); + File secondFile = new File(TEST_OUTPUT_PATH, "determinism-second.lua"); + Files.write(first.getBytes(Charsets.UTF_8), firstFile); + Files.write(second.getBytes(Charsets.UTF_8), secondFile); + fail("the same program compiled to different Lua twice in one run." + + "\n" + describeFirstDifferences(first, second) + + "\nboth kept at " + firstFile.getPath() + " and " + secondFile.getPath()); + } + } + + /** The first few differing lines, with their numbers, so a failure says where to look. */ + private static String describeFirstDifferences(String first, String second) { + String[] a = first.split("\n", -1); + String[] b = second.split("\n", -1); + StringBuilder sb = new StringBuilder(); + int reported = 0; + for (int i = 0; i < Math.max(a.length, b.length) && reported < 5; i++) { + String lineA = i < a.length ? a[i] : ""; + String lineB = i < b.length ? b[i] : ""; + if (!lineA.equals(lineB)) { + sb.append(" line ").append(i + 1).append(":\n") + .append(" first: ").append(lineA).append('\n') + .append(" second: ").append(lineB).append('\n'); + reported++; + } + } + if (reported == 0) { + sb.append(" the scripts differ but no line does, so it is line endings or trailing bytes"); + } + return sb.toString(); } @Test From b5ce109b1b0f363f701a012ad4d36fffbca4bc46 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 10:25:13 +0200 Subject: [PATCH 2/5] Keep the determinism scripts past the end of the CI job The failure wrote both scripts beside the test output, which preserves nothing where it matters: on a runner they go when the runner goes, and the only upload step is the release archive, which does not run on failure at all. So the evidence the last change promised was still gone by the time anyone read the check. The workflow now uploads the two scripts when a job fails, ignoring their absence since most failures are not this test. The message no longer stops at five differing lines either: it states how many differ and lists up to forty, because an artifact needs fetching while the check is what gets read first. --- .github/workflows/build.yml | 15 +++++++++++++ .../tests/LuaTranslationTests.java | 21 +++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index fa77fdecc..1eaae2807 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -127,6 +127,21 @@ jobs: check_name: Test Results (${{ matrix.os }}) fail_on_failure: false + # luaOutputIsDeterministicForGenericOverrideSlots compiles one program twice and keeps both + # scripts when they differ. They are written to the runner's filesystem, so without this the + # evidence goes with the runner and the failure is as unactionable as it was before it kept + # anything. Most failures are not this test, hence if-no-files-found: ignore. + - name: Upload determinism scripts (on failure) + if: failure() + uses: actions/upload-artifact@v4 + with: + name: determinism-scripts-${{ matrix.os }} + path: | + de.peeeq.wurstscript/test-output/determinism-first.lua + de.peeeq.wurstscript/test-output/determinism-second.lua + if-no-files-found: ignore + retention-days: 14 + - name: Upload packaged artifact (per-OS) uses: actions/upload-artifact@v4 with: 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 c20dfeb3a..16c533f29 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 @@ -1434,13 +1434,30 @@ public void luaOutputIsDeterministicForGenericOverrideSlots() throws IOException } } - /** The first few differing lines, with their numbers, so a failure says where to look. */ + /** + * Every differing line, with its number, so the failure itself says what changed. + *

+ * The scripts are also uploaded as an artifact on a failing CI run, but the message has to stand + * on its own: an artifact needs fetching, and the check is what gets read first. Bounded so a + * wholesale difference does not bury the report, with the total stated either way. + */ private static String describeFirstDifferences(String first, String second) { + final int reportLimit = 40; String[] a = first.split("\n", -1); String[] b = second.split("\n", -1); StringBuilder sb = new StringBuilder(); + int differing = 0; + for (int i = 0; i < Math.max(a.length, b.length); i++) { + String lineA = i < a.length ? a[i] : ""; + String lineB = i < b.length ? b[i] : ""; + if (!lineA.equals(lineB)) { + differing++; + } + } + sb.append(" ").append(differing).append(" line(s) differ") + .append(differing > reportLimit ? ", first " + reportLimit + ":\n" : ":\n"); int reported = 0; - for (int i = 0; i < Math.max(a.length, b.length) && reported < 5; i++) { + for (int i = 0; i < Math.max(a.length, b.length) && reported < reportLimit; i++) { String lineA = i < a.length ? a[i] : ""; String lineB = i < b.length ? b[i] : ""; if (!lineA.equals(lineB)) { From d5ca85a8c14e6b4d0b2bccaed2e6553f8108806d Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 11:18:01 +0200 Subject: [PATCH 3/5] Align the two scripts before saying what changed Comparing equal line indexes is not a diff. Emission order moving a block shifts every line after it, so the count became "everything from here down" and the listed pairs were unrelated - and a moved block is exactly what this diagnostic exists to investigate, so that was the case it described worst. Aligned on the longest common subsequence now: a moved block reads as one removal and one addition, and the summary counts each side separately. A pathological pair falls back to reporting the sizes rather than allocating a table for it. DeterminismDiffReportTest covers the summary itself - insertion, move, in-place replacement, and two scripts whose lines all match. It will only ever be read during a failure nobody can reproduce, so it is worth testing rather than trusting. --- .../tests/DeterminismDiffReportTest.java | 66 +++++++++++++++ .../tests/LuaTranslationTests.java | 83 +++++++++++++------ 2 files changed, 124 insertions(+), 25 deletions(-) create mode 100644 de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java new file mode 100644 index 000000000..ba83cfebc --- /dev/null +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java @@ -0,0 +1,66 @@ +package tests.wurstscript.tests; + +import org.testng.annotations.Test; + +import static org.testng.Assert.assertTrue; + +/** + * Covers the summary the determinism test prints when it fails. + *

+ * The failure it describes has happened once and is not reproducible on demand, so the summary is + * the only thing that will be read when it happens again. That makes it worth testing on its own: + * a description which misreports what changed is worse than none, because it sends the next person + * looking in the wrong place. + */ +public class DeterminismDiffReportTest { + + /** + * A block appearing in one script and not the other reads as one addition. Comparing equal + * indexes instead would report every line after the insertion as differing, which is the case + * emission-order nondeterminism actually produces. + */ + @Test + public void anInsertedBlockIsOneAdditionRatherThanEverythingAfterIt() { + String first = "a\nb\nc\nd\ne\nf\ng\nh\n"; + String second = "a\nb\nINSERTED\nc\nd\ne\nf\ng\nh\n"; + + String report = LuaTranslationTests.describeFirstDifferences(first, second); + + assertTrue(report.contains("0 line(s) only in the first, 1 only in the second"), + "one inserted line should read as one addition:\n" + report); + assertTrue(report.contains("INSERTED"), "the added line should be named:\n" + report); + assertTrue(report.contains("line 3"), "the added line's number should be given:\n" + report); + } + + /** A block moved rather than inserted reads as one removal and one addition, not a cascade. */ + @Test + public void aMovedLineIsOneRemovalAndOneAddition() { + String first = "one\nmoved\ntwo\nthree\nfour\n"; + String second = "one\ntwo\nthree\nmoved\nfour\n"; + + String report = LuaTranslationTests.describeFirstDifferences(first, second); + + assertTrue(report.contains("1 line(s) only in the first, 1 only in the second"), + "a moved line should read as one removal and one addition:\n" + report); + } + + /** Replacing a line in place is a removal and an addition at the same position. */ + @Test + public void aReplacedLineNamesBothVersions() { + String first = "x\nbefore\nz\n"; + String second = "x\nafter\nz\n"; + + String report = LuaTranslationTests.describeFirstDifferences(first, second); + + assertTrue(report.contains("before"), "the first version should be named:\n" + report); + assertTrue(report.contains("after"), "the second version should be named:\n" + report); + } + + /** Identical scripts differing only in trailing bytes say so rather than listing nothing. */ + @Test + public void identicalLinesReportTheTrailingByteCase() { + String report = LuaTranslationTests.describeFirstDifferences("a\nb\n", "a\nb\n"); + + assertTrue(report.contains("no line does"), "expected the trailing byte wording:\n" + report); + } +} 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 16c533f29..21b6fd298 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 @@ -1435,40 +1435,73 @@ public void luaOutputIsDeterministicForGenericOverrideSlots() throws IOException } /** - * Every differing line, with its number, so the failure itself says what changed. + * What changed between the two scripts, aligned rather than compared line by line. + *

+ * Comparing equal indexes is not a diff: emission order moving a block shifts every line after + * it, so the count becomes "everything from here down" and the listed pairs are unrelated. That + * is the shape this diagnostic exists to investigate, so it is the shape it has to describe. + * Aligned on the longest common subsequence, a moved block reads as one removal and one addition. *

* The scripts are also uploaded as an artifact on a failing CI run, but the message has to stand * on its own: an artifact needs fetching, and the check is what gets read first. Bounded so a - * wholesale difference does not bury the report, with the total stated either way. + * wholesale difference does not bury the report, with the totals stated either way. */ - private static String describeFirstDifferences(String first, String second) { + private static final String NEWLINE_RE = "\n"; + private static final char NEWLINE = '\n'; + + static String describeFirstDifferences(String first, String second) { final int reportLimit = 40; - String[] a = first.split("\n", -1); - String[] b = second.split("\n", -1); - StringBuilder sb = new StringBuilder(); - int differing = 0; - for (int i = 0; i < Math.max(a.length, b.length); i++) { - String lineA = i < a.length ? a[i] : ""; - String lineB = i < b.length ? b[i] : ""; - if (!lineA.equals(lineB)) { - differing++; + String[] a = first.split(NEWLINE_RE, -1); + String[] b = second.split(NEWLINE_RE, -1); + + // O(n*m) in memory, so a pathological pair falls back to reporting the sizes rather than + // exhausting the worker. The scripts this compares are a few hundred lines. + if ((long) a.length * b.length > 4_000_000L) { + return " too large to align: " + a.length + " lines against " + b.length; + } + + int[][] common = new int[a.length + 1][b.length + 1]; + for (int i = a.length - 1; i >= 0; i--) { + for (int j = b.length - 1; j >= 0; j--) { + common[i][j] = a[i].equals(b[j]) + ? common[i + 1][j + 1] + 1 + : Math.max(common[i + 1][j], common[i][j + 1]); } } - sb.append(" ").append(differing).append(" line(s) differ") - .append(differing > reportLimit ? ", first " + reportLimit + ":\n" : ":\n"); - int reported = 0; - for (int i = 0; i < Math.max(a.length, b.length) && reported < reportLimit; i++) { - String lineA = i < a.length ? a[i] : ""; - String lineB = i < b.length ? b[i] : ""; - if (!lineA.equals(lineB)) { - sb.append(" line ").append(i + 1).append(":\n") - .append(" first: ").append(lineA).append('\n') - .append(" second: ").append(lineB).append('\n'); - reported++; + + List entries = new ArrayList<>(); + int removed = 0; + int added = 0; + int i = 0; + int j = 0; + while (i < a.length || j < b.length) { + if (i < a.length && j < b.length && a[i].equals(b[j])) { + i++; + j++; + } else if (j >= b.length || (i < a.length && common[i + 1][j] >= common[i][j + 1])) { + removed++; + if (entries.size() < reportLimit) { + entries.add(" only in first, line " + (i + 1) + ": " + a[i]); + } + i++; + } else { + added++; + if (entries.size() < reportLimit) { + entries.add(" only in second, line " + (j + 1) + ": " + b[j]); + } + j++; } } - if (reported == 0) { - sb.append(" the scripts differ but no line does, so it is line endings or trailing bytes"); + + if (removed == 0 && added == 0) { + return " the scripts differ but no line does, so it is line endings or trailing bytes"; + } + StringBuilder sb = new StringBuilder(); + sb.append(" ").append(removed).append(" line(s) only in the first, ") + .append(added).append(" only in the second"); + sb.append(removed + added > entries.size() ? ", first " + entries.size() + ":" + NEWLINE : ":" + NEWLINE); + for (String entry : entries) { + sb.append(entry).append(NEWLINE); } return sb.toString(); } From 833023c95a9556ab5e4e6f1123f603594332e67c Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 11:34:05 +0200 Subject: [PATCH 4/5] Split on either terminator, so the line-ending case is reachable The branch reporting "no line differs" could not be reached. The caller only asks after finding the two unequal, and splitting on "\n" alone leaves the carriage return in the line text - so a CRLF script differed from an LF one on every line, and any trailing difference read as a changed line. The test I wrote for that branch passed two identical strings, a state production never reaches, which is why it looked covered. Splitting on either terminator aligns the two line for line, so scripts whose lines all match now reach the branch and it says what it means: the difference is in terminators or trailing bytes, with both lengths. The test passes CRLF against LF and asserts no line is reported as removed. --- .../imtranslation/LuaDispatchPreparation.java | 53 +++++++++++++++++-- .../lua/translation/LuaTranslator.java | 30 +++++++++++ .../tests/DeterminismDiffReportTest.java | 50 +++++++++++++++-- .../tests/LuaTranslationTests.java | 14 ++++- 4 files changed, 136 insertions(+), 11 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..9e574d8b6 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,46 @@ 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; + } + ImMethod previous = claimedBy.put(composed, method); + if (previous != null && previous != method) { + 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 +198,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()) { 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..6174b0532 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 @@ -1029,10 +1029,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 +1049,27 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List ambiguousSemanticNames(ImClass c) { + Map counts = new TreeMap<>(); + for (ImMethod m : collectMethodsInHierarchy(c)) { + if (m == null) { + continue; + } + String semanticName = semanticNameFromMethodName(m.getName()); + if (!semanticName.isEmpty()) { + counts.merge(semanticName, 1, Integer::sum); + } + } + Set ambiguous = new TreeSet<>(); + counts.forEach((name, count) -> { + if (count > 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/DeterminismDiffReportTest.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java index ba83cfebc..8f890ab84 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java @@ -2,6 +2,8 @@ import org.testng.annotations.Test; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotEquals; import static org.testng.Assert.assertTrue; /** @@ -56,11 +58,51 @@ public void aReplacedLineNamesBothVersions() { assertTrue(report.contains("after"), "the second version should be named:\n" + report); } - /** Identical scripts differing only in trailing bytes say so rather than listing nothing. */ + /** + * Two scripts whose lines all match but which are not equal differ in how the lines end. The + * caller only asks after finding them unequal, so passing identical strings would test a state + * production never reaches — this passes CRLF against LF, which it can. + */ + @Test + public void differingOnlyInLineEndingsSaysSoRatherThanListingEveryLine() { + String crlf = "alpha\r\nbeta\r\ngamma\r\n"; + String lf = "alpha\nbeta\ngamma\n"; + assertNotEquals(crlf, lf, "the two inputs must be unequal for this to mean anything"); + + String report = LuaTranslationTests.describeFirstDifferences(crlf, lf); + + assertTrue(report.contains("line terminators"), + "line endings should be named rather than every line reported as changed:\n" + report); + assertFalse(report.contains("only in the first"), + "no line should be reported as removed:\n" + report); + } + + @Test + public void probeShowsTheWording() { + System.err.println("PROBE-CRLF: " + LuaTranslationTests.describeFirstDifferences( + "alpha +beta +", "alpha +beta +")); + System.err.println("PROBE-MOVE: " + LuaTranslationTests.describeFirstDifferences( + "one +moved +two +three +", "one +two +three +moved +")); + } + + /** Bytes after the last line reach the same branch: every line matches, the scripts do not. */ @Test - public void identicalLinesReportTheTrailingByteCase() { - String report = LuaTranslationTests.describeFirstDifferences("a\nb\n", "a\nb\n"); + public void trailingBytesReachTheSameCase() { + String report = LuaTranslationTests.describeFirstDifferences("a\nb\n", "a\nb"); - assertTrue(report.contains("no line does"), "expected the trailing byte wording:\n" + report); + assertTrue(report.contains("line terminators") || report.contains("only in the"), + "a trailing difference should be described one way or the other:\n" + report); } } 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 21b6fd298..c64ac4144 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 @@ -1446,7 +1446,12 @@ public void luaOutputIsDeterministicForGenericOverrideSlots() throws IOException * on its own: an artifact needs fetching, and the check is what gets read first. Bounded so a * wholesale difference does not bury the report, with the totals stated either way. */ - private static final String NEWLINE_RE = "\n"; + /** + * Splits on either terminator, so a CRLF script aligns against an LF one line for line. Splitting + * on "\n" alone leaves the carriage return in the line text, which makes every line of a CRLF + * script differ from its LF counterpart and buries the actual difference. + */ + private static final String NEWLINE_RE = "\r?\n"; private static final char NEWLINE = '\n'; static String describeFirstDifferences(String first, String second) { @@ -1494,7 +1499,12 @@ static String describeFirstDifferences(String first, String second) { } if (removed == 0 && added == 0) { - return " the scripts differ but no line does, so it is line endings or trailing bytes"; + // Reached because the split accepts either terminator: two scripts whose lines all match + // and which are still unequal differ in how those lines end, or in bytes after the last + // one. Worth saying plainly rather than reporting every line as changed, which is what + // splitting on "\n" alone did - it left the carriage returns in the line text. + return " every line matches, so the two differ only in line terminators or trailing" + + " bytes: " + first.length() + " characters against " + second.length(); } StringBuilder sb = new StringBuilder(); sb.append(" ").append(removed).append(" line(s) only in the first, ") From d8d3f1b5a58239881182fb92a005aa6644e50e11 Mon Sep 17 00:00:00 2001 From: Frotty Date: Mon, 17 Aug 2026 11:37:57 +0200 Subject: [PATCH 5/5] Take the dispatch slot changes back out of this branch They belong to the junk dispatch slot work and reached this branch by riding along on a branch switch with uncommitted changes, then went out in the previous commit here. They also break two override chain tests, so this branch was red as well as carrying work that is not its own. This branch is the determinism diagnostic and nothing else. The probe I left in the diff report test while checking its wording is gone too. --- .../imtranslation/LuaDispatchPreparation.java | 53 ++----------------- .../lua/translation/LuaTranslator.java | 30 ----------- .../tests/DeterminismDiffReportTest.java | 20 ------- 3 files changed, 5 insertions(+), 98 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 9e574d8b6..86b95b25c 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,8 +18,6 @@ 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; @@ -116,11 +114,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)); @@ -150,46 +146,7 @@ private static String uniqueName(String name, Set usedNames) { return result; } - /** - * 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; - } - ImMethod previous = claimedBy.put(composed, method); - if (previous != null && previous != method) { - 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) { + private static void addDirectAliases(ImMethod method, Set aliases) { if (method == null) { return; } @@ -198,9 +155,9 @@ private static void addDirectAliases(ImMethod method, Set aliases, aliases.add(methodName); } ImClass owner = method.attrClass(); - String composed = directAliasFor(method); - if (composed != null && !ambiguousDirectAliases.contains(composed)) { - aliases.add(composed); + String semanticName = semanticNameFromMethodName(methodName); + if (owner != null && !semanticName.isEmpty()) { + aliases.add(owner.getName() + "_" + semanticName); } String sourceSemanticName = sourceSemanticName(method); if (owner != null && isClosureGeneratedClass(owner) && !sourceSemanticName.isEmpty()) { 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 6174b0532..ea86cbfc1 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 @@ -1029,19 +1029,10 @@ 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)); } } @@ -1049,27 +1040,6 @@ private Set collectDispatchSlotNames(ImClass receiverClass, List ambiguousSemanticNames(ImClass c) { - Map counts = new TreeMap<>(); - for (ImMethod m : collectMethodsInHierarchy(c)) { - if (m == null) { - continue; - } - String semanticName = semanticNameFromMethodName(m.getName()); - if (!semanticName.isEmpty()) { - counts.merge(semanticName, 1, Integer::sum); - } - } - Set ambiguous = new TreeSet<>(); - counts.forEach((name, count) -> { - if (count > 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/DeterminismDiffReportTest.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java index 8f890ab84..7aaa1dc2d 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java @@ -77,26 +77,6 @@ public void differingOnlyInLineEndingsSaysSoRatherThanListingEveryLine() { "no line should be reported as removed:\n" + report); } - @Test - public void probeShowsTheWording() { - System.err.println("PROBE-CRLF: " + LuaTranslationTests.describeFirstDifferences( - "alpha -beta -", "alpha -beta -")); - System.err.println("PROBE-MOVE: " + LuaTranslationTests.describeFirstDifferences( - "one -moved -two -three -", "one -two -three -moved -")); - } - /** Bytes after the last line reach the same branch: every line matches, the scripts do not. */ @Test public void trailingBytesReachTheSameCase() {