From 8d6283228bea03e4e393292d4a2ce86816122836 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 18:55:41 +0200 Subject: [PATCH 01/19] Model manager: check the users of Jass declared in .wurst files, and clear what used a removed or added unit (cherry picked from commit d3247f46edd8e7276654fec7c4594cca4b479743) --- .../wurstio/languageserver/ModelManager.java | 27 +- .../languageserver/ModelManagerImpl.java | 93 ++++--- .../wurstscript/tests/ModelManagerTests.java | 243 ++++++++++++++++++ 3 files changed, 324 insertions(+), 39 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManager.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManager.java index 67c7b15ad..24231b785 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManager.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManager.java @@ -141,15 +141,26 @@ class Changes { private static final Changes EMPTY = new Changes(HashSet.empty(), HashSet.empty()); private final HashSet affectedFiles; private final HashSet affectedPackageNames; + private final boolean jassNamesChanged; public Changes(Iterable affectedFiles, Iterable affectedPackageNames) { + this(affectedFiles, affectedPackageNames, false); + } + + /** + * @param jassNamesChanged whether a unit which declared Jass names was replaced or removed (see + * {@link #isJassNamesChanged()}) + */ + public Changes(Iterable affectedFiles, Iterable affectedPackageNames, boolean jassNamesChanged) { this.affectedFiles = HashSet.ofAll(affectedFiles); this.affectedPackageNames = HashSet.ofAll(affectedPackageNames); + this.jassNamesChanged = jassNamesChanged; } public Changes(Stream affectedFiles, Stream affectedPackageNames) { this.affectedFiles = HashSet.ofAll(affectedFiles); this.affectedPackageNames = HashSet.ofAll(affectedPackageNames); + this.jassNamesChanged = false; } public static Changes empty() { @@ -164,17 +175,27 @@ public HashSet getAffectedPackageNames() { return affectedPackageNames; } + /** + * Whether a unit which declared Jass names (a .j file, or Jass outside of the packages of a .wurst or .jurst + * file) was replaced or removed. Every package sees those names without an import, so every unit has to be + * checked again, and a removed unit is not in the model any more to tell. + */ + public boolean isJassNamesChanged() { + return jassNamesChanged; + } + public Changes mergeWith(Changes affected) { HashSet newF = affectedFiles.addAll(affected.affectedFiles); HashSet newP = affectedPackageNames.addAll(affected.affectedPackageNames); - if (newF == affectedFiles && newP == affectedPackageNames) { + boolean newJ = jassNamesChanged || affected.jassNamesChanged; + if (newF == affectedFiles && newP == affectedPackageNames && newJ == jassNamesChanged) { return this; } - return new Changes(newF, newP); + return new Changes(newF, newP, newJ); } public boolean isEmpty() { - return affectedFiles.isEmpty() && affectedPackageNames.isEmpty(); + return affectedFiles.isEmpty() && affectedPackageNames.isEmpty() && !jassNamesChanged; } } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManagerImpl.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManagerImpl.java index ce91e9021..0eb2190ae 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManagerImpl.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManagerImpl.java @@ -136,6 +136,7 @@ private List getJassdocCUs(Path jassdoc, WurstGui gui) { public Changes removeCompilationUnit(WFile resource) { WurstModel model2 = model; List toRemove = new ArrayList<>(); + boolean jassNamesRemoved = false; if (model2 != null) { for (CompilationUnit compilationUnit : model2) { if (wFile(compilationUnit).equals(resource)) { @@ -148,16 +149,18 @@ public Changes removeCompilationUnit(WFile resource) { .flatMap(cu -> cu.getPackages().stream()) .map(WPackage::getName) .collect(Collectors.toSet()); - Set dependents = toRemove.isEmpty() ? Collections.emptySet() - : calculateCUsToUpdate(Collections.emptyList(), removedPackages, model2); - synchronized (modelLock) { - model2.removeAll(toRemove); - uncheckedUnits.removeAll(toRemove); + jassNamesRemoved = toRemove.stream().anyMatch(ModelManagerImpl::declaresJassNames); + if (!toRemove.isEmpty()) { + Set dependents = + calculateCUsToUpdate(Collections.emptyList(), removedPackages, jassNamesRemoved, model2); dependents.removeAll(toRemove); - uncheckedUnits.addAll(dependents); - if (toRemove.stream().anyMatch(cu -> cu.getCuInfo().getFile().endsWith(".j"))) { - // Jass names are visible everywhere - uncheckedUnits.addAll(model2); + // what used the removed units resolves its names again, as after a replacement: a check of the + // whole model before the reconciliation must not find the bindings into the removed units + clearCompilationUnits(dependents); + synchronized (modelLock) { + model2.removeAll(toRemove); + uncheckedUnits.removeAll(toRemove); + uncheckedUnits.addAll(dependents); } } } @@ -173,7 +176,8 @@ public Changes removeCompilationUnit(WFile resource) { toRemove.stream() .flatMap(cu -> cu.getPackages().stream()) .map(WPackage::getName) - .collect(Collectors.toList()) + .collect(Collectors.toList()), + jassNamesRemoved ); } @@ -743,7 +747,8 @@ private void updateModel(CompilationUnit cu, WurstGui gui) { // get old provided packages: Set oldPackages = providedPackages(c); reconciliations++; - Set mustUpdate = calculateCUsToUpdate(Collections.singletonList(cu), oldPackages, model2); + Set mustUpdate = calculateCUsToUpdate(Collections.singletonList(cu), oldPackages, + declaresJassNames(c), model2); GlobalCaches.clearLookupCacheFor(Collections.singletonList(c)); clearCompilationUnits(mustUpdate); @@ -769,10 +774,16 @@ private void updateModel(CompilationUnit cu, WurstGui gui) { uncheckedUnits.add(cu); } } - if (!everyUnitUnchecked) { - // what imports the new packages (or sees the new Jass names) has to be checked again + if (everyUnitUnchecked) { + // the packages and names of the model include the new unit + model2.clearAttributesLocal(); + } else { + // what imports the new packages (or sees the new Jass names) has to be checked again, and resolves + // its names again, as after a replacement reconciliations++; - Set mustUpdate = calculateCUsToUpdate(Collections.singletonList(cu), Collections.emptySet(), model2); + Set mustUpdate = calculateCUsToUpdate(Collections.singletonList(cu), + Collections.emptySet(), false, model2); + clearCompilationUnits(mustUpdate); synchronized (modelLock) { uncheckedUnits.addAll(mustUpdate); } @@ -887,8 +898,9 @@ public Changes syncCompilationUnitContent(WFile filename, String contents) { return Changes.empty(); } Set oldPackages = declaredPackages(filename); + boolean oldJassNames = declaredJassNames(filename); replaceCompilationUnit(filename, contents, false); - return new Changes(io.vavr.collection.HashSet.of(filename), oldPackages); + return new Changes(io.vavr.collection.HashSet.of(filename), oldPackages, oldJassNames); } private Set declaredPackages(WFile f) { @@ -909,6 +921,21 @@ private Set declaredPackages(WFile f) { return Collections.emptySet(); } + /** Whether the unit of the file declares Jass names, see {@link #declaresJassNames(CompilationUnit)}. */ + private boolean declaredJassNames(WFile f) { + CompilationUnit cu = getCompilationUnit(f); + return cu != null && declaresJassNames(cu); + } + + /** + * Whether the unit declares Jass names: a .j file, or Jass declared outside of the packages of a .wurst or .jurst + * file. Every package sees those names without an import (the scope of the model holds the Jass declarations of + * every unit), so a change to them concerns every unit of the model. + */ + private static boolean declaresJassNames(CompilationUnit cu) { + return cu.getCuInfo().getFile().endsWith(".j") || !cu.getJassDecls().isEmpty(); + } + @Override public CompilationUnit replaceCompilationUnitContent(WFile filename, String contents, boolean reportErrors) { return replaceCompilationUnit(filename, contents, reportErrors); @@ -939,11 +966,12 @@ public Changes syncCompilationUnit(WFile f) { return Changes.empty(); } Set oldPackages = declaredPackages(f); + boolean oldJassNames = declaredJassNames(f); replaceCompilationUnit(f, contents, true); WLogger.debug("replaced file " + f); WurstGui gui = new WurstGuiLogger(); - doTypeCheckPartial(gui, ImmutableList.of(f), oldPackages); - return new Changes(io.vavr.collection.HashSet.of(f), oldPackages); + doTypeCheckPartial(gui, ImmutableList.of(f), oldPackages, oldJassNames); + return new Changes(io.vavr.collection.HashSet.of(f), oldPackages, oldJassNames); } private @Nullable String readCompilationUnitContents(WFile filename, boolean preferOpenBuffer) throws IOException { @@ -1116,7 +1144,8 @@ public void onCompilationResult(Consumer f) { onCompilationResultListeners.add(f); } - private void doTypeCheckPartial(WurstGui gui, List toCheckFilenames, Set oldPackages) { + private void doTypeCheckPartial(WurstGui gui, List toCheckFilenames, Set oldPackages, + boolean oldJassNames) { WLogger.debug("do typecheck partial of " + toCheckFilenames); WurstCompilerJassImpl comp = getCompiler(gui); List toCheck = getCompilationUnits(toCheckFilenames); @@ -1126,7 +1155,7 @@ private void doTypeCheckPartial(WurstGui gui, List toCheckFilenames, Set< return; } - Collection toCheckRec = calculateCUsToUpdate(toCheck, oldPackages, model2); + Collection toCheckRec = calculateCUsToUpdate(toCheck, oldPackages, oldJassNames, model2); partialTypecheck(model2, toCheckRec, gui, comp); } @@ -1144,14 +1173,11 @@ public void reconcile(Changes changes) { } } Set oldPackageNames = changes.getAffectedPackageNames().toJavaSet(); - Collection toCheckRec = calculateCUsToUpdate(toCheck1, oldPackageNames, model2); - boolean jassFileChanged = changes.getAffectedFiles().toJavaSet().stream() + // A removed or replaced unit is no longer in the model, so calculateCUsToUpdate cannot see its Jass names + // among the changed compilation units. + boolean jassNamesChanged = changes.isJassNamesChanged() || changes.getAffectedFiles().toJavaSet().stream() .anyMatch(file -> file.getUriString().endsWith(".j")); - if (jassFileChanged) { - // A removed Jass CU is no longer in the model, so calculateCUsToUpdate - // cannot see it among the changed compilation units. - toCheckRec.addAll(model2); - } + Collection toCheckRec = calculateCUsToUpdate(toCheck1, oldPackageNames, jassNamesChanged, model2); WurstGui gui = new WurstGuiLogger(); WurstCompilerJassImpl comp = getCompiler(gui); partialTypecheck(model2, toCheckRec, gui, comp); @@ -1204,23 +1230,18 @@ private Collection withUncheckedUnits(WurstModel model2, Collec * * @param changed the set of compilation units that were changed * @param oldPackages packages that were provided before the update (which might have been removed now) + * @param oldJassNames whether the units before the update (which might have been removed now) declared Jass names * @param model the complete AST * @return the set of compilation units that might be affected by the changes, including the changed compilation units */ - private Set calculateCUsToUpdate(Collection changed, Set oldPackages, WurstModel model) { + private Set calculateCUsToUpdate(Collection changed, Set oldPackages, + boolean oldJassNames, WurstModel model) { Set result = new TreeSet<>(Comparator.comparing(cu -> cu.getCuInfo().getFile())); result.addAll(changed); - boolean b = false; - for (CompilationUnit compilationUnit : changed) { - if (compilationUnit.getCuInfo().getFile().endsWith(".j")) { - b = true; - break; - } - } - if (b) { - // when plain Jass files are changed, everything must be checked again: + if (oldJassNames || changed.stream().anyMatch(ModelManagerImpl::declaresJassNames)) { + // Jass names are visible everywhere, so everything must be checked again: result.addAll(model); return result; } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ModelManagerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ModelManagerTests.java index 3dc87ab11..e56b58263 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ModelManagerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/ModelManagerTests.java @@ -1615,4 +1615,247 @@ public void sendProgress(String whatsRunningNow) { return checks.get(); } + /** + * A .wurst file may declare Jass functions, globals and natives outside of any package. Every package sees those + * names without an import (the model scope holds the Jass declarations of every unit), exactly like the names of a + * .j file, so a change to them has to check their users. + */ + @Test + public void editingJassDeclaredOutsideAPackageOfAWurstFileChecksItsUsers() throws IOException { + File projectFolder = new File("./temp/testProject_jassInWurstFileEdit/"); + File wurstFolder = new File(projectFolder, "wurst"); + newCleanFolder(wurstFolder); + + WFile fileWurst = WFile.create(new File(wurstFolder, "Wurst.wurst")); + WFile fileDefs = WFile.create(new File(wurstFolder, "Defs.wurst")); + WFile fileUser = WFile.create(new File(wurstFolder, "User.wurst")); + writeFile(fileWurst, "package Wurst\n"); + writeFile(fileDefs, string( + "function JassHelper takes nothing returns integer", + " return 1", + "endfunction", + "")); + writeFile(fileUser, string("package User", "init", " BJDebugMsg(I2S(JassHelper()))", "")); + + ModelManagerImpl manager = new ModelManagerImpl(projectFolder, new BufferManager()); + Map results = keepErrorsInMap(manager); + manager.buildProject(); + assertEquals(results.get(fileUser), "", "JassHelper is visible without an import"); + + String withoutHelper = string( + "function OtherHelper takes nothing returns integer", + " return 2", + "endfunction", + ""); + writeFile(fileDefs, withoutHelper); + ModelManager.Changes changes = manager.syncCompilationUnitContent(fileDefs, withoutHelper); + manager.reconcile(changes); + + // a build from scratch reports the call to JassHelper + assertThat(results.get(fileUser), containsString("severity = Error")); + assertTrue(manager.hasErrors(), "a run must not translate the call to what is gone"); + } + + /** The same when the Jass declarations go from a .wurst file which keeps no Jass declaration and no package. */ + @Test + public void removingTheJassDeclarationsOfAWurstFileChecksTheirUsers() throws IOException { + File projectFolder = new File("./temp/testProject_jassInWurstFileRemoved/"); + File wurstFolder = new File(projectFolder, "wurst"); + newCleanFolder(wurstFolder); + + WFile fileWurst = WFile.create(new File(wurstFolder, "Wurst.wurst")); + WFile fileDefs = WFile.create(new File(wurstFolder, "Defs.wurst")); + WFile fileUser = WFile.create(new File(wurstFolder, "User.wurst")); + writeFile(fileWurst, "package Wurst\n"); + writeFile(fileDefs, string( + "function JassHelper takes nothing returns integer", + " return 1", + "endfunction", + "")); + writeFile(fileUser, string("package User", "init", " BJDebugMsg(I2S(JassHelper()))", "")); + + ModelManagerImpl manager = new ModelManagerImpl(projectFolder, new BufferManager()); + Map results = keepErrorsInMap(manager); + manager.buildProject(); + assertEquals(results.get(fileUser), ""); + + writeFile(fileDefs, "\n"); + ModelManager.Changes changes = manager.syncCompilationUnitContent(fileDefs, "\n"); + manager.reconcile(changes); + + assertThat(results.get(fileUser), containsString("severity = Error")); + } + + @Test + public void deletingAWurstFileWithJassDeclarationsChecksTheirUsers() throws IOException { + File projectFolder = new File("./temp/testProject_jassInWurstFileDelete/"); + File wurstFolder = new File(projectFolder, "wurst"); + newCleanFolder(wurstFolder); + + WFile fileWurst = WFile.create(new File(wurstFolder, "Wurst.wurst")); + WFile fileDefs = WFile.create(new File(wurstFolder, "Defs.wurst")); + WFile fileUser = WFile.create(new File(wurstFolder, "User.wurst")); + writeFile(fileWurst, "package Wurst\n"); + writeFile(fileDefs, string( + "globals", + " integer jassG = 0", + "endglobals", + "")); + writeFile(fileUser, string("package User", "init", " jassG = 3", "")); + + ModelManagerImpl manager = new ModelManagerImpl(projectFolder, new BufferManager()); + Map results = keepErrorsInMap(manager); + manager.buildProject(); + assertEquals(results.get(fileUser), ""); + + assertTrue(new File(wurstFolder, "Defs.wurst").delete()); + ModelManager.Changes changes = manager.removeCompilationUnit(fileDefs); + assertFalse(manager.isFullyChecked(manager.getModel()), "User used what is gone"); + manager.reconcile(changes); + + assertThat(results.get(fileUser), containsString("severity = Error")); + } + + /** + * The language worker removes a deleted file at once and reconciles after its debounce, and serves a run in + * between. The run checks the whole model: it must find that B imports what is gone, and must not certify the + * model for the next run. + */ + @Test + public void aCompilationBeforeTheReconcileOfADeletedPackageReportsTheMissingImport() throws IOException { + File projectFolder = new File("./temp/testProject_compileBeforeReconcileDelete/"); + File wurstFolder = new File(projectFolder, "wurst"); + newCleanFolder(wurstFolder); + WFile fileWurst = WFile.create(new File(wurstFolder, "Wurst.wurst")); + WFile fileA = WFile.create(new File(wurstFolder, "A.wurst")); + WFile fileB = WFile.create(new File(wurstFolder, "B.wurst")); + writeFile(fileWurst, "package Wurst\n"); + writeFile(fileA, string("package A", "public function a() returns int", " return 4711", "")); + writeFile(fileB, string("package B", "import A", "init", " BJDebugMsg(I2S(a()))", "")); + ModelManagerImpl manager = new ModelManagerImpl(projectFolder, new BufferManager()); + manager.buildProject(); + + assertTrue(new File(wurstFolder, "A.wurst").delete()); + manager.removeCompilationUnit(fileA); + assertFalse(manager.isFullyChecked(manager.getModel()), "B imports what is gone"); + assertFalse(manager.hasErrors(), "nothing has reported B yet, so a run is not stopped before it checks"); + + WurstGui gui = new WurstGuiLogger(); + RunArgs runArgs = new RunArgs("-lua"); + WurstCompilerJassImpl compiler = new WurstCompilerJassImpl(new TimeTaker.Default(), projectFolder, gui, null, runArgs); + MapRequest.checkModel(manager, compiler, manager.getModel(), runArgs); + + assertThat(gui.getErrorList().toString(), containsString("Could not find imported package A")); + assertFalse(manager.isFullyChecked(manager.getModel()), "a check which found errors does not certify the model"); + } + + /** + * The same with a move, as an editor rename sends it: the old path is removed, the new one synced, which does not + * check at once. The run must compile what a build from scratch compiles. + */ + @Test + public void aCompilationBeforeTheReconcileOfAMovedPackageEmitsWhatAFreshBuildEmits() throws IOException { + File projectFolder = new File("./temp/testProject_compileBeforeReconcileMove/"); + File wurstFolder = new File(projectFolder, "wurst"); + newCleanFolder(wurstFolder); + newCleanFolder(new File(wurstFolder, "moved")); + WFile fileWurst = WFile.create(new File(wurstFolder, "Wurst.wurst")); + WFile fileA = WFile.create(new File(wurstFolder, "A.wurst")); + WFile fileMovedA = WFile.create(new File(wurstFolder, "moved/A.wurst")); + WFile fileB = WFile.create(new File(wurstFolder, "B.wurst")); + writeFile(fileWurst, "package Wurst\n"); + writeFile(fileA, string("package A", "public function a() returns int", " return 4711", "")); + writeFile(fileB, string("package B", "import A", "init", " BJDebugMsg(I2S(a()))", "")); + ModelManagerImpl manager = new ModelManagerImpl(projectFolder, new BufferManager()); + manager.buildProject(); + + String movedA = string("package A", "public function a() returns int", " return 4712", ""); + assertTrue(new File(wurstFolder, "A.wurst").delete()); + writeFile(fileMovedA, movedA); + manager.removeCompilationUnit(fileA); + manager.syncCompilationUnitContent(fileMovedA, movedA); + + String incremental = luaOf(projectFolder, manager); + + ModelManagerImpl fresh = new ModelManagerImpl(projectFolder, new BufferManager()); + fresh.buildProject(); + String expected = luaOf(projectFolder, fresh); + assertThat(expected, containsString("4712")); + assertEquals(incremental, expected, "the run compiled the binding B had before the move"); + } + + /** + * A unit added while every unit is unchecked is not reconciled with the others, as there is nothing to find which + * is not unchecked already. The packages of the model include it from then on all the same: a request which read + * them meanwhile must not leave a check of the whole model with the packages from before. + */ + @Test + public void aCompilationSeesAPackageAddedWhileEveryUnitIsUnchecked() throws IOException { + File projectFolder = new File("./temp/testProject_addWhileEveryUnitUnchecked/"); + File wurstFolder = new File(projectFolder, "wurst"); + newCleanFolder(wurstFolder); + newCleanFolder(new File(wurstFolder, "second")); + WFile fileWurst = WFile.create(new File(wurstFolder, "Wurst.wurst")); + WFile fileJass = WFile.create(new File(wurstFolder, "extra.j")); + WFile fileA = WFile.create(new File(wurstFolder, "A.wurst")); + WFile fileSecondA = WFile.create(new File(wurstFolder, "second/A.wurst")); + writeFile(fileWurst, "package Wurst\n"); + writeFile(fileJass, string("function F takes nothing returns integer", " return 1", "endfunction", "")); + writeFile(fileA, "package A\n"); + ModelManagerImpl manager = new ModelManagerImpl(projectFolder, new BufferManager()); + manager.buildProject(); + + // a change to Jass names leaves every unit unchecked + manager.syncCompilationUnitContent(fileJass, + string("function F takes nothing returns integer", " return 2", "endfunction", "")); + // a request reads the packages of the model meanwhile, as the resolution of an import does + assertNotNull(manager.getModel().attrPackages().get("A")); + + writeFile(fileSecondA, "package A\n"); + manager.syncCompilationUnitContent(fileSecondA, "package A\n"); + + WurstGui gui = new WurstGuiLogger(); + RunArgs runArgs = new RunArgs("-lua"); + WurstCompilerJassImpl compiler = new WurstCompilerJassImpl(new TimeTaker.Default(), projectFolder, gui, null, runArgs); + MapRequest.checkModel(manager, compiler, manager.getModel(), runArgs); + + assertThat(gui.getErrorList().toString(), containsString("Package 'A' is defined multiple times.")); + assertFalse(manager.isFullyChecked(manager.getModel()), "a check which found errors does not certify the model"); + } + + /** + * A compilation only marks the model checked when it used the manager's checks: after a compilation with the + * legacy Jass checks, which tolerate a Jass return type mismatch, the next strict compilation must not adopt it. + */ + @Test + public void aLegacyCompilationDoesNotCertifyTheModelForStrictCompilations() throws IOException { + File projectFolder = new File("./temp/testProject_legacyCheckDoesNotCertify/"); + File wurstFolder = new File(projectFolder, "wurst"); + newCleanFolder(wurstFolder); + WFile fileWurst = WFile.create(new File(wurstFolder, "Wurst.wurst")); + WFile fileJass = WFile.create(new File(wurstFolder, "extra.j")); + writeFile(fileWurst, "package Wurst\n"); + writeFile(fileJass, string("function F takes nothing returns integer", " return 1", "endfunction", "")); + ModelManagerImpl manager = new ModelManagerImpl(projectFolder, new BufferManager()); + manager.buildProject(); + assertTrue(manager.isFullyChecked(manager.getModel())); + + // a mismatch which the legacy checks downgrade to a warning; synced, not reconciled + manager.syncCompilationUnitContent(fileJass, + string("function F takes nothing returns integer", " return 1.5", "endfunction", "")); + + assertEquals(checkErrors(projectFolder, manager, "-legacyJassChecks"), 0, "the legacy checks tolerate it"); + assertFalse(manager.isFullyChecked(manager.getModel()), "a legacy check must not certify the model"); + assertTrue(checkErrors(projectFolder, manager) > 0, "a strict compilation still reports the mismatch"); + } + + /** The errors a compilation's check of the managed model reports. */ + private int checkErrors(File projectFolder, ModelManagerImpl manager, String... args) { + WurstGui gui = new WurstGuiLogger(); + RunArgs runArgs = new RunArgs(args); + WurstCompilerJassImpl compiler = new WurstCompilerJassImpl(new TimeTaker.Default(), projectFolder, gui, null, runArgs); + MapRequest.checkModel(manager, compiler, manager.getModel(), runArgs); + return gui.getErrorCount(); + } + } From db7d9c3e4b2928651a5fa700abbff6fd71dd744d Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 18:55:41 +0200 Subject: [PATCH 02/19] Tests: a partly checked model, Jass and Jurst files parsed ahead, the CLI check gate (cherry picked from commit ef10435588e8f340eb408441d8248dc4a5776d2a) --- .../languageserver/ParallelLoadTests.java | 20 ++++++++++++ .../languageserver/UpdateModelTests.java | 24 ++++++++++++++ .../wurstscript/tests/CliBuildMapTests.java | 31 +++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/ParallelLoadTests.java b/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/ParallelLoadTests.java index b583e01f6..92e644f3b 100644 --- a/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/ParallelLoadTests.java +++ b/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/ParallelLoadTests.java @@ -157,6 +157,26 @@ public void aParallelLoadBuildsTheModelOfTheSequentialLoad() throws IOException assertEquals(parallel.manager.parsesTakenAhead(), sources, "every file was parsed ahead"); } + /** Jass and Jurst files go through other parsers than .wurst files, on the parse threads as well. */ + @Test + public void jassAndJurstFilesOfTheProjectAreParsedAheadToTheSameUnits() throws IOException { + Path root = project(false); + write(root.resolve("wurst/war3map.j"), + "globals\n integer jassG = 0\nendglobals\nfunction JassF takes nothing returns integer\n return jassG\nendfunction\n"); + write(root.resolve("wurst/sub/extra.j"), "function ExtraF takes nothing returns integer\n return 2\nendfunction\n"); + write(root.resolve("wurst/sub/Legacy.jurst"), + "function JurstF takes nothing returns integer\n return ExtraF()\nendfunction\n" + + "package Legacy\nimport NoWurst\nimport P01\npublic function legacy() returns int\n return p01() + JurstF()\nend\nendpackage\n"); + Loaded sequential = new Loaded(root, 1); + Loaded parallel = new Loaded(root, 4); + + assertSameLoad(sequential, parallel); + assertTrue(sequential.diagnostics.isEmpty(), sequential.diagnostics.toString()); + assertTrue(sequential.files.stream().anyMatch(f -> f.endsWith("Legacy.jurst")), sequential.files.toString()); + assertEquals(parallel.manager.parsesTakenAhead(), parallel.units.size() - 2, + "every source but common.j and blizzard.j, the .j and .jurst files included, was parsed ahead"); + } + @Test public void aSyntaxErrorIsReportedTheSameWay() throws IOException { Path root = project(true); diff --git a/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/UpdateModelTests.java b/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/UpdateModelTests.java index 991ed45bd..ea7686394 100644 --- a/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/UpdateModelTests.java +++ b/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/UpdateModelTests.java @@ -58,4 +58,28 @@ public void aJassFileAddedAfterACheckMakesEveryUnitUncheckedAgain() throws IOExc assertEquals(manager.uncheckedCount(), manager.getModel().size(), "plain Jass names are visible everywhere: every unit has to be checked again"); } + + /** Only a model whose units are all unchecked may skip the reconciliation, not one which is partly checked. */ + @Test + public void aFileAddedWhileSomeUnitsAreUncheckedIsReconciled() throws IOException { + Path root = ParallelLoadTests.project(false); + ModelManagerImpl manager = new ModelManagerImpl(root.toFile(), new BufferManager()); + manager.buildProject(); + WFile p00 = WFile.create(root.resolve("wurst/P00.wurst").toFile()); + manager.syncCompilationUnitContent(p00, Files.readString(root.resolve("wurst/P00.wurst")) + "// edited\n"); + int unchecked = manager.uncheckedCount(); + assertTrue(unchecked > 0 && unchecked < manager.getModel().size(), "a partly checked model: " + unchecked); + + // a second definition of P15: the other P15 and everything importing it (P14 .. P00) must be checked again + Path dup = root.resolve("wurst/dup/P15.wurst"); + String text = "package P15\nimport NoWurst\npublic function p15() returns int\n return 0\n"; + Files.createDirectories(dup.getParent()); + Files.writeString(dup, text); + int before = manager.reconciliations(); + manager.syncCompilationUnitContent(WFile.create(dup.toFile()), text); + + assertEquals(manager.reconciliations(), before + 1, "a partly checked model reconciles a new unit"); + assertTrue(manager.uncheckedCount() >= 17, + "the new unit, the other P15 and P14 .. P00: " + manager.uncheckedCount()); + } } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CliBuildMapTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CliBuildMapTests.java index 01188630c..6c9c35020 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CliBuildMapTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CliBuildMapTests.java @@ -23,7 +23,9 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; +import static org.testng.Assert.expectThrows; public class CliBuildMapTests { @@ -124,6 +126,35 @@ public void extractingTheMapScriptDoesNotCheckTheModel() throws Exception { "no check has run against the unconfigured script: " + modelManager.getFirstErrorDescription()); } + /** + * The CLI loads the project without checking it, so the check with the map script is the only gate: a type error it + * finds fails the build before anything is compiled. + */ + @Test + public void theCliBuildFailsOnATypeErrorFoundByItsSingleCheck() throws Exception { + File projectFolder = new File("./temp/testProject_cli_single_check_error/"); + File wurstFolder = new File(projectFolder, "wurst"); + wurstFolder.mkdirs(); + Files.writeString(new File(wurstFolder, "Wurst.wurst").toPath(), "package Wurst\n"); + Files.writeString(new File(wurstFolder, "Main.wurst").toPath(), + "package Main\nfunction f() returns int\n return missingVariable\n"); + Files.writeString(new File(wurstFolder, "war3map.j").toPath(), + "function main takes nothing returns nothing\nendfunction\n"); + File configured = new File(projectFolder, "configured.j"); + Files.writeString(configured.toPath(), "function main takes nothing returns nothing\nendfunction\n"); + + CountingModelManager modelManager = new CountingModelManager(projectFolder); + modelManager.loadProject(); + assertFalse(modelManager.hasErrors(), "loading does not check"); + CapturingCliBuildMap request = new CapturingCliBuildMap(projectFolder); + + RuntimeException e = expectThrows(RuntimeException.class, () -> request.compile(modelManager, configured)); + + assertTrue(e.getMessage().contains("missingVariable"), e.getMessage()); + assertEquals(modelManager.checks, 1, "the model is checked once"); + assertNull(request.compiledModel, "nothing is compiled when the check finds errors"); + } + private static final String NAME_WARNING = "Function names should start with an lower case character."; /** Reported by the lexer, when the project is loaded: no check of the model reports it. */ private static final String INDENTATION_WARNING = "Use an even number of spaces for indentation."; From a8b423851af7d38fd85d731a38e21eaa7901ec1a Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 18:55:39 +0200 Subject: [PATCH 03/19] Tests: optimised old-generics int round trips on Lua (cherry picked from commit f6139e934ade695bd8f93499f4c5395dd4a09d55) --- .../tests/LuaBackendAuditTests.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 2921ed6a4..eaf4ecf8b 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -1394,6 +1394,62 @@ public void oldGenericsHandleRoundTripKeepsIdentityAndNull() throws IOException " testSuccess()"); } + /** {@link #oldGenericsIntRoundTripKeepsValuesAndNull} with the optimiser, which inlines the casts. */ + @Test + public void oldGenericsIntRoundTripKeepsValuesAndNullOptimized() throws IOException { + test().testLua(true).inline().localOptimizations().executeProg().lines( + "package Test", + "native testSuccess()", + "int array slots", + "class Store", + " function put(int key, T value)", + " slots[key] = value castTo int", + " function get(int key) returns T", + " return slots[key] castTo T", + " function isNull(int key) returns bool", + " return get(key) == null", + "class C", + " int v", + " construct(int v)", + " this.v = v", + "init", + " let ints = new Store()", + " ints.put(0, 0)", + " ints.put(1, -5)", + " ints.put(2, 123456)", + " let objs = new Store()", + " let c = new C(42)", + " objs.put(10, c)", + " objs.put(11, null)", + " if ints.get(0) == 0 and ints.get(1) == -5 and ints.get(2) == 123456 and objs.get(10) == c and objs.get(10).v == 42 and objs.get(11) == null and objs.isNull(11) and objs.isNull(12) and not objs.isNull(10)", + " testSuccess()"); + } + + /** {@link #oldGenericsIntZeroIsNotNull} with the optimiser, which inlines the casts. */ + @Test + public void oldGenericsIntZeroIsNotNullOptimized() throws IOException { + test().testLua(true).inline().localOptimizations().executeProg().lines( + "package Test", + "native testSuccess()", + "int array slots", + "interface Visit", + " function run(T t) returns int", + "class Store", + " function put(int key, T value)", + " slots[key] = value castTo int", + " function visit(int key, Visit v) returns int", + " return v.run(slots[key] castTo T)", + " function isNull(int key) returns bool", + " return (slots[key] castTo T) == null", + "init", + " let ints = new Store()", + " ints.put(0, 0)", + " ints.put(1, 3)", + " let total = ints.visit(0, x -> x + 10) + ints.visit(1, x -> x + 10)", + " if total == 23 and not ints.isNull(0) and ints.isNull(7)", + " testSuccess()"); + } + @Test public void legacyGenericHandleCastsUseObjectIndexMap() throws IOException { // The stdlib's TypeCasting functions, which the Lua backend maps to the object index; a From 9d2ac67024c544682631a2d959a0c2bf4bffd852 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 18:57:06 +0200 Subject: [PATCH 04/19] Lua: normalise an enum-bound old-generics value like an int (cherry picked from commit 62d7c22dc10fcbb2cfd18e031877c20b95aaa7ee) --- .../imtranslation/ExprTranslation.java | 25 ++++++++----------- .../tests/LuaBackendAuditTests.java | 22 ++++++++++++++++ 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java index 3dd0de101..51be30d60 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java @@ -114,21 +114,17 @@ static ImExpr wrapLua(Element trace, ImTranslator t, ImExpr translated, WurstTyp // normalization available to callers which explicitly cross an // external boundary; ordinary Wurst expressions must not pay for it. if (t.isLuaTarget() && actualType instanceof WurstTypeBoundTypeParam wtb) { - + WurstType base = wtb.getBaseType().normalize(); @Nullable ImFunction ensureType = null; - switch (wtb.getName()) { - case "integer": - ensureType = t.ensureIntFunc; - break; - case "string": - ensureType = t.ensureStrFunc; - break; - case "boolean": - ensureType = t.ensureBoolFunc; - break; - case "real": - ensureType = t.ensureRealFunc; - break; + // An enum is an int whose default is its first constant, so it is normalised like one. + if (base instanceof WurstTypeInt || base instanceof WurstTypeEnum) { + ensureType = t.ensureIntFunc; + } else if (base instanceof WurstTypeString) { + ensureType = t.ensureStrFunc; + } else if (base instanceof WurstTypeBool) { + ensureType = t.ensureBoolFunc; + } else if (base instanceof WurstTypeReal) { + ensureType = t.ensureRealFunc; } if(ensureType != null) { // Lua already has the exact cheap operation needed for the @@ -870,6 +866,7 @@ static boolean isCompositeExpectedTypeExpression(Expr e) { private static boolean isPrimitiveType(WurstType type) { WurstType normalized = type.normalize(); return normalized instanceof WurstTypeInt + || normalized instanceof WurstTypeEnum || normalized instanceof WurstTypeBool || normalized instanceof WurstTypeReal || normalized instanceof WurstTypeString; diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index eaf4ecf8b..0e32ffc4d 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -1450,6 +1450,28 @@ public void oldGenericsIntZeroIsNotNullOptimized() throws IOException { " testSuccess()"); } + /** An unset old-generics entry read as an enum is the enum's default, its first constant, as on Jass. */ + @Test + public void oldGenericEnumDefaultIsFirstConstant() { + test().testLua(true).luaOnly(false).executeProg().lines( + "package Test", + "native testSuccess()", + "int array slots", + "enum Color", + " RED", + " GREEN", + "class Store", + " function put(int key, T value)", + " slots[key] = value castTo int", + " function get(int key) returns T", + " return slots[key] castTo T", + "init", + " let s = new Store()", + " s.put(1, Color.GREEN)", + " if s.get(1) == Color.GREEN and s.get(7) == Color.RED", + " testSuccess()"); + } + @Test public void legacyGenericHandleCastsUseObjectIndexMap() throws IOException { // The stdlib's TypeCasting functions, which the Lua backend maps to the object index; a From ccb47f897fb0d164b2b9eaeeac58da0ad0b79509 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 18:59:53 +0200 Subject: [PATCH 05/19] Lua: cast an old-generics result as its type argument's type (cherry picked from commit f9895fce93a1ce56f2604fe898316acfb373a75c) --- .../imtranslation/ExprTranslation.java | 16 ++++- .../tests/LuaBackendAuditTests.java | 59 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java index 51be30d60..c0b3bdef4 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java @@ -551,9 +551,21 @@ private static int tupleSize(WurstType t) { } public static ImExpr translateIntern(ExprCast e, ImTranslator t, ImFunction f) { - ImExpr et = e.getExpr().imTranslateExpr(t, f); + Expr operand = e.getExpr(); + ImExpr et = operand.imTranslateExpr(t, f); + ImType fromType = et.attrTyp(); + if (fromType instanceof ImAnyType && operand.attrTypRaw() instanceof WurstTypeBoundTypeParam bound) { + // An old-generics result is erased in the IM, but here it has its type argument's + // type: the cast converts a value of that type, not the generic storage encoding. + // The value leaves erased storage here, so on Lua it gets that type's default. + ImType argumentType = bound.imTranslateType(t); + if (!(argumentType instanceof ImAnyType)) { + et = wrapLua(operand, t, et, bound); + fromType = argumentType; + } + } ImType toType = e.getTyp().attrTyp().imTranslateType(t); - return JassIm.ImCast(et, et.attrTyp(), toType); + return JassIm.ImCast(et, fromType, toType); } public static ImExpr translateIntern(FunctionCall e, ImTranslator t, ImFunction f) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 0e32ffc4d..598c149f0 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -1450,6 +1450,65 @@ public void oldGenericsIntZeroIsNotNullOptimized() throws IOException { " testSuccess()"); } + /** + * Outside the generic class an old-generics result has its type argument's type, so a cast of + * it is a cast of an enum or int value: 0 stays 0 and is not stored as the generic zero sentinel. + */ + @Test + public void oldGenericBoundEnumAndIntCastsKeepZero() throws IOException { + test().testLua(true).luaOnly(false).executeProg().lines( + "package Test", + "native testSuccess()", + "enum Color", + " RED", + " GREEN", + "class Box", + " T elem", + " construct(T e)", + " elem = e", + " function get() returns T", + " return elem", + "init", + " let colors = new Box(Color.RED)", + " let ints = new Box(0)", + " let ordinal = colors.get() castTo int", + " let asColor = ints.get() castTo Color", + " if ordinal == 0 and asColor == Color.RED", + " testSuccess()"); + String init = topLevelFunctionBodyWithPrefix(compiledLua("oldGenericBoundEnumAndIntCastsKeepZero"), "init_Test"); + assertFalse("a cast outside the generic class is not a generic storage cast:\n" + init, + init.contains("__wurst_oldGenericsZero")); + } + + /** Such a cast still reads a missing entry as 0, the default of the type argument, as on Jass. */ + @Test + public void oldGenericBoundCastOfMissingEntryIsZero() { + test().testLua(true).luaOnly(false).executeProg().lines( + "package Test", + "native testSuccess()", + "int array slots", + "enum Color", + " RED", + " GREEN", + "class Store", + " function put(int key, T value)", + " slots[key] = value castTo int", + " function get(int key) returns T", + " return slots[key] castTo T", + "init", + " let colors = new Store()", + " let ints = new Store()", + " colors.put(1, Color.GREEN)", + " ints.put(2, 0)", + " ints.put(3, 5)", + " let missingOrdinal = colors.get(7) castTo int", + " let missingColor = ints.get(7) castTo Color", + " let zeroColor = ints.get(2) castTo Color", + " let fiveColor = ints.get(3) castTo Color", + " if missingOrdinal == 0 and missingColor == Color.RED and colors.get(1) castTo int == 1 and zeroColor == Color.RED and fiveColor castTo int == 5", + " testSuccess()"); + } + /** An unset old-generics entry read as an enum is the enum's default, its first constant, as on Jass. */ @Test public void oldGenericEnumDefaultIsFirstConstant() { From 9936ae4fda0ed6de89406e4fbafec75e17d281cf Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 19:11:30 +0200 Subject: [PATCH 06/19] Optimiser tests: cover the incremental garbage rounds, the flatten skip and the liveness join (cherry picked from commit e852ff6fc6c3706fa500fdd8e75b77cc8da33c39) --- .../wurstscript/tests/OptimizerTests.java | 175 +++++++++++++++++- 1 file changed, 171 insertions(+), 4 deletions(-) diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 2e7990a6e..505764863 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -1967,6 +1967,64 @@ public void localMergerLiveness() throws IOException { } } + /** + * The liveness where the two branches of an if join: one branch's set holds the other's (first if: then {x,y}, + * else {x}), and two sets of which neither holds the other make a new one (second if: then {c,x,y,z}, else + * {c,w,x,y}). The key of an if in the result is its condition, which the control flow graph puts in its node. + */ + @Test + public void localMergerLivenessJoinsTheBranchesOfAnIf() { + Element trace = Ast.NoExpr(); + LocalMerger localMerger = new LocalMerger(); + ImVar y = JassIm.ImVar(trace, TypesHelper.imInt(), "y", false); + ImVar x = JassIm.ImVar(trace, TypesHelper.imInt(), "x", false); + ImVar z = JassIm.ImVar(trace, TypesHelper.imInt(), "z", false); + ImVar w = JassIm.ImVar(trace, TypesHelper.imInt(), "w", false); + ImVar c = JassIm.ImVar(trace, TypesHelper.imInt(), "c", false); + ImVar sinkA = JassIm.ImVar(trace, TypesHelper.imInt(), "sinkA", false); + ImVar sinkB = JassIm.ImVar(trace, TypesHelper.imInt(), "sinkB", false); + ImFunction sink = JassIm.ImFunction(trace, "sink", JassIm.ImTypeVars(), JassIm.ImVars(sinkA, sinkB), + JassIm.ImVoid(), JassIm.ImVars(), JassIm.ImStmts(), Collections.emptyList()); + CallType normal = CallType.NORMAL; + + ImSet setY = JassIm.ImSet(trace, JassIm.ImVarAccess(y), JassIm.ImIntVal(2)); + ImSet setX = JassIm.ImSet(trace, JassIm.ImVarAccess(x), JassIm.ImIntVal(1)); + ImSet setZ = JassIm.ImSet(trace, JassIm.ImVarAccess(z), JassIm.ImIntVal(3)); + ImSet setW = JassIm.ImSet(trace, JassIm.ImVarAccess(w), JassIm.ImIntVal(4)); + ImSet setC = JassIm.ImSet(trace, JassIm.ImVarAccess(c), JassIm.ImIntVal(0)); + ImExpr cond2 = JassIm.ImOperatorCall(de.peeeq.wurstscript.WurstOperator.EQ, + JassIm.ImExprs(JassIm.ImVarAccess(c), JassIm.ImIntVal(1))); + ImFunctionCall readZ = JassIm.ImFunctionCall(trace, sink, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImVarAccess(z), JassIm.ImVarAccess(z)), false, normal); + ImFunctionCall readW = JassIm.ImFunctionCall(trace, sink, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImVarAccess(w), JassIm.ImVarAccess(w)), false, normal); + ImIf second = JassIm.ImIf(trace, cond2, JassIm.ImStmts(readZ), JassIm.ImStmts(readW)); + ImExpr cond1 = JassIm.ImOperatorCall(de.peeeq.wurstscript.WurstOperator.EQ, + JassIm.ImExprs(JassIm.ImVarAccess(c), JassIm.ImIntVal(0))); + ImFunctionCall readXY = JassIm.ImFunctionCall(trace, sink, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImVarAccess(x), JassIm.ImVarAccess(y)), false, normal); + ImFunctionCall readX = JassIm.ImFunctionCall(trace, sink, JassIm.ImTypeArguments(), + JassIm.ImExprs(JassIm.ImVarAccess(x), JassIm.ImVarAccess(x)), false, normal); + ImIf first = JassIm.ImIf(trace, cond1, JassIm.ImStmts(readXY), JassIm.ImStmts(readX)); + ImFunction branching = JassIm.ImFunction(trace, "branching", JassIm.ImTypeVars(), JassIm.ImVars(), + JassIm.ImVoid(), JassIm.ImVars(y, x, z, w, c), + JassIm.ImStmts(setY, setX, setZ, setW, setC, second, first), Collections.emptyList()); + + Map> live = localMerger.calculateLiveness(branching); + + assertEquals(live.get(readXY), HashSet.empty()); + assertEquals(live.get(readX), HashSet.empty()); + assertEquals(live.get(cond1), HashSet.of(x, y)); // then {x,y} holds else {x} + assertEquals(live.get(readZ), HashSet.of(c, x, y)); + assertEquals(live.get(readW), HashSet.of(c, x, y)); + assertEquals(live.get(cond2), HashSet.of(c, w, x, y, z)); // {c,x,y,z} and {c,w,x,y} make a new set + assertEquals(live.get(setC), HashSet.of(c, w, x, y, z)); + assertEquals(live.get(setW), HashSet.of(w, x, y, z)); + assertEquals(live.get(setZ), HashSet.of(x, y, z)); + assertEquals(live.get(setX), HashSet.of(x, y)); + assertEquals(live.get(setY), HashSet.of(y)); + } + @Test public void localMergerKeepsImplicitEntryLocalSeparateFromParameter() { WurstModel model = Ast.WurstModel(); @@ -2064,10 +2122,10 @@ public void garbageRemovalRemovesAChainOfAssignmentsInsideOneFunction() { // `v1 = v0; v2 = v1; ...` where nothing reads the last: each round makes the variable before it unread, so a // chain takes a round for each link. The removal goes on until the first link is gone, however long it is: it // does not stop after some rounds and leave the rest, and it does not take a converging program for one which - // never settles. + // never settles. (In unit-test mode, so the rounds are checked against an analysis of the whole program.) int links = 300; WurstModel model = Ast.WurstModel(); - ImTranslator translator = new ImTranslator(model, false, new RunArgs()); + ImTranslator translator = new ImTranslator(model, true, new RunArgs()); ImProg prog = translator.getImProg(); ImVars locals = JassIm.ImVars(); ImStmts body = JassIm.ImStmts(); @@ -2100,7 +2158,7 @@ public void garbageRemovalRemovesAChainOfAssignmentsInFunctionsOfTheirOwn() { // function of the link after it has lost its read. int links = 300; WurstModel model = Ast.WurstModel(); - ImTranslator translator = new ImTranslator(model, false, new RunArgs()); + ImTranslator translator = new ImTranslator(model, true, new RunArgs()); ImProg prog = translator.getImProg(); List globals = new ArrayList<>(); for (int i = 0; i <= links; i++) { @@ -2206,7 +2264,7 @@ public void garbageRemovalKeepsTheVariableWhichFlatteningAnEffectMakes() { // second argument come before the call: the first one, which has an effect, is saved in a variable of the // function first. That variable is declared, and the removal does not take it for one nothing reads. WurstModel model = Ast.WurstModel(); - ImTranslator translator = new ImTranslator(model, false, new RunArgs()); + ImTranslator translator = new ImTranslator(model, true, new RunArgs()); ImProg prog = translator.getImProg(); ImVar a = JassIm.ImVar(model, TypesHelper.imInt(), "a", false); ImVar b = JassIm.ImVar(model, TypesHelper.imInt(), "b", false); @@ -2258,6 +2316,55 @@ public void visit(ImVarAccess e) { assertTrue(translator.isFlat(), "what is left is flat: " + main.getBody()); } + /** + * A global whose name a variable event refers to stays, and so does the assignment to it, when its last read in a + * function goes in a later round of the removal (`copy = myVar` goes in the first). + */ + @Test + public void garbageRemovalKeepsAPreservedGlobalWhoseLastReadGoesInALaterRound() throws IOException { + test().lines( + "type trigger extends handle", + "type event extends handle", + "type limitop extends handle", + "package test", + " int myVar = 0", + " @extern native TriggerRegisterVariableEvent(trigger whichTrigger, string varName, limitop opcode, real limitval) returns event", + " function copyIt()", + " int copy = myVar", + " init", + " TriggerRegisterVariableEvent(null, \"test_myVar\", null, 0.0)", + " copyIt()", + " myVar = 5", + "endpackage"); + String out = Files.toString(new File( + "test-output/OptimizerTests_garbageRemovalKeepsAPreservedGlobalWhoseLastReadGoesInALaterRound_no_opts.j"), + Charsets.UTF_8); + assertTrue(out.contains("integer test_myVar") && out.contains("set test_myVar = 5"), out); + } + + /** + * A global of blizzard.j whose last read goes in a later round of the removal makes it analyse the program again, + * so what its initial value reads (bj_PI) is not kept for it. The unit-test cross-check at the end of the removal + * compares the rounds with an analysis of the whole program. + */ + @Test + public void garbageRemovalAnalysesAgainWhenABlizzardGlobalLosesItsLastRead() { + test().executeProg(true).compilationUnits( + compilationUnit("blizzard.j", + "globals", + " constant real bj_PI = 3.14159", + " constant real bj_DEGTORAD = bj_PI/180.0", + "endglobals"), + compilationUnit("test.wurst", + "package Test", + "native testSuccess()", + "function copyIt()", + " real copy = bj_DEGTORAD", + "init", + " copyIt()", + " testSuccess()")); + } + @Test public void aFlattenLeavesTheFunctionsWhichWereNotModifiedSinceTheLastOne() { WurstModel model = Ast.WurstModel(); @@ -2303,6 +2410,66 @@ public void aFlattenLeavesTheFunctionsWhichWereNotModifiedSinceTheLastOne() { assertTrue(translator.isFlat(), "what was added is flat now: " + changed.getBody()); } + /** + * The same in unit-test mode (the functions a flatten leaves are checked) for a function of a class, changed + * through a setter below an if and two lists, a replacement whose parent is not a list, and a transfer into an + * empty block (the inliner's), each followed by a flatten. + */ + @Test + public void aFlattenFlattensAgainAClassFunctionChangedDeepInsideItsBody() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, true, new RunArgs()); + ImProg prog = translator.getImProg(); + CallType normal = CallType.NORMAL; + ImFunction tock = JassIm.ImFunction(model, "tock", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImVoid(), + JassIm.ImVars(), JassIm.ImStmts(), Collections.singletonList(FunctionFlagEnum.IS_NATIVE)); + ImVar local = JassIm.ImVar(model, TypesHelper.imInt(), "local", false); + ImFunction method = JassIm.ImFunction(model, "method", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImVoid(), + JassIm.ImVars(local), + JassIm.ImStmts(JassIm.ImIf(model, JassIm.ImBoolVal(true), + JassIm.ImStmts(JassIm.ImSet(model, JassIm.ImVarAccess(local), JassIm.ImIntVal(1))), JassIm.ImStmts())), + Collections.emptyList()); + ImClass c = JassIm.ImClass(model, "C", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImMethods(), + JassIm.ImFunctions(method), new ArrayList<>()); + prog.getFunctions().add(tock); + prog.getClasses().add(c); + + prog.flatten(translator); + ImStmts tockBody = tock.getBody(); + + // 1. a setter below an if and two lists + ImIf theIf = (ImIf) method.getBody().get(0); + int seen = method.modificationCount(); + ((ImSet) theIf.getThenBlock().get(0)).setRight(JassIm.ImStatementExpr(JassIm.ImStmts( + JassIm.ImFunctionCall(model, tock, JassIm.ImTypeArguments(), JassIm.ImExprs(), false, normal)), + JassIm.ImIntVal(2))); + assertNotEquals(method.modificationCount(), seen, "a setter deep in the body counts for the function"); + prog.flatten(translator); + assertTrue(translator.isFlat(), "after the setter: " + method.getBody()); + assertSame(tock.getBody(), tockBody, "the function which was not modified is left alone"); + + // 2. a replacement whose parent is not a list (replaceBy falls back to set(i, ...)) + theIf = (ImIf) method.getBody().get(0); + ImSet last = (ImSet) theIf.getThenBlock().get(theIf.getThenBlock().size() - 1); + last.getRight().replaceBy(JassIm.ImStatementExpr(JassIm.ImStmts( + JassIm.ImFunctionCall(model, tock, JassIm.ImTypeArguments(), JassIm.ImExprs(), false, normal)), + JassIm.ImIntVal(3))); + prog.flatten(translator); + assertTrue(translator.isFlat(), "after replaceBy: " + method.getBody()); + + // 3. a transfer into an empty block (addAllMoved takes the source's array) + theIf = (ImIf) method.getBody().get(0); + ImStmts moved = JassIm.ImStmts(JassIm.ImSet(model, JassIm.ImVarAccess(local), JassIm.ImStatementExpr( + JassIm.ImStmts(JassIm.ImFunctionCall(model, tock, JassIm.ImTypeArguments(), JassIm.ImExprs(), false, normal)), + JassIm.ImIntVal(4)))); + seen = method.modificationCount(); + theIf.getElseBlock().addAllMoved(moved); + assertNotEquals(method.modificationCount(), seen, "a transfer into the function counts for it"); + prog.flatten(translator); + assertTrue(translator.isFlat(), "after addAllMoved: " + method.getBody()); + assertSame(tock.getBody(), tockBody, "still left alone"); + } + @Test public void luaArithmeticHelperRetryRespectsFunctionLocalBudget() { WurstModel model = Ast.WurstModel(); From 7023e05d648a78e78a1c076359bbf4f6d513e1e3 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 19:15:41 +0200 Subject: [PATCH 07/19] Optimiser: keep an unread integer division which may stop the thread (cherry picked from commit 95f58f9d4caa77541153636703e4affc355d1fef) --- .../optimizer/LocalMerger.java | 5 +- .../translation/imoptimizer/ImOptimizer.java | 39 +++++++++------- .../translation/imtranslation/Flatten.java | 34 ++++++++++++++ .../wurstscript/tests/OptimizerTests.java | 46 +++++++++++++++++++ 4 files changed, 106 insertions(+), 18 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java index fe2ac57b6..11c7322c1 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java @@ -3,6 +3,7 @@ import de.peeeq.datastructures.GraphInterpreter; import de.peeeq.wurstscript.intermediatelang.optimizer.ControlFlowGraph.Node; import de.peeeq.wurstscript.jassIm.*; +import de.peeeq.wurstscript.translation.imtranslation.Flatten; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; import de.peeeq.wurstscript.types.TypesHelper; @@ -292,7 +293,8 @@ private void eliminateDeadCode(Map> livenessInfo) { if (v == null || v.isGlobal()) continue; - if (!livenessInfo.get(s).contains(v)) { + if (!livenessInfo.get(s).contains(v) && !Flatten.mayStopTheThread(set.getRight())) { + // (an assignment of a division which may stop the thread is the statement which evaluates it) final List raw = new ArrayList<>(); collectLhsSideEffects(lhs, raw); if (hasSideEffects(set.getRight())) raw.add(set.getRight()); @@ -341,6 +343,7 @@ private boolean hasSideEffects(Element e) { if (e instanceof ImMethodCall) return true; if (e instanceof ImFunctionCall call && (translator == null || !translator.isTrapFreeLuaIntrinsicCall(call))) return true; + if (e instanceof ImExpr expr && Flatten.mayStopTheThread(expr)) return true; for (int i = 0; i < e.size(); i++) if (hasSideEffects(e.get(i))) return true; return false; } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java index 55cdfa657..77bb2815d 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java @@ -2,7 +2,6 @@ import com.google.common.collect.Lists; import de.peeeq.wurstio.TimeTaker; -import de.peeeq.wurstscript.WurstOperator; import de.peeeq.wurstscript.WLogger; import de.peeeq.wurstscript.intermediatelang.optimizer.BranchMerger; import de.peeeq.wurstscript.intermediatelang.optimizer.ConstantAndCopyPropagation; @@ -13,6 +12,7 @@ import de.peeeq.wurstscript.intermediatelang.optimizer.SideEffectAnalyzer; import de.peeeq.wurstscript.intermediatelang.optimizer.SimpleRewrites; import de.peeeq.wurstscript.jassIm.*; +import de.peeeq.wurstscript.translation.imtranslation.Flatten; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; import de.peeeq.wurstscript.types.TypesHelper; @@ -164,7 +164,8 @@ private boolean removeGarbageWithFacts() { /** * How many things the first round could remove: the global variables, fields, functions, locals and assignments * which the program had. Every round which changes something removes one of them (an assignment which is replaced - * leaves statements which are not assignments to an unread variable: the variables a flatten makes are read), so + * leaves statements which are not assignments to an unread variable which go: the variables a flatten makes are + * read, except the local of a division which may stop the thread, and an assignment of that to a local stays), so * the rounds cannot be more than that. A program which needs more never settles, which is a bug, and is reported. * The real programs measured take up to ten rounds (castle fight: nine). -1 before the first round counted them. */ @@ -329,6 +330,8 @@ private boolean garbageRound(boolean incremental, @Nullable List newlyUnr } // remove set statements to unread variables final List>> replacements = Lists.newArrayList(); + // the unread locals of the assignments which stay because they evaluate a division which may stop the thread + final Set keptLocals = new LinkedHashSet<>(); if (countThings) { things += trans.setStatementsOf(f).size() + f.getLocals().size(); } @@ -336,8 +339,14 @@ private boolean garbageRound(boolean incremental, @Nullable List newlyUnr if (e.getLeft() instanceof ImVarAccess) { ImVarAccess va = (ImVarAccess) e.getLeft(); if (!readVars.contains(va.getVar()) && !NamePreservation.isPreserved(va.getVar())) { - List sideEffects = collectSideEffects(e.getRight(), sideEffectAnalyzer); - replacements.add(Pair.create(e, sideEffects)); + if (va.getVar().getParent() == f.getLocals() && Flatten.mayStopTheThread(e.getRight())) { + // What it does besides assigning is the division, and the statement which evaluates + // that is an assignment to a local (a flatten makes one): this one. + keptLocals.add(va.getVar()); + } else { + List sideEffects = collectSideEffects(e.getRight(), sideEffectAnalyzer); + replacements.add(Pair.create(e, sideEffects)); + } } } else if (e.getLeft() instanceof ImVarArrayAccess) { ImVarArrayAccess va = (ImVarArrayAccess) e.getLeft(); @@ -362,7 +371,9 @@ private boolean garbageRound(boolean incremental, @Nullable List newlyUnr // make locals of their own (a flatten saves the arguments in front of one which has statements): the // variables which were read when the round was analysed are the ones to keep, those are not among them. if (!incremental || changedBefore) { - changes |= f.getLocals().retainAll(readVars); + changes |= keptLocals.isEmpty() + ? f.getLocals().retainAll(readVars) + : f.getLocals().removeIf(v -> !readVars.contains(v) && !keptLocals.contains(v)); } if (!replacements.isEmpty()) { @@ -393,8 +404,9 @@ private void replaceByEffects(ImFunction f, List>> rep throw new IllegalStateException("An assignment which is not in a statement list: " + pair.getA()); } // The effects are parts of the assignment, which goes, and they are statements now: an expression which is - // one (`a and f()`, a call with a statement expression for an argument) becomes what a flatten makes of it, - // the statements which the backends translate (an if, the call with its arguments in variables). + // one (`a and f()`, a call with a statement expression for an argument, `10 div d`) becomes what a flatten + // makes of it, the statements which the backends translate (an if, the call with its arguments in + // variables, the assignment of the division to a local). List statements = new ArrayList<>(pair.getB().size()); for (ImExpr effect : pair.getB()) { effect.setParent(null); @@ -475,16 +487,9 @@ private boolean mayTrapAtRuntime(Element elem, Map function } } - if (elem instanceof ImOperatorCall opCall) { - WurstOperator op = opCall.getOp(); - if ((op == WurstOperator.DIV_INT || op == WurstOperator.MOD_INT || op == WurstOperator.JASS_MOD_INT) - && opCall.getArguments().size() >= 2) { - ImExpr denominator = opCall.getArguments().get(1); - // Preserve integer div/mod unless denominator is provably non-zero. - if (!(denominator instanceof ImIntVal imIntVal) || imIntVal.getValI() == 0) { - return true; - } - } + // Preserve integer div/mod unless the divisor is provably non-zero. + if (elem instanceof ImExpr expr && Flatten.mayStopTheThread(expr)) { + return true; } for (int i = 0; i < elem.size(); i++) { Element child = elem.get(i); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/Flatten.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/Flatten.java index bf0575de6..97f40e138 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/Flatten.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/Flatten.java @@ -232,6 +232,31 @@ public List getLExprs() { } + /** + * Whether evaluating {@code e} itself can stop the thread: an integer division or modulo by a divisor which is not a + * nonzero constant. A division by zero stops the thread on Jass ({@code I2S(1 div 0)} is the deliberate abort) and + * raises an error on Lua, so such an operator is evaluated where nothing uses its value. As a statement it is the + * assignment of its value to a local which nothing reads ({@link #exprToStatements}), and the removals of + * assignments to unread variables keep an assignment of it to a local: no smaller statement evaluates it. + */ + public static boolean mayStopTheThread(ImExpr e) { + if (!(e instanceof ImOperatorCall opCall)) { + return false; + } + WurstOperator op = opCall.getOp(); + if ((op != WurstOperator.DIV_INT && op != WurstOperator.MOD_INT && op != WurstOperator.JASS_MOD_INT) + || opCall.getArguments().size() < 2) { + return false; + } + ImExpr divisor = opCall.getArguments().get(1); + return !(divisor instanceof ImIntVal value) || value.getValI() == 0; + } + + /** + * Puts the effects of evaluating {@code e} into {@code result} as statements: the calls, deallocations and method + * calls, an and/or whose right side has statements as an if, and an operator which may stop the thread as an + * assignment. The rest of an expression only produces a value, and it goes. + */ private static void exprToStatements(List result, Element e, ImTranslator t, ImFunction f) { if (e instanceof ImFunctionCall imFunctionCall) { Result res = flatten(imFunctionCall, t, f); @@ -248,6 +273,15 @@ private static void exprToStatements(List result, Element e, ImTranslato } else if (e instanceof ImStatementExpr e2) { flattenStatementsInto(result, e2.getStatements(), t, f); exprToStatements(result, e2, t, f); + } else if (e instanceof ImOperatorCall oc && mayStopTheThread(oc)) { + // Jass and Lua have no expression statement, so the value goes in a local which nothing reads. + ImType type = oc.attrTyp(); + de.peeeq.wurstscript.ast.Element trace = oc.attrTrace(); + Result res = flatten(oc, t, f); + result.addAll(res.stmts); + ImVar tempVar = JassIm.ImVar(trace, type, getTempVarName(), false); + f.getLocals().add(tempVar); + result.add(ImSet(trace, ImVarAccess(tempVar), res.expr)); } else if (e instanceof ImOperatorCall oc && (oc.getOp() == WurstOperator.AND || oc.getOp() == WurstOperator.OR)) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 505764863..1d4580b10 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -2316,6 +2316,52 @@ public void visit(ImVarAccess e) { assertTrue(translator.isFlat(), "what is left is flat: " + main.getBody()); } + /** + * An integer division by a divisor which may be zero stops the thread, so the removal of an assignment to a + * variable nothing reads keeps the division, without and with the optimisations, on Jass and on Lua. + */ + @Test + public void garbageRemovalKeepsAnUnreadDivisionWhichMayStopTheThread() throws IOException { + String[] program = {"package Test", "native testSuccess()", "int zero = 0", + "function f(int d)", " int unused = 10 div d", "init", " f(zero)", " testSuccess()"}; + test().executeProg(false).lines(program); + for (String variant : new String[]{"no_opts", "opt", "inl", "inlopt"}) { + String out = Files.toString(new File("test-output/OptimizerTests_garbageRemovalKeepsAnUnreadDivisionWhichMayStopTheThread_" + + variant + ".j"), Charsets.UTF_8); + assertTrue(out.contains("10 / "), variant + ": the division which may stop the thread was dropped:\n" + out); + } + for (boolean optimised : new boolean[]{false, true}) { + TestConfig lua = test().testLua(true).executeProg(false); + if (optimised) { + lua = lua.inline().localOptimizations(); + } + lua.lines(program); + String out = Files.toString(new File("test-output/lua/OptimizerTests_garbageRemovalKeepsAnUnreadDivisionWhichMayStopTheThread.lua"), + Charsets.UTF_8); + assertTrue(out.contains("10 //") || out.contains("__wurst_intDiv(10,"), + "Lua" + (optimised ? " with the optimisations" : "") + ": the division which may stop the thread was dropped:\n" + out); + } + } + + /** The same for a division inside an operator, and for one assigned to a global, a field and an array element. */ + @Test + public void garbageRemovalKeepsUnreadDivisionsInsideExpressionsAndOfOtherVariables() throws IOException { + String[] program = {"package Test", "native testSuccess()", "int zero = 0", "int unreadGlobal", + "int array unreadArray", "class C", " int unreadField", + "function g() returns int", " return 3", + "function f(int d)", " int unused = 11 div d + g()", " unreadGlobal = 12 mod d", " unreadArray[2] = 13 div d", + " C c = new C", " c.unreadField = 14 div d", " if 15 div d == 0", " skip", + "init", " f(zero)", " testSuccess()"}; + test().executeProg(false).lines(program); + for (String variant : new String[]{"no_opts", "opt", "inl", "inlopt"}) { + String out = Files.toString(new File("test-output/OptimizerTests_garbageRemovalKeepsUnreadDivisionsInsideExpressionsAndOfOtherVariables_" + + variant + ".j"), Charsets.UTF_8); + for (String division : new String[]{"11 / ", "ModuloInteger(12, ", "13 / ", "14 / ", "15 / "}) { + assertTrue(out.contains(division), variant + ": the division " + division + "was dropped:\n" + out); + } + } + } + /** * A global whose name a variable event refers to stays, and so does the assignment to it, when its last read in a * function goes in a later round of the removal (`copy = myVar` goes in the first). From 37cbcb2044a435a6c1ded557d8f07a5c3decfe8d Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 19:18:59 +0200 Subject: [PATCH 08/19] Local merger: remove the code which no path reaches with the assignments only it reads (cherry picked from commit 1f06d76c926fd6f5e468a86c57d2da35f9acb543) --- .../optimizer/ControlFlowGraph.java | 51 ++++++++++++++++ .../optimizer/LocalMerger.java | 16 ++++- .../wurstscript/tests/OptimizerTests.java | 60 +++++++++++++++++++ 3 files changed, 125 insertions(+), 2 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ControlFlowGraph.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ControlFlowGraph.java index 0da92bf73..296826ef5 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ControlFlowGraph.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/ControlFlowGraph.java @@ -4,8 +4,11 @@ import de.peeeq.wurstscript.jassIm.*; import it.unimi.dsi.fastutil.objects.ObjectArrayList; import it.unimi.dsi.fastutil.objects.Reference2ObjectOpenHashMap; +import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet; import org.eclipse.jdt.annotation.Nullable; +import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.List; public class ControlFlowGraph { @@ -36,13 +39,61 @@ public static final class Node { private final Reference2ObjectOpenHashMap loopEnd = new Reference2ObjectOpenHashMap<>(); private final Reference2ObjectOpenHashMap varargLoopEnd = new Reference2ObjectOpenHashMap<>(); private final ObjectArrayList nodeList = new ObjectArrayList<>(); + private final ImStmts body; public ControlFlowGraph(ImStmts stmts) { + this.body = stmts; // a light hint helps the first growth step avoid rehash nodes.trim(0); buildCfg(stmts); } + /** + * The statements which no path from the first statement reaches, without the statements inside them: what follows + * a return, a loop which has no exitwhen of its own, or an if neither branch of which comes to its end. + */ + public List unreachableStatements() { + if (body.isEmpty()) { + return List.of(); + } + ReferenceOpenHashSet reached = new ReferenceOpenHashSet<>(nodeList.size()); + ArrayDeque work = new ArrayDeque<>(); + Node entry = getNode(body.get(0)); + reached.add(entry); + work.add(entry); + while (!work.isEmpty()) { + ObjectArrayList successors = work.poll().successors; + for (int i = 0; i < successors.size(); i++) { + Node successor = successors.get(i); + if (reached.add(successor)) { + work.add(successor); + } + } + } + if (reached.size() == nodeList.size()) { + return List.of(); + } + List result = new ArrayList<>(); + collectUnreachable(body, reached, result); + return result; + } + + private void collectUnreachable(ImStmts stmts, ReferenceOpenHashSet reached, List result) { + for (int i = 0; i < stmts.size(); i++) { + ImStmt s = stmts.get(i); + if (!reached.contains(nodes.get(s))) { + result.add(s); + } else if (s instanceof ImIf imIf) { + collectUnreachable(imIf.getThenBlock(), reached, result); + collectUnreachable(imIf.getElseBlock(), reached, result); + } else if (s instanceof ImLoop imLoop) { + collectUnreachable(imLoop.getBody(), reached, result); + } else if (s instanceof ImVarargLoop imVarargLoop) { + collectUnreachable(imVarargLoop.getBody(), reached, result); + } + } + } + private void buildCfg(ImStmts stmts) { final int n = stmts.size(); for (int i = 0; i < n; i++) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java index 11c7322c1..388d8a53d 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java @@ -47,6 +47,16 @@ private void optimizeFunctions(List functions) { void optimizeFunc(ImFunction func) { LivenessAnalysis liveness = analyzeLiveness(func); + // The liveness is that of the code which a path reaches: a local which only code no path reaches reads (after + // a loop which nothing leaves, say) is live nowhere, and eliminateDeadCode removes the assignments to it. That + // code goes too, so that no read is left with no assignment before it, which pjass rejects. + List unreachable = liveness.cfg.unreachableStatements(); + if (!unreachable.isEmpty()) { + for (ImStmt s : unreachable) { + AstEdits.deleteStmt(s); + } + liveness = analyzeLiveness(func); + } Map> livenessInfo = liveness.liveOut; eliminateDeadCode(livenessInfo); mergeLocals(livenessInfo, liveness.liveAtEntry, func); @@ -503,7 +513,7 @@ protected Collection getIncidentNodes(Node t) { Set liveAtEntry = N == 0 ? io.vavr.collection.HashSet.empty() : variables.toSet(in[0], converted); - return new LivenessAnalysis(result, liveAtEntry); + return new LivenessAnalysis(cfg, result, liveAtEntry); } private static final int[] NO_VARIABLES = new int[0]; @@ -624,10 +634,12 @@ private static int[] difference(int[] a, int[] removed) { } private static final class LivenessAnalysis { + private final ControlFlowGraph cfg; private final Map> liveOut; private final Set liveAtEntry; - private LivenessAnalysis(Map> liveOut, Set liveAtEntry) { + private LivenessAnalysis(ControlFlowGraph cfg, Map> liveOut, Set liveAtEntry) { + this.cfg = cfg; this.liveOut = liveOut; this.liveAtEntry = liveAtEntry; } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 1d4580b10..d9b9a351f 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -2411,6 +2411,66 @@ public void garbageRemovalAnalysesAgainWhenABlizzardGlobalLosesItsLastRead() { " testSuccess()")); } + /** + * The only read of `a` is after a loop which always returns, so it is never reached. The optimisations do not + * leave that read with no assignment before it, which pjass rejects. + */ + @Test + public void localOptimizationsLeaveNoUninitialisedReadAfterAReturningLoop() { + test().executeProg().lines("package test", "native testSuccess()", "int trace = 0", + "@noinline function side(int k) returns int", " trace = trace * 5 + k", " return trace mod 7", + "@noinline function f(int x) returns int", " int a = x", " for i1 = 0 to 0", " a = x + 1", + " for i2 = 0 to 3", " return side(i2)", " return a", + "init", " if f(1) != 12345", " testSuccess()"); + } + + /** + * The same after an if both branches of which return: the local merger removes the assignment to `a`, whose only + * read no path reaches, and that read with it. (`x` is read after the assignment, so `a` cannot share its slot, + * which would have hidden the read with no assignment before it.) + */ + @Test + public void localMergerRemovesTheCodeAfterAnIfBothBranchesOfWhichReturn() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, new RunArgs()); + ImProg prog = translator.getImProg(); + ImVar x = JassIm.ImVar(model, TypesHelper.imInt(), "x", false); + ImVar a = JassIm.ImVar(model, TypesHelper.imInt(), "a", false); + ImIf bothReturn = JassIm.ImIf(model, JassIm.ImOperatorCall(de.peeeq.wurstscript.WurstOperator.GREATER, + JassIm.ImExprs(JassIm.ImVarAccess(x), JassIm.ImIntVal(0))), + JassIm.ImStmts(JassIm.ImReturn(model, JassIm.ImIntVal(1))), + JassIm.ImStmts(JassIm.ImReturn(model, JassIm.ImIntVal(2)))); + ImFunction f = JassIm.ImFunction(model, "f", JassIm.ImTypeVars(), JassIm.ImVars(x), TypesHelper.imInt(), + JassIm.ImVars(a), + JassIm.ImStmts(JassIm.ImSet(model, JassIm.ImVarAccess(a), JassIm.ImVarAccess(x)), bothReturn, + JassIm.ImReturn(model, JassIm.ImVarAccess(a))), + Collections.emptyList()); + prog.getFunctions().add(f); + + new LocalMerger().optimize(translator, new LocalPlayerContextAnalyzer(prog)); + + List assigned = new ArrayList<>(); + List read = new ArrayList<>(); + f.accept(new ImFunction.DefaultVisitor() { + @Override + public void visit(ImSet set) { + set.getRight().accept(this); + assigned.add(((ImVarAccess) set.getLeft()).getVar()); + } + + @Override + public void visit(ImVarAccess access) { + read.add(access.getVar()); + } + }); + for (ImVar v : read) { + assertTrue(v == x || assigned.contains(v), "a read of " + v.getName() + " with no assignment: " + f.getBody()); + } + assertEquals(f.getBody().size(), 1, "only the if is left: " + f.getBody()); + assertSame(f.getBody().get(0), bothReturn); + assertTrue(f.getLocals().isEmpty(), "the local nothing reads any more: " + f.getLocals()); + } + @Test public void aFlattenLeavesTheFunctionsWhichWereNotModifiedSinceTheLastOne() { WurstModel model = Ast.WurstModel(); From 0378f0d11caefcf4c3f29b5fa9076a109e6102e3 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 19:23:15 +0200 Subject: [PATCH 09/19] Deterministic checks: FSM siblings and same-named classes in several packages (cherry picked from commit 9c1ac52b2a65efe7e89ff383543bf0ae5074d916) --- .../tests/DeterministicChecks.java | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterministicChecks.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterministicChecks.java index 8cf2546df..261bfa17c 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterministicChecks.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterministicChecks.java @@ -621,4 +621,139 @@ private static int countOccurrences(String text, String part) { return n; } + /** Compiles {@code units} to Lua, runs them, and returns the script, which is written under {@code name}. */ + private String compileToLua(String name, List units) throws IOException { + testNamed(name).testLua(true).executeProg().compilationUnits(units.toArray(new CU[0])); + return Files.toString(new File("test-output/lua/DeterministicChecks_" + name + ".lua"), Charsets.UTF_8); + } + + /** + * The FSM of AGENTS.md section 8 with each sibling state in a package of its own: every sibling binds the root + * slot FSM.update calls to its own update, never to NoOpState's, and the script is the same with the + * compilation units in the reverse order. + */ + @Test + public void fsmSiblingsInSeparatePackagesBindRootSlotInAnyUnitOrder() throws IOException { + List units = new ArrayList<>(); + units.add(compilationUnit("FsmLib.wurst", + "package FsmLib", + "public abstract class State", + " function enter(T owner)", + " function update(T owner, real dt)", + " function exit(T owner)", + "public class NoOpState extends State", + " override function enter(T owner)", + " override function update(T owner, real dt)", + " override function exit(T owner)", + "public class FSM", + " T owner", + " State currentState = null", + " construct(T owner)", + " this.owner = owner", + " function setInitialState(State st)", + " currentState = st", + " if currentState != null", + " currentState.enter(owner)", + " function update(real dt)", + " if currentState != null", + " currentState.update(owner, dt)")); + units.add(compilationUnit("FsmOwner.wurst", + "package FsmOwner", + "import FsmLib", + "public class Owner", + " FSM fsm = new FSM(this)", + " int ticks = 0")); + StringBuilder check = new StringBuilder(" if runOne(idle) == 0"); + List imports = new ArrayList<>(); + for (int n = 1; n <= 5; n++) { + units.add(compilationUnit("FsmS" + n + ".wurst", + "package FsmS" + n, + "import FsmLib", + "import FsmOwner", + "public class St" + n + " extends NoOpState", + " override function update(Owner o, real dt)", + " o.ticks += " + n, + "public constant st" + n + "State = new St" + n + "()")); + imports.add("import FsmS" + n); + check.append(" and runOne(st").append(n).append("State) == ").append(5 * n); + } + List main = new ArrayList<>(List.of("package FsmMain", "import FsmLib", "import FsmOwner")); + main.addAll(imports); + main.addAll(List.of( + "native testSuccess()", + "public constant idle = new NoOpState()", + "function runOne(State st) returns int", + " let o = new Owner()", + " o.fsm.setInitialState(st)", + " for i = 0 to 4", + " o.fsm.update(0.1)", + " return o.ticks", + "init", + check.toString(), + " testSuccess()")); + units.add(compilationUnit("FsmMain.wurst", main.toArray(new String[0]))); + + String first = compileToLua("fsmSiblingsInSeparatePackages", units); + // the slot FSM.update calls, as in __wurst_objectClass[FSM_currentState_storage[this1]].State_update(..., dt) + java.util.regex.Matcher call = java.util.regex.Pattern + .compile("\\]\\.(\\w+)\\([^()\\n]*,\\s*dt\\w*\\)").matcher(first); + assertTrue(call.find(), first); + String slot = call.group(1); + for (int n = 1; n <= 5; n++) { + assertTrue(first.matches("(?s).*\\bSt" + n + "\\." + slot + " = St" + n + "_\\w*update\\b.*"), + "St" + n + " binds " + slot + " to its own update:\n" + first); + } + List reversed = new ArrayList<>(units); + java.util.Collections.reverse(reversed); + assertEquals(compileToLua("fsmSiblingsInSeparatePackages", reversed), first, + "Lua must not depend on the order of the compilation units"); + } + + /** + * Three packages each declare a class Node, extending a shared abstract class, and an implementor of a shared + * interface, with closures of it: receivers whose names are equal must bind their slots in an order which does + * not depend on the order of the compilation units. + */ + @Test + public void sameNamedClassesInSeveralPackagesEmitTheSameLuaInAnyUnitOrder() throws IOException { + List units = new ArrayList<>(); + units.add(compilationUnit("Lib.wurst", + "package Lib", + "public interface Visitor", + " function visit(int x) returns int", + "public abstract class Shape", + " abstract function area() returns int", + "public function apply(Visitor v, int x) returns int", + " return v.visit(x)", + "public function areaOf(Shape s) returns int", + " return s.area()")); + for (String p : List.of("P1", "P2", "P3")) { + units.add(compilationUnit(p + ".wurst", + "package " + p, + "import Lib", + "public class Node extends Shape", + " override function area() returns int", + " return " + p.charAt(1) + "0", + "public class Impl implements Visitor", + " override function visit(int x) returns int", + " return x + 1", + "public function run" + p + "() returns int", + " return areaOf(new Node()) + apply(new Impl(), 1) + apply((int x) -> x * 2, 3) + apply(x -> x * 3, 4)")); + } + units.add(compilationUnit("Main.wurst", + "package Main", + "import P1", + "import P2", + "import P3", + "native testSuccess()", + "init", + // 10+2+6+12 + 20+2+6+12 + 30+2+6+12 + " if runP1() + runP2() + runP3() == 120", + " testSuccess()")); + String first = compileToLua("sameNamedClassesInSeveralPackages", units); + List shuffled = List.of(units.get(4), units.get(2), units.get(0), units.get(3), units.get(1)); + assertEquals(compileToLua("sameNamedClassesInSeveralPackages", shuffled), first, + "same-named receivers must bind in an order independent of the compilation units"); + } + } From aebe6786c130856b4024aa0b7a79affb726d245d Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 19:23:28 +0200 Subject: [PATCH 10/19] Lua dispatch: an interface default beats a method inherited from outside the interface, as on Jass Through the interface, Jass and the interpreter never take the method a class inherits from a superclass which does not implement the interface; Lua chose between the two by class name. Calls through the superclass's method get a slot of their own where one class would need both answers under one key. (cherry picked from commit e95142cddc9fafcdbb9348290a47b5fdcfc32f3b) --- .../lua/translation/LuaTranslator.java | 265 ++++++++++++++++-- .../tests/LuaBackendAuditTests.java | 80 ++++++ 2 files changed, 329 insertions(+), 16 deletions(-) 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 bb054813b..a543c25fd 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 @@ -13,6 +13,7 @@ import de.peeeq.wurstscript.translation.imtranslation.LuaNativeLowering; import de.peeeq.wurstscript.translation.lua.printing.LuaPrinter; import de.peeeq.wurstscript.types.TypesHelper; +import de.peeeq.wurstscript.types.WurstTypeInterface; import de.peeeq.wurstscript.utils.Lazy; import de.peeeq.wurstscript.utils.Utils; import de.peeeq.wurstscript.validation.NamePreservation; @@ -1595,6 +1596,10 @@ private void resolveDispatchSlots() { setResolvedDispatchSlot(pending, "__wurst_destroy"); continue; } + if (needsOwnDispatchSlot(pending.method)) { + ensureOwnDispatchSlot(pending); + continue; + } Set commonSlots = receivers.isEmpty() ? Collections.emptySet() : commonConcreteReceiverSlots(pending.method); @@ -1657,25 +1662,12 @@ private void ensureCanonicalDispatchSlot(PendingDispatch pending) { String canonicalName = semantic; String slot = canonicalDispatchSlots.computeIfAbsent(group, ignored -> uniqueName("__wurst_dispatch_" + canonicalName)); - Set semanticNames = new HashSet<>(); - if (!imTr.dispatchSegmentOf(pending.method).isEmpty()) { - semanticNames.add(imTr.dispatchSegmentOf(pending.method)); - } - String sourceName = sourceSemanticName(pending.method); - if (!sourceName.isEmpty()) { - semanticNames.add(sourceName); - } + Set semanticNames = dispatchSemanticNames(pending.method); Set receivers = concreteReceiversFor(pending.method); List sortedReceivers = new ArrayList<>(receivers); sortedReceivers.sort(Comparator.comparing(this::classSortKey)); for (ImClass receiver : sortedReceivers) { - List candidates = new ArrayList<>(); - for (ImMethod candidate : collectMethodsInHierarchy(receiver)) { - if (sameDispatchFamily(pending.method, candidate) - && sharesDispatchSemanticName(candidate, semanticNames)) { - candidates.add(candidate); - } - } + List candidates = dispatchCandidatesOf(receiver, pending.method, semanticNames); ImMethod implementation = chooseBestImplementationForClass(receiver, candidates); if (implementation == null) { throw new RuntimeException("Wurst Lua backend assertion failed: no implementation for dispatch slot '" @@ -1692,6 +1684,31 @@ && sharesDispatchSemanticName(candidate, semanticNames)) { pending.target.setFieldName(slot); } + /** The names under which a method's family can be found in a class's hierarchy: its segment and its source name. */ + private Set dispatchSemanticNames(ImMethod method) { + Set semanticNames = new HashSet<>(); + if (!imTr.dispatchSegmentOf(method).isEmpty()) { + semanticNames.add(imTr.dispatchSegmentOf(method)); + } + String sourceName = sourceSemanticName(method); + if (!sourceName.isEmpty()) { + semanticNames.add(sourceName); + } + return semanticNames; + } + + /** The methods of {@code receiver}'s hierarchy which can implement a call through {@code method}. */ + private List dispatchCandidatesOf(ImClass receiver, ImMethod method, Set semanticNames) { + List candidates = new ArrayList<>(); + for (ImMethod candidate : collectMethodsInHierarchy(receiver)) { + if (sameDispatchFamily(method, candidate) + && sharesDispatchSemanticName(candidate, semanticNames)) { + candidates.add(candidate); + } + } + return candidates; + } + private Set dispatchCandidateSlots(ImMethod method) { Set candidates = new TreeSet<>(); candidates.add(dispatchSlotName(method.getName())); @@ -2137,16 +2154,33 @@ private ImMethod chooseBestImplementationForClass(ImClass receiverClass, List compareDispatchCandidates(receiverClass, a, b)); + Set passedOver = passedOverByAnInterfaceDefault(concrete); + concrete.sort((a, b) -> compareDispatchCandidates(receiverClass, a, b, passedOver)); return concrete.get(0); } private int compareDispatchCandidates(ImClass receiverClass, ImMethod a, ImMethod b) { + return compareDispatchCandidates(receiverClass, a, b, passedOverByAnInterfaceDefault(List.of(a, b))); + } + + /** + * Orders two implementations a class could bind for one slot, the better first: the class's own, then one no + * interface default among the candidates passes over ({@link #implementationsOutsideItsInterface}), then the + * nearer class. Two which are still tied come from different branches of the hierarchy at the same distance, + * where Jass and the interpreter take the one on the side of the method called through; the name order only + * makes that choice deterministic. + */ + private int compareDispatchCandidates(ImClass receiverClass, ImMethod a, ImMethod b, Set passedOver) { boolean aLocal = isImplementationFromClass(a, receiverClass); boolean bLocal = isImplementationFromClass(b, receiverClass); if (aLocal != bLocal) { return aLocal ? -1 : 1; } + boolean aPassedOver = passedOver.contains(a); + boolean bPassedOver = passedOver.contains(b); + if (aPassedOver != bPassedOver) { + return aPassedOver ? 1 : -1; + } int aDist = classDistance(receiverClass, a.attrClass()); int bDist = classDistance(receiverClass, b.attrClass()); if (aDist != bDist) { @@ -2172,6 +2206,205 @@ private boolean isImplementationFromClass(ImMethod method, ImClass ownerClass) { return method.attrClass() == ownerClass; } + /** The candidates which an interface default among them passes over, see {@link #implementationsOutsideItsInterface}. */ + private Set passedOverByAnInterfaceDefault(List candidates) { + Set result = null; + for (ImMethod interfaceDefault : candidates) { + Set outside = implementationsOutsideItsInterface(interfaceDefault); + if (outside.isEmpty()) { + continue; + } + for (ImMethod candidate : candidates) { + if (candidate != interfaceDefault && outside.contains(candidate)) { + if (result == null) { + result = Collections.newSetFromMap(new IdentityHashMap<>()); + } + result.add(candidate); + } + } + } + return result == null ? Collections.emptySet() : result; + } + + private final Map> implementationsOutsideInterface = new IdentityHashMap<>(); + + /** + * The implementations which the sub-methods of an interface's default method list from classes outside that + * interface. A class which implements the interface and inherits the method from a superclass which does not + * ({@code C extends Base implements Omega}) has the superclass's method recorded as its implementation of the + * interface's. Jass and the interpreter never call it through the interface: the Jass dispatch of + * {@code Omega.m} only takes methods of the classes below Omega, and the interpreter only sub-methods of + * subclasses, so through the interface the default stands. Through the superclass it is the other way round + * (see {@link #needsOwnDispatchSlot}). + * + *

Asked of the IM classes, which the Jass dispatch walks, and of the source as well, because Lua's targeted + * specialisation moves implementations between erased and specialised classes, whose IM relation then no + * longer says whether the source class implements the interface. + */ + private Set implementationsOutsideItsInterface(ImMethod method) { + return implementationsOutsideInterface.computeIfAbsent(method, interfaceDefault -> { + ImClass owner = interfaceDefault.attrClass(); + if (interfaceDefault.getIsAbstract() || interfaceDefault.getImplementation() == null || owner == null + || !(owner.attrTrace() instanceof InterfaceDef interfaceDef)) { + return Collections.emptySet(); + } + Set outside = Collections.newSetFromMap(new IdentityHashMap<>()); + Set visited = Collections.newSetFromMap(new IdentityHashMap<>()); + ArrayDeque queue = new ArrayDeque<>(interfaceDefault.getSubMethods()); + while (!queue.isEmpty()) { + ImMethod sub = queue.removeFirst(); + if (!visited.add(sub)) { + continue; + } + queue.addAll(sub.getSubMethods()); + ImClass subClass = sub.attrClass(); + if (subClass != null && !subClass.isSubclassOf(owner) && !declaredInAnImplementationOf(sub, interfaceDef)) { + outside.add(sub); + } + } + return outside.isEmpty() ? Collections.emptySet() : outside; + }); + } + + /** Whether the source declares {@code method} in a class which implements {@code interfaceDef}; unknown counts as yes. */ + private static boolean declaredInAnImplementationOf(ImMethod method, InterfaceDef interfaceDef) { + if (!(method.attrTrace() instanceof FuncDef funcDef)) { + return true; + } + ClassDef classDef = funcDef.attrNearestClassDef(); + if (classDef == null) { + return true; + } + for (WurstTypeInterface implemented : classDef.attrTypC().transitiveSuperInterfaces()) { + if (implemented.getDef() == interfaceDef) { + return true; + } + } + return false; + } + + private Set passedOverAnywhere; + + /** Every implementation some interface default passes over; almost always none. */ + private Set passedOverAnywhere() { + if (passedOverAnywhere == null) { + passedOverAnywhere = Collections.newSetFromMap(new IdentityHashMap<>()); + for (ImClass c : prog.getClasses()) { + for (ImMethod m : c.getMethods()) { + passedOverAnywhere.addAll(implementationsOutsideItsInterface(m)); + } + } + } + return passedOverAnywhere; + } + + private final Map> reachedThroughCache = new IdentityHashMap<>(); + + /** The implementations a call through {@code method} can run on Jass: the method and its sub-methods. */ + private Set reachedThrough(ImMethod method) { + return reachedThroughCache.computeIfAbsent(method, root -> { + Set reached = Collections.newSetFromMap(new IdentityHashMap<>()); + ArrayDeque queue = new ArrayDeque<>(); + queue.add(root); + while (!queue.isEmpty()) { + ImMethod current = queue.removeFirst(); + if (reached.add(current)) { + queue.addAll(current.getSubMethods()); + } + } + return reached; + }); + } + + /** + * The interface defaults among {@code candidates} which a call reaching {@code reached} must not run: they are no + * implementation it reaches, but pass over one which is. + */ + private Set defaultsBesideTheCall(List candidates, Set reached) { + Set result = null; + for (ImMethod interfaceDefault : candidates) { + if (reached.contains(interfaceDefault)) { + continue; + } + Set outside = implementationsOutsideItsInterface(interfaceDefault); + for (ImMethod candidate : candidates) { + if (outside.contains(candidate) && reached.contains(candidate)) { + if (result == null) { + result = Collections.newSetFromMap(new IdentityHashMap<>()); + } + result.add(interfaceDefault); + break; + } + } + } + return result == null ? Collections.emptySet() : result; + } + + /** + * Whether calls through {@code method} need a slot of their own. A group has one slot, and a class binds one + * implementation to it. C extends Base implements Omega binds Omega's default, which is right through Omega; but + * through Base, Jass and the interpreter run Base.m on C, so where Base.m is dispatched (it has overrides), its + * calls cannot read the group's slot. + */ + private boolean needsOwnDispatchSlot(ImMethod method) { + Set passedOver = passedOverAnywhere(); + if (passedOver.isEmpty()) { + return false; + } + return needsOwnDispatchSlotCache.computeIfAbsent(method, m -> { + Set reached = reachedThrough(m); + if (reached.stream().noneMatch(passedOver::contains)) { + return false; + } + Set semanticNames = dispatchSemanticNames(m); + for (ImClass receiver : concreteReceiversFor(m)) { + List candidates = dispatchCandidatesOf(receiver, m, semanticNames); + ImMethod bound = chooseBestImplementationForClass(receiver, candidates); + if (bound != null && defaultsBesideTheCall(candidates, reached).contains(bound)) { + return true; + } + } + return false; + }); + } + + private final Map needsOwnDispatchSlotCache = new IdentityHashMap<>(); + + private final Map ownDispatchSlots = new IdentityHashMap<>(); + + /** + * Give calls through {@code pending.method} a slot of their own, which every receiver binds to the implementation + * the call reaches: the one the group binds, unless that is an interface default beside the call. + */ + private void ensureOwnDispatchSlot(PendingDispatch pending) { + ImMethod method = pending.method; + String slot = ownDispatchSlots.get(method); + if (slot == null) { + String semantic = dispatchSlotName(imTr.dispatchSegmentOf(method)); + slot = uniqueName("__wurst_dispatch_" + (semantic.isEmpty() ? "method" : semantic)); + ownDispatchSlots.put(method, slot); + Set reached = reachedThrough(method); + Set semanticNames = dispatchSemanticNames(method); + List sortedReceivers = new ArrayList<>(concreteReceiversFor(method)); + sortedReceivers.sort(Comparator.comparing(this::classSortKey)); + for (ImClass receiver : sortedReceivers) { + List candidates = dispatchCandidatesOf(receiver, method, semanticNames); + Set beside = defaultsBesideTheCall(candidates, reached); + candidates.removeIf(beside::contains); + ImMethod implementation = chooseBestImplementationForClass(receiver, candidates); + if (implementation == null) { + throw new RuntimeException("Wurst Lua backend assertion failed: no implementation for dispatch slot '" + + slot + "' in descriptor for " + receiver.getName() + "."); + } + registerDispatchSlot(receiver, slot, dispatchGroupOf(method)); + deferMainInit(LuaAst.LuaAssignment( + LuaAst.LuaExprFieldAccess(LuaAst.LuaExprVarAccess(luaClassVar.getFor(receiver)), slot), + LuaAst.LuaExprFuncRef(luaFunc.getFor(implementation.getImplementation())))); + } + } + pending.target.setFieldName(slot); + } + private boolean isNoOpImplementation(ImMethod method) { return method != null && method.getImplementation() != null diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java index 598c149f0..ec02e8aa7 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaBackendAuditTests.java @@ -4839,6 +4839,86 @@ public void aStateNamedLikeTheNoOpBaseKeepsItsInheritedUpdate() { " testSuccess()")); } + /** + * C inherits m from its superclass and a default m from an interface it implements. Through the interface, Jass + * and the interpreter run the interface's default: the dispatch of Omega.m only takes implementations from the + * subclasses of Omega, and Base is none. Lua bound whichever of the two sorted first by class name. + */ + @Test + public void interfaceDefaultVersusInheritedMethodDoesNotDependOnClassNames() throws IOException { + test().testLua(true).luaOnly(false).executeProg().lines( + "package test", "native testSuccess()", "interface Omega", " function m() returns int", " return 2", + "class Base", " function m() returns int", " return 1", "class C extends Base implements Omega", + "function viaOmega(Omega a) returns int", " return a.m()", + "init", " if viaOmega(new C()) == 2", " testSuccess()"); + String lua = compiledLua("interfaceDefaultVersusInheritedMethodDoesNotDependOnClassNames"); + String slot = dispatchedSlot(lua, "viaOmega"); + assertTrue("C binds " + slot + " to Omega's default:\n" + lua, lua.contains("C." + slot + " = Omega_Omega_m")); + } + + /** The slot the dispatching call in {@code functionName} reads. */ + private String dispatchedSlot(String lua, String functionName) { + java.util.regex.Matcher call = java.util.regex.Pattern.compile("__wurst_objectClass\\[\\w+\\]\\.(\\w+)\\(") + .matcher(luaFunctionBody(lua, functionName)); + assertTrue("expected a dispatching call in " + functionName + ":\n" + lua, call.find()); + return call.group(1); + } + + /** The same with the superclass sorting after the interface. */ + @Test + public void interfaceDefaultVersusInheritedMethodDoesNotDependOnClassNamesSortedTheOtherWay() { + test().testLua(true).luaOnly(false).executeProg().lines( + "package test", "native testSuccess()", "interface Omega", " function m() returns int", " return 2", + "class Zeta", " function m() returns int", " return 1", "class C extends Zeta implements Omega", + "function viaOmega(Omega a) returns int", " return a.m()", + "init", " if viaOmega(new C()) == 2", " testSuccess()"); + } + + /** The default is also the one through an interface which C implements by an interface extending it. */ + @Test + public void interfaceDefaultReachedThroughAnExtendingInterfaceBeatsTheInheritedMethod() { + test().testLua(true).luaOnly(false).executeProg().lines( + "package test", "native testSuccess()", "interface Omega", " function m() returns int", " return 2", + "interface Omega2 extends Omega", + "class Base", " function m() returns int", " return 1", "class C extends Base implements Omega2", + "class D extends C", + "function viaOmega(Omega a) returns int", " return a.m()", + "init", " if viaOmega(new C()) == 2 and viaOmega(new D()) == 2", " testSuccess()"); + } + + /** + * Through the superclass, C runs the method it inherits from there, also when that method has overrides, so + * that it is dispatched as well: one object answers 2 through the interface and 1 through the superclass, on + * Jass and in the interpreter. F, which overrides both, makes the two one dispatch family. + */ + @Test + public void interfaceDefaultAndInheritedMethodEachDispatchThroughTheirOwnType() throws IOException { + for (String superclass : List.of("Base", "Zeta")) { + String name = "interfaceDefaultAndInheritedMethodEachDispatchThroughTheirOwnType_" + superclass; + testNamed(name) + .testLua(true).luaOnly(false).executeProg().lines( + "package test", "native testSuccess()", "interface Omega", " function m() returns int", " return 2", + "class " + superclass, " function m() returns int", " return 1", + "class C extends " + superclass + " implements Omega", + "class E extends " + superclass, " override function m() returns int", " return 5", + "class F extends " + superclass + " implements Omega", " override function m() returns int", " return 3", + "function viaOmega(Omega a) returns int", " return a.m()", + "function viaSuper(" + superclass + " a) returns int", " return a.m()", + "init", + " if viaOmega(new C()) == 2 and viaSuper(new C()) == 1 and viaSuper(new E()) == 5", + " if viaOmega(new F()) == 3 and viaSuper(new F()) == 3", + " testSuccess()"); + // one class table cannot give C both answers under one key: the two calls read different slots + String lua = compiledLua(name); + String throughOmega = dispatchedSlot(lua, "viaOmega"); + String throughSuper = dispatchedSlot(lua, "viaSuper"); + assertTrue("C binds " + throughOmega + " to Omega's default:\n" + lua, + lua.contains("C." + throughOmega + " = Omega_Omega_m")); + assertTrue("C binds " + throughSuper + " to " + superclass + "'s method:\n" + lua, + lua.contains("C." + throughSuper + " = " + superclass + "_" + superclass + "_m")); + } + } + /** * Constructor helper methods are named create, create1, create2, ... in * class-translation order, while method dispatch slots use (normalized) From de029effa2278e1d7e579d27538c5570340d8801 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 19:23:43 +0200 Subject: [PATCH 11/19] Lua dispatch: drop the NoOpState name tie-breaker Since a class's own method is told by its class, it decides no binding: the Lua of LuaBackendAuditTests, LuaTranslationTests, DeterministicChecks, InterfaceTests and ClassesTests is byte-identical without it. (cherry picked from commit 04056283c24b4632fc907a4ce5444c165403fb7c) --- .../translation/lua/translation/LuaTranslator.java | 11 ----------- 1 file changed, 11 deletions(-) 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 a543c25fd..741a324ba 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 @@ -2186,11 +2186,6 @@ private int compareDispatchCandidates(ImClass receiverClass, ImMethod a, ImMetho if (aDist != bDist) { return Integer.compare(aDist, bDist); } - boolean aNoOp = isNoOpImplementation(a); - boolean bNoOp = isNoOpImplementation(b); - if (aNoOp != bNoOp) { - return aNoOp ? 1 : -1; - } return methodSortKey(a).compareTo(methodSortKey(b)); } @@ -2405,12 +2400,6 @@ private void ensureOwnDispatchSlot(PendingDispatch pending) { pending.target.setFieldName(slot); } - private boolean isNoOpImplementation(ImMethod method) { - return method != null - && method.getImplementation() != null - && method.getImplementation().getName().contains("NoOpState_"); - } - private int classDistance(ImClass from, ImClass to) { if (from == null || to == null) { return Integer.MAX_VALUE; From 5e8635f2280391544202fc554c5199ddac3edaf6 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 18:42:42 +0200 Subject: [PATCH 12/19] Local-player tests: test the analysis, and the edges no test pinned Six OptimizerTests compared with Player(n) in the second condition, a native call, which the side-effect check refuses to move a statement across whatever the analysis says: they passed with an analysis which reports nothing. The native is read into a local first now, so each fails when the analysis misses its edge. Nine unit tests pin the edges which no test of the analysis caught when removed: the return value's tree parent, an exit under a local branch, vararg arguments and loop variables, every assigned left side, method receivers and implementations, the control edge into a return, and the stop at a nested loop. (cherry picked from commit ddc1e101f97c4107d875946037f59971ba2cae3a) --- .../LocalPlayerContextAnalyzerTests.java | 264 ++++++++++++++++++ .../wurstscript/tests/OptimizerTests.java | 18 +- 2 files changed, 276 insertions(+), 6 deletions(-) diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LocalPlayerContextAnalyzerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LocalPlayerContextAnalyzerTests.java index 7f2ebde0d..c5b74318d 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LocalPlayerContextAnalyzerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LocalPlayerContextAnalyzerTests.java @@ -294,4 +294,268 @@ public void analysisOfAConstantOnlyFunctionBodyIsClean() { assertFalse(analyzer.isLocalPlayerDependent(variable)); assertFalse(analyzer.functionUsesLocalPlayer(function)); } + + /** + * The inlining barrier follows data flow through the whole return expression: a function which does + * not call a client-local native itself, but returns {@code wrapper() + 1}, returns a client-local value. + * (Pins the tree-parent step of the data-only pass; every existing barrier test returns the call itself.) + */ + @Test + public void aReturnValueWhichContainsAClientLocalCallIsAnInliningBarrier() { + ImFunction wrapper = function("wrapper", JassIm.ImVars(), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(trace, call(localPlayer))), TypesHelper.imInt()); + ImFunction offset = function("offset", JassIm.ImVars(), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(trace, JassIm.ImOperatorCall(WurstOperator.PLUS, + JassIm.ImExprs(call(wrapper), JassIm.ImIntVal(1))))), TypesHelper.imInt()); + LocalPlayerContextAnalyzer analyzer = new LocalPlayerContextAnalyzer(prog(JassIm.ImVars(), wrapper, offset)); + + assertTrue(analyzer.functionInliningIsLocalPlayerSensitive(offset), + "offset() returns a value computed from a client-local one"); + } + + /** + * A {@code break} under a client-local branch ({@code if local then exitwhen true}) decides how often the + * rest of the loop runs, although the exit's own condition is a constant. + */ + @Test + public void aBreakUnderALocalBranchControlsTheRestOfTheLoop() { + ImVar inLoop = intVar("inLoop"); + ImLoop loop = JassIm.ImLoop(trace, JassIm.ImStmts( + JassIm.ImIf(trace, localCondition(), + JassIm.ImStmts(JassIm.ImExitwhen(trace, JassIm.ImBoolVal(true))), JassIm.ImStmts()), + set(inLoop, JassIm.ImIntVal(1)))); + ImFunction function = voidFunction("function", JassIm.ImVars(inLoop), loop); + LocalPlayerContextAnalyzer analyzer = new LocalPlayerContextAnalyzer(prog(JassIm.ImVars(), function)); + + assertTrue(analyzer.isLocalPlayerDependent(inLoop), "runs only while the local branch did not break"); + } + + /** An exit of an inner loop leaves only that loop: what the outer loop runs after it is not guarded by it. */ + @Test + public void anExitOfAnInnerLoopDoesNotGuardTheOuterLoop() { + ImVar outerOnly = intVar("outerOnly"); + ImLoop inner = JassIm.ImLoop(trace, JassIm.ImStmts(JassIm.ImExitwhen(trace, localCondition()))); + ImLoop outer = JassIm.ImLoop(trace, JassIm.ImStmts( + inner, + set(outerOnly, JassIm.ImIntVal(1)), + JassIm.ImExitwhen(trace, JassIm.ImBoolVal(true)))); + ImFunction function = voidFunction("function", JassIm.ImVars(outerOnly), outer); + LocalPlayerContextAnalyzer analyzer = new LocalPlayerContextAnalyzer(prog(JassIm.ImVars(), function)); + + assertTrue(analyzer.isLocalPlayerDependent(inner)); + assertFalse(analyzer.isLocalPlayerDependent(outerOnly), "the outer loop does not leave on the local exit"); + } + + /** + * A client-local value passed in the vararg part of a call reaches the vararg parameter and the variable + * of a vararg loop over it, and not the positional parameter. + */ + @Test + public void aVarargArgumentReachesTheVarargParameterAndTheLoopVariable() { + ImVar first = intVar("first"); + ImVar rest = intVar("rest"); + ImVar element = intVar("element"); + ImVar sum = intVar("sum"); + ImVar result = intVar("result"); + ImFunction varargFunction = JassIm.ImFunction(trace, "sum", JassIm.ImTypeVars(), JassIm.ImVars(first, rest), + TypesHelper.imInt(), JassIm.ImVars(element, sum), + JassIm.ImStmts( + JassIm.ImVarargLoop(trace, + JassIm.ImStmts(set(sum, JassIm.ImOperatorCall(WurstOperator.PLUS, + JassIm.ImExprs(JassIm.ImVarAccess(sum), JassIm.ImVarAccess(element))))), + JassIm.ImVarargLoopVars(JassIm.ImVarargLoopVar(element))), + JassIm.ImReturn(trace, JassIm.ImVarAccess(sum))), + Collections.singletonList(FunctionFlagEnum.IS_VARARG)); + ImFunction user = voidFunction("user", JassIm.ImVars(result), + set(result, call(varargFunction, JassIm.ImIntVal(1), JassIm.ImIntVal(2), call(localPlayer)))); + LocalPlayerContextAnalyzer analyzer = + new LocalPlayerContextAnalyzer(prog(JassIm.ImVars(), varargFunction, user)); + + assertTrue(analyzer.isLocalPlayerDependent(rest), "the client-local value is one of the varargs"); + assertTrue(analyzer.isLocalPlayerDependent(element), "the loop variable takes every vararg"); + assertTrue(analyzer.isLocalPlayerDependent(sum)); + assertFalse(analyzer.isLocalPlayerDependent(first), "the positional parameter receives a constant"); + } + + /** + * Every variable an assignment writes depends on what it assigns, whatever the shape of its left side: + * a tuple, a component of a tuple variable, a statement expression, an array element, a member. + */ + @Test + public void everyShapeOfAnAssignedLeftSideIsDependent() { + ImVar tupleA = intVar("tupleA"); + ImVar tupleB = intVar("tupleB"); + ImVar selected = intVar("selected"); + ImVar viaStatementExpr = intVar("viaStatementExpr"); + ImVar array = intVar("array"); + ImVar field = intVar("field"); + ImVar receiver = intVar("receiver"); + ImVar clean = intVar("clean"); + ImFunction function = voidFunction("function", JassIm.ImVars(tupleA, tupleB, selected, viaStatementExpr, + receiver, clean), + JassIm.ImSet(trace, JassIm.ImTupleExpr(JassIm.ImExprs(JassIm.ImVarAccess(tupleA), + JassIm.ImVarAccess(tupleB))), call(localPlayer)), + JassIm.ImSet(trace, JassIm.ImTupleSelection(JassIm.ImVarAccess(selected), 0), call(localPlayer)), + JassIm.ImSet(trace, JassIm.ImStatementExpr(JassIm.ImStmts(), JassIm.ImVarAccess(viaStatementExpr)), + call(localPlayer)), + // The index and the receiver are what is client-local here, not the value. + JassIm.ImSet(trace, JassIm.ImVarArrayAccess(trace, array, JassIm.ImExprs(call(localPlayer))), + JassIm.ImIntVal(1)), + JassIm.ImSet(trace, JassIm.ImMemberAccess(trace, call(localPlayer), JassIm.ImTypeArguments(), field, + JassIm.ImExprs()), JassIm.ImIntVal(1)), + set(clean, JassIm.ImIntVal(1))); + LocalPlayerContextAnalyzer analyzer = + new LocalPlayerContextAnalyzer(prog(JassIm.ImVars(array, field), function)); + + for (ImVar variable : List.of(tupleA, tupleB, selected, viaStatementExpr, array, field)) { + assertTrue(analyzer.isLocalPlayerDependent(variable), variable.getName() + " is written client-locally"); + } + assertFalse(analyzer.isLocalPlayerDependent(clean)); + } + + /** + * The receiver of a method call is passed as {@code this}: a client-local receiver reaches the parameters + * of the implementations, with one operand (each operand connected to each parameter) and with several + * (through one intermediate fact). + */ + @Test + public void aClientLocalReceiverReachesTheParametersOfTheImplementations() { + ImVar getThis = intVar("getThis"); + ImVar baseThis = intVar("baseThis"); + ImVar baseValue = intVar("baseValue"); + ImVar subThis = intVar("subThis"); + ImVar subValue = intVar("subValue"); + ImVar receiver = intVar("receiver"); + ImVar argument = intVar("argument"); + ImVar result = intVar("result"); + ImFunction getImplementation = function("C_get", JassIm.ImVars(getThis), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(trace, JassIm.ImIntVal(0))), TypesHelper.imInt()); + ImFunction baseImplementation = function("C_m", JassIm.ImVars(baseThis, baseValue), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(trace, JassIm.ImIntVal(0))), TypesHelper.imInt()); + ImFunction subImplementation = function("D_m", JassIm.ImVars(subThis, subValue), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(trace, JassIm.ImIntVal(1))), TypesHelper.imInt()); + ImClass base = JassIm.ImClass(trace, "C", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImMethods(), + JassIm.ImFunctions(getImplementation, baseImplementation, subImplementation), Collections.emptyList()); + ImClassType baseType = JassIm.ImClassType(base, JassIm.ImTypeArguments()); + ImMethod get = JassIm.ImMethod(trace, baseType, "get", getImplementation, Collections.emptyList(), + Collections.emptyList(), "get", false); + ImMethod subMethod = JassIm.ImMethod(trace, baseType, "m", subImplementation, Collections.emptyList(), + Collections.emptyList(), "m", false); + ImMethod method = JassIm.ImMethod(trace, baseType, "m", baseImplementation, List.of(subMethod), + Collections.emptyList(), "m", false); + ImFunction user = voidFunction("user", JassIm.ImVars(receiver, argument, result), + set(receiver, call(localPlayer)), + set(result, JassIm.ImMethodCall(trace, get, JassIm.ImTypeArguments(), JassIm.ImVarAccess(receiver), + JassIm.ImExprs(), false)), + set(result, JassIm.ImMethodCall(trace, method, JassIm.ImTypeArguments(), JassIm.ImVarAccess(receiver), + JassIm.ImExprs(JassIm.ImVarAccess(argument)), false))); + ImProg prog = prog(JassIm.ImVars(), user); + prog.getClasses().add(base); + LocalPlayerContextAnalyzer analyzer = new LocalPlayerContextAnalyzer(prog); + + for (ImVar parameter : List.of(getThis, baseThis, baseValue, subThis, subValue)) { + assertTrue(analyzer.isLocalPlayerDependent(parameter), + parameter.getName() + " may receive the client-local receiver"); + } + assertFalse(analyzer.isLocalPlayerDependent(argument), "an operand is not changed by being passed on"); + } + + /** + * A method call whose operands are all synchronized is still client-local when an implementation it can + * dispatch to returns a client-local value (here only the override does). + */ + @Test + public void aMethodCallIsDependentWhenAnImplementationReturnsAClientLocalValue() { + ImVar baseThis = intVar("baseThis"); + ImVar subThis = intVar("subThis"); + ImVar receiver = intVar("receiver"); + ImVar result = intVar("result"); + ImVar viaBase = intVar("viaBase"); + ImFunction baseImplementation = function("C_m", JassIm.ImVars(baseThis), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(trace, JassIm.ImIntVal(0))), TypesHelper.imInt()); + ImFunction subImplementation = function("D_m", JassIm.ImVars(subThis), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(trace, call(localPlayer))), TypesHelper.imInt()); + ImClass base = JassIm.ImClass(trace, "C", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImMethods(), + JassIm.ImFunctions(baseImplementation, subImplementation), Collections.emptyList()); + ImClassType baseType = JassIm.ImClassType(base, JassIm.ImTypeArguments()); + ImMethod subMethod = JassIm.ImMethod(trace, baseType, "m", subImplementation, Collections.emptyList(), + Collections.emptyList(), "m", false); + ImMethod method = JassIm.ImMethod(trace, baseType, "m", baseImplementation, List.of(subMethod), + Collections.emptyList(), "m", false); + ImMethodCall methodCall = JassIm.ImMethodCall(trace, method, JassIm.ImTypeArguments(), + JassIm.ImVarAccess(receiver), JassIm.ImExprs(), false); + ImFunction user = voidFunction("user", JassIm.ImVars(receiver, result, viaBase), + set(result, methodCall), + set(viaBase, JassIm.ImMethodCall(trace, subMethod, JassIm.ImTypeArguments(), + JassIm.ImVarAccess(receiver), JassIm.ImExprs(), false))); + ImProg prog = prog(JassIm.ImVars(), user); + prog.getClasses().add(base); + LocalPlayerContextAnalyzer analyzer = new LocalPlayerContextAnalyzer(prog); + + assertTrue(analyzer.isLocalPlayerDependent(methodCall), "the call may dispatch to D_m"); + assertTrue(analyzer.isLocalPlayerDependent(result)); + assertTrue(analyzer.isLocalPlayerDependent(viaBase)); + assertFalse(analyzer.isLocalPlayerDependent(receiver)); + } + + /** A method called under client-local control runs every implementation it can dispatch to under that control. */ + @Test + public void aMethodCalledUnderLocalControlControlsEveryImplementation() { + ImVar baseThis = intVar("baseThis"); + ImVar subThis = intVar("subThis"); + ImVar baseWritten = intVar("baseWritten"); + ImVar subWritten = intVar("subWritten"); + ImVar receiver = intVar("receiver"); + ImFunction baseImplementation = function("C_m", JassIm.ImVars(baseThis), JassIm.ImVars(), + JassIm.ImStmts(set(baseWritten, JassIm.ImIntVal(1))), JassIm.ImVoid()); + ImFunction subImplementation = function("D_m", JassIm.ImVars(subThis), JassIm.ImVars(), + JassIm.ImStmts(set(subWritten, JassIm.ImIntVal(1))), JassIm.ImVoid()); + ImClass base = JassIm.ImClass(trace, "C", JassIm.ImTypeVars(), JassIm.ImVars(), JassIm.ImMethods(), + JassIm.ImFunctions(baseImplementation, subImplementation), Collections.emptyList()); + ImClassType baseType = JassIm.ImClassType(base, JassIm.ImTypeArguments()); + ImMethod subMethod = JassIm.ImMethod(trace, baseType, "m", subImplementation, Collections.emptyList(), + Collections.emptyList(), "m", false); + ImMethod method = JassIm.ImMethod(trace, baseType, "m", baseImplementation, List.of(subMethod), + Collections.emptyList(), "m", false); + ImFunction user = voidFunction("user", JassIm.ImVars(receiver), + JassIm.ImIf(trace, localCondition(), + JassIm.ImStmts(JassIm.ImMethodCall(trace, method, JassIm.ImTypeArguments(), + JassIm.ImVarAccess(receiver), JassIm.ImExprs(), false)), + JassIm.ImStmts())); + ImProg prog = prog(JassIm.ImVars(baseWritten, subWritten), user); + prog.getClasses().add(base); + LocalPlayerContextAnalyzer analyzer = new LocalPlayerContextAnalyzer(prog); + + assertTrue(analyzer.isLocalPlayerDependent(baseWritten)); + assertTrue(analyzer.isLocalPlayerDependent(subWritten), "the override can run under the local branch too"); + assertFalse(analyzer.isLocalPlayerDependent(baseThis), "the receiver itself is synchronized"); + } + + /** + * A function which returns under client-local control returns a client-local value to its callers, even + * when every value it returns is a constant and the call has no operand. (Pins the control edge into the + * return fact; OptimizerTests.localPlayerControlMustPropagateIntoFunctionReturns does not, see its fix.) + */ + @Test + public void aReturnUnderLocalControlMakesTheCallDependent() { + ImVar result = intVar("result"); + ImVar other = intVar("other"); + // pick(): if GetLocalPlayer() == 0 then return 1 end; return 0 + ImFunction pick = function("pick", JassIm.ImVars(), JassIm.ImVars(), + JassIm.ImStmts( + JassIm.ImIf(trace, localCondition(), + JassIm.ImStmts(JassIm.ImReturn(trace, JassIm.ImIntVal(1))), JassIm.ImStmts()), + JassIm.ImReturn(trace, JassIm.ImIntVal(0))), + TypesHelper.imInt()); + ImFunction constant = function("constant", JassIm.ImVars(), JassIm.ImVars(), + JassIm.ImStmts(JassIm.ImReturn(trace, JassIm.ImIntVal(0))), TypesHelper.imInt()); + ImFunction user = voidFunction("user", JassIm.ImVars(result, other), + set(result, call(pick)), + set(other, call(constant))); + LocalPlayerContextAnalyzer analyzer = + new LocalPlayerContextAnalyzer(prog(JassIm.ImVars(), pick, constant, user)); + + assertTrue(analyzer.isLocalPlayerDependent(result), "which return runs depends on the local player"); + assertFalse(analyzer.isLocalPlayerDependent(other)); + } } diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index d9b9a351f..364458b82 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -3502,7 +3502,8 @@ public void branchMergerMustTrackLocalPlayerControlDependentAssignments() throws " selected = Player(0)", " else", " selected = Player(1)", - " if selected == Player(0)", + " player playerZero = Player(0)", + " if selected == playerZero", " result = 11", " else", " result = 11", @@ -3595,7 +3596,8 @@ public void localPlayerControlMustPropagateThroughCalledFunctions() throws Excep " select(Player(0))", " else", " select(Player(1))", - " if selected == Player(0)", + " player playerZero = Player(0)", + " if selected == playerZero", " result = 13", " else", " result = 13", @@ -3625,7 +3627,8 @@ public void localPlayerControlMustPropagateIntoFunctionReturns() throws Exceptio " return Player(1)", "init", " player selected = selectedPlayer()", - " if selected == Player(0)", + " player playerZero = Player(0)", + " if selected == playerZero", " result = 17", " else", " result = 17", @@ -3655,7 +3658,8 @@ public void statementsAfterLocalEarlyReturnMustRemainLocallyControlled() throws " selected = Player(1)", "init", " updateUnlessLocalPlayerZero()", - " if selected == Player(1)", + " player playerOne = Player(1)", + " if selected == playerOne", " result = 29", " else", " result = 29", @@ -3685,7 +3689,8 @@ public void andRightOperandMustInheritLocalPlayerControl() throws Exception { "init", " if (GetLocalPlayer() == Player(0)) and updateSelectedState()", " print(0)", - " if selected == Player(0)", + " player playerZero = Player(0)", + " if selected == playerZero", " result = 19", " else", " result = 19", @@ -3715,7 +3720,8 @@ public void orRightOperandMustInheritLocalPlayerControl() throws Exception { "init", " if (GetLocalPlayer() == Player(0)) or updateSelectedState()", " print(0)", - " if selected == Player(0)", + " player playerZero = Player(0)", + " if selected == playerZero", " result = 23", " else", " result = 23", From 72a1f7c88dd70f6e004cf57e019a298617f287c8 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 18:43:40 +0200 Subject: [PATCH 13/19] Inliner test: compare inlined callees with their twins around loops No test inlined an early return next to break or continue, a return-free loop after a returning one, a switch in a loop, a for-in loop closing before the return or a tuple return. Each callee has an @noinline twin with the same body; the program fails unless both give the same results and the same trace of effects. (cherry picked from commit fc8fa5d3fbc4eaba34c9c194692308f8049d3a64) --- .../wurstscript/tests/OptimizerTests.java | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 364458b82..1b9e7254f 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -4205,4 +4205,211 @@ public void localPlayerDependenceReachesTheEnclosingElementsAndNothingElse() { assertFalse(analyzer.isLocalPlayerDependent(cleanAssignment)); assertFalse(analyzer.isLocalPlayerDependent(clean)); } + + /** + * Every callee has an @noinline twin with the same body, and the program fails unless both give the same results + * and the same trace of effects: early returns next to break and continue, a return-free loop after a returning + * one, switch in a loop, for-in closing before the return, tuple returns, and inlined callees with returns of + * their own in conditions and return values. + */ + @Test + public void inlinedReturnLoweringMatchesTheCalledFunction() { + test().testLua(true).luaOnly(false).inline().localOptimizations().executeProg().lines( + "package test", + "native testSuccess()", + "int trace = 0", + "int closed = 0", + "int failures = 0", + "tuple pair(int a, int b)", + "@noinline function side(int k) returns int", + " trace = trace * 5 + k", + " return trace mod 7", + "class It", + " int i = 0", + " int n", + " construct(int n)", + " this.n = n", + " function hasNext() returns bool", + " return i < n", + " function next() returns int", + " i++", + " return i", + " function close()", + " closed++", + " destroy this", + "class Range", + " int n", + " construct(int n)", + " this.n = n", + " function iterator() returns It", + " return new It(n)", + "@noinline function scanRef(int x) returns int", + " for i = 0 to 5", + " if i == 1", + " continue", + " trace = trace * 3 + i", + " if i == x", + " return i * 10 + side(i)", + " if trace > 400", + " break", + " for j = 0 to 2", + " if j == x", + " break", + " trace = trace * 3 + 7", + " return -side(x)", + "@inline function scanInl(int x) returns int", + " for i = 0 to 5", + " if i == 1", + " continue", + " trace = trace * 3 + i", + " if i == x", + " return i * 10 + side(i)", + " if trace > 400", + " break", + " for j = 0 to 2", + " if j == x", + " break", + " trace = trace * 3 + 7", + " return -side(x)", + "@noinline function tupRef(int x) returns pair", + " for i = 0 to 3", + " if i == x", + " return pair(i, side(i))", + " trace = trace * 3 + i", + " return pair(-1, side(9))", + "@inline function tupInl(int x) returns pair", + " for i = 0 to 3", + " if i == x", + " return pair(i, side(i))", + " trace = trace * 3 + i", + " return pair(-1, side(9))", + "@noinline function swRef(int x) returns int", + " for i = 0 to 4", + " switch (i + x) mod 4", + " case 0", + " trace = trace * 3 + 1", + " case 1", + " if i > 1", + " return i * 100 + side(x)", + " trace = trace * 3 + 2", + " case 2", + " continue", + " default", + " if x > 3", + " break", + " trace = trace * 3 + 3", + " trace += 1", + " return -side(x)", + "@inline function swInl(int x) returns int", + " for i = 0 to 4", + " switch (i + x) mod 4", + " case 0", + " trace = trace * 3 + 1", + " case 1", + " if i > 1", + " return i * 100 + side(x)", + " trace = trace * 3 + 2", + " case 2", + " continue", + " default", + " if x > 3", + " break", + " trace = trace * 3 + 3", + " trace += 1", + " return -side(x)", + "@noinline function findRef(Range r, int x) returns int", + " for v in r", + " if v == x", + " return v * 10 + side(v)", + " trace = trace * 3 + v", + " return -1", + "@inline function findInl(Range r, int x) returns int", + " for v in r", + " if v == x", + " return v * 10 + side(v)", + " trace = trace * 3 + v", + " return -1", + "@noinline function innerRef(int x) returns int", + " for i = 0 to 2", + " if i == x", + " return side(i) + 1", + " if x > 5", + " return 7", + " return side(x)", + "@inline function innerInl(int x) returns int", + " for i = 0 to 2", + " if i == x", + " return side(i) + 1", + " if x > 5", + " return 7", + " return side(x)", + "@noinline function outerRef(int x) returns int", + " if x < 0", + " return innerRef(-x) * 2", + " for j = 0 to 1", + " if innerRef(x + j) == 3", + " return innerRef(j) + innerRef(x)", + " return innerRef(x + 1) - innerRef(x)", + "@inline function outerInl(int x) returns int", + " if x < 0", + " return innerInl(-x) * 2", + " for j = 0 to 1", + " if innerInl(x + j) == 3", + " return innerInl(j) + innerInl(x)", + " return innerInl(x + 1) - innerInl(x)", + "@noinline function voidRef(int x)", + " for i = 0 to 3", + " if i == x", + " return", + " if i == 2", + " continue", + " trace = trace * 3 + i", + " trace = trace * 3 + 9", + "@inline function voidInl(int x)", + " for i = 0 to 3", + " if i == x", + " return", + " if i == 2", + " continue", + " trace = trace * 3 + i", + " trace = trace * 3 + 9", + "@noinline function run(int which, int x, bool inl) returns int", + " if which == 0", + " return inl ? scanInl(x) : scanRef(x)", + " if which == 1", + " pair p = inl ? tupInl(x) : tupRef(x)", + " return p.a * 100 + p.b", + " if which == 2", + " return inl ? swInl(x) : swRef(x)", + " if which == 3", + " let r = new Range(4)", + " int res = inl ? findInl(r, x) : findRef(r, x)", + " destroy r", + " return res", + " if which == 4", + " return inl ? outerInl(x) : outerRef(x)", + " for k = 0 to 2", + " if inl", + " voidInl(x + k)", + " else", + " voidRef(x + k)", + " if k == x", + " break", + " return 0", + "init", + " for which = 0 to 5", + " for x = -2 to 6", + " trace = 0", + " closed = 0", + " int r1 = run(which, x, false)", + " int t1 = trace", + " int c1 = closed", + " trace = 0", + " closed = 0", + " int r2 = run(which, x, true)", + " if r1 != r2 or t1 != trace or c1 != closed", + " failures++", + " if failures == 0", + " testSuccess()"); + } } From 3fb29a2d2de934c65f7ba4760551af8c3d750818 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 19:30:11 +0200 Subject: [PATCH 14/19] LanguageWorkerTest: wait up to 60 s for the completion behind the initial build The first case waits for a completion which comes after the server's initial build; on a cold JVM it took 8 s of the 10 s limit and timed out once in a run of many test classes. The limit only guards against a hang. --- .../de/peeeq/wurstio/languageserver/LanguageWorkerTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/LanguageWorkerTest.java b/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/LanguageWorkerTest.java index 3259abf04..9982afac2 100644 --- a/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/LanguageWorkerTest.java +++ b/de.peeeq.wurstscript/src/test/java/de/peeeq/wurstio/languageserver/LanguageWorkerTest.java @@ -70,9 +70,11 @@ public void firstNativeResolveWaitsForColdDatabaseWithoutBlockingWorker(String s try { System.setProperty("WURST_JASSDOC_DB_PATH", database.toString()); worker.setRootPath(WFile.create(root)); + // only guards against a hang: the completion waits for the initial build, which takes 8 s when the JVM + // is cold and the machine busy CompletionList completions = service.completion(new CompletionParams( new TextDocumentIdentifier(source.toUri().toString()), new Position(2, 4 + prefix.length()))) - .get(10, TimeUnit.SECONDS).getRight(); + .get(60, TimeUnit.SECONDS).getRight(); CompletionItem item = completions.getItems().stream().filter(candidate -> symbol.equals(candidate.getLabel())) .findFirst().orElseThrow(); java.util.concurrent.CompletableFuture pending; From 0616fa4518b2bba2514e4903afde7fb5023e73f0 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 19:50:19 +0200 Subject: [PATCH 15/19] Interfaces: give a class its own method for an abstract method it inherits C extends Base implements Omega, with Omega's m abstract and Base, which is not an Omega, declaring m: the Jass dispatch over Omega's implementations follows the classes below Omega and takes the method declared in each, so C kept the abstract method and landed in another implementor's branch, and the interpreter called the abstract method. C gets a method of its own with Base's implementation now. --- .../imtranslation/InterfaceTranslator.java | 17 ++++++++++++ .../wurstscript/tests/InterfaceTests.java | 26 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java index 8e66563cc..616f6b7f0 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java @@ -103,11 +103,28 @@ private void translateInterfaceFuncDef(FuncDef f) { ImMethod m = translator.getMethodFor(subM); ImClass mClass = translator.getClassFor(subC); + if (f.attrHasEmptyBody() && !subClasses.contains(subM.attrNearestClassDef())) { + m = methodOfItsOwn(mClass, m); + } OverrideUtils.addOverride(translator, f, mClass, m, subM, typeBinding); } } + /** + * A method of {@code imClass} itself for the abstract method of this interface, which the class implements with a + * method it inherits from a class outside the interface ({@code C extends Base implements Omega}). The dispatch + * over the implementations of the interface method follows the classes below the interface and takes at each the + * method declared in that class, and so does the interpreter: Base is not on that path, so C gets a method of its + * own with Base's implementation, which its subclasses inherit. + */ + private ImMethod methodOfItsOwn(ImClass imClass, ImMethod inherited) { + ImMethod own = JassIm.ImMethod(inherited.getTrace(), translator.selfType(imClass), inherited.getName(), + inherited.getImplementation(), JassIm.ImMethods(), new java.util.ArrayList<>(), "", false); + imClass.getMethods().add(own); + return own; + } + /** * The type arguments a class gives this interface where it implements it: itself, through an interface which * extends this one, or through a superclass (the instances are the subclasses of an implementing class too). The diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java index 84bb5e3f7..9c48b5555 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java @@ -612,4 +612,30 @@ public void testEmptyImplements() { } + + /** + * C implements Omega's abstract m with the m it inherits from Base, which is not an Omega. The dispatch over + * Omega's implementations reaches C (and D below it) and must call Base's m, not the one of another implementor. + */ + @Test + public void anAbstractInterfaceMethodImplementedByAnInheritedMethodIsDispatched() { + test().testLua(true).luaOnly(false).executeProg().lines( + "package test", + "native testSuccess()", + "interface Omega", + " function m() returns int", + "class Base", + " function m() returns int", + " return 1", + "class C extends Base implements Omega", + "class D extends C", + "class X implements Omega", + " function m() returns int", + " return 3", + "function viaOmega(Omega a) returns int", + " return a.m()", + "init", + " if viaOmega(new C()) == 1 and viaOmega(new D()) == 1 and viaOmega(new X()) == 3 and new C().m() == 1", + " testSuccess()"); + } } From 4ffb32a5f0ac5b5cbef7e34fa33dd12ecb96abcd Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 20:08:11 +0200 Subject: [PATCH 16/19] Interfaces: link a class's own method to the overrides below it The method a class gets of its own for an abstract interface method it inherits had no sub-methods, so a deeper override was reachable only through the interface method, not from the bridge (AGENTS.md section 8). Once every class is translated, the bridge takes the overrides of the inherited method below its class. Generic classes keep what they did: their methods are specialised with functions the class owns, and a generic class with an override in a non-generic subclass does not compile yet. --- .../imtranslation/ImTranslator.java | 28 +++++++++ .../imtranslation/InterfaceTranslator.java | 12 +++- .../wurstscript/tests/InterfaceTests.java | 59 +++++++++++++++++++ 3 files changed, 96 insertions(+), 3 deletions(-) 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 b2d6b00d4..10796d301 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 @@ -422,6 +422,7 @@ public ImProg translateProg() { for (CompilationUnit cu : wurstProg) { translateCompilationUnit(cu); } + linkBridgedOverrides(); if (mainFunc == null) { mainFunc = ImFunction(emptyTrace, "main", ImTypeVars(), ImVars(), ImVoid(), ImVars(), ImStmts(), flags()); @@ -455,6 +456,33 @@ public ImProg translateProg() { } } + /** A method a class has of its own for a method it inherits ({@code InterfaceTranslator}), and that method. */ + private record Bridge(ImMethod bridge, ImMethod inherited) { + } + + private final List bridges = new ArrayList<>(); + + /** + * Gives {@code bridge} the overrides of {@code inherited} which are below its class as sub-methods, once every + * class is translated: the sub-methods of {@code inherited} come from the translation of its class, which may come + * later. Then a deeper override stays reachable from the bridge, as AGENTS.md section 8 asks of a bridge. + */ + public void linkOverridesBelow(ImMethod bridge, ImMethod inherited) { + bridges.add(new Bridge(bridge, inherited)); + } + + private void linkBridgedOverrides() { + for (Bridge b : bridges) { + ImClass bridgeClass = b.bridge().attrClass(); + for (ImMethod sub : b.inherited().getSubMethods()) { + ImClass subClass = sub.attrClass(); + if (subClass != bridgeClass && subClass.isSubclassOf(bridgeClass)) { + b.bridge().getSubMethods().add(sub); + } + } + } + } + public void removeEmptyPackageInits() { Set emptyInitFunctions = new HashSet<>(); for (ImFunction initFunc : new LinkedHashSet<>(initFuncMap.values())) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java index 616f6b7f0..60f98790a 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java @@ -103,7 +103,8 @@ private void translateInterfaceFuncDef(FuncDef f) { ImMethod m = translator.getMethodFor(subM); ImClass mClass = translator.getClassFor(subC); - if (f.attrHasEmptyBody() && !subClasses.contains(subM.attrNearestClassDef())) { + if (f.attrHasEmptyBody() && !subClasses.contains(subM.attrNearestClassDef()) + && mClass.getTypeVariables().isEmpty() && m.attrClass().getTypeVariables().isEmpty()) { m = methodOfItsOwn(mClass, m); } OverrideUtils.addOverride(translator, f, mClass, m, subM, typeBinding); @@ -116,15 +117,20 @@ private void translateInterfaceFuncDef(FuncDef f) { * method it inherits from a class outside the interface ({@code C extends Base implements Omega}). The dispatch * over the implementations of the interface method follows the classes below the interface and takes at each the * method declared in that class, and so does the interpreter: Base is not on that path, so C gets a method of its - * own with Base's implementation, which its subclasses inherit. + * own with Base's implementation, which its subclasses inherit. The overrides below C are its sub-methods, as they + * are Base's ({@link ImTranslator#linkOverridesBelow}). Not for generic classes: a method of a generic class is + * specialised with the functions the class owns, and a generic class with an override in a non-generic subclass + * does not compile yet (EliminateGenerics.adaptSubmethods), so those keep what they did. */ private ImMethod methodOfItsOwn(ImClass imClass, ImMethod inherited) { ImMethod own = JassIm.ImMethod(inherited.getTrace(), translator.selfType(imClass), inherited.getName(), - inherited.getImplementation(), JassIm.ImMethods(), new java.util.ArrayList<>(), "", false); + inherited.getImplementation(), Lists.newArrayList(), new java.util.ArrayList<>(), "", false); imClass.getMethods().add(own); + translator.linkOverridesBelow(own, inherited); return own; } + /** * The type arguments a class gives this interface where it implements it: itself, through an interface which * extends this one, or through a superclass (the instances are the subclasses of an implementing class too). The diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java index 9c48b5555..e5be7b221 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java @@ -638,4 +638,63 @@ public void anAbstractInterfaceMethodImplementedByAnInheritedMethodIsDispatched( " if viaOmega(new C()) == 1 and viaOmega(new D()) == 1 and viaOmega(new X()) == 3 and new C().m() == 1", " testSuccess()"); } + + private static final String[] OVERRIDE_BELOW_THE_INHERITING_CLASS = { + "package test", + "native testSuccess()", + "interface Omega", + " function m() returns int", + "class Base", + " function m() returns int", + " return 1", + "class C extends Base implements Omega", + "class D extends C", + " override function m() returns int", + " return 4", + "class E extends D", + "class X implements Omega", + " function m() returns int", + " return 3", + "function viaOmega(Omega a) returns int", + " return a.m()", + "function viaBase(Base b) returns int", + " return b.m()", + "init", + " if viaOmega(new C()) == 1 and viaOmega(new D()) == 4 and viaOmega(new E()) == 4 and viaOmega(new X()) == 3", + " and viaBase(new D()) == 4 and viaBase(new C()) == 1", + " testSuccess()"}; + + /** An override below the class which inherits the implementation is reached through the interface too. */ + @Test + public void anOverrideBelowTheClassWhichInheritsTheImplementationIsDispatched() { + test().testLua(true).luaOnly(false).executeProg().lines(OVERRIDE_BELOW_THE_INHERITING_CLASS); + } + + /** + * The method C has of its own for the m it inherits keeps D's override as a sub-method, as Base's m does: a + * bridge stays linked to the overrides below it (AGENTS.md section 8), which the dispatch preparation and the + * specialisation follow. + */ + @Test + public void theMethodAClassHasOfItsOwnKeepsTheOverridesBelowIt() { + de.peeeq.wurstscript.gui.WurstGuiCliImpl gui = new de.peeeq.wurstscript.gui.WurstGuiCliImpl(); + de.peeeq.wurstio.WurstCompilerJassImpl compiler = + new de.peeeq.wurstio.WurstCompilerJassImpl(null, gui, null, new de.peeeq.wurstscript.RunArgs()); + de.peeeq.wurstscript.ast.WurstModel model = parseFiles(java.util.Collections.emptyList(), + java.util.Collections.singletonList(new CU("InterfaceTests.wurst", String.join("\n", OVERRIDE_BELOW_THE_INHERITING_CLASS))), + false, compiler); + compiler.checkProg(model); + org.testng.Assert.assertTrue(gui.getErrorList().isEmpty(), gui.getErrorList().toString()); + compiler.translateProgToIm(model); + java.util.Map classes = new java.util.HashMap<>(); + for (de.peeeq.wurstscript.jassIm.ImClass c : compiler.getImProg().getClasses()) { + classes.put(c.getName(), c); + } + de.peeeq.wurstscript.jassIm.ImMethod baseM = classes.get("Base").getMethods().stream() + .filter(m -> !m.getName().startsWith("destroy")).findFirst().orElseThrow(); + de.peeeq.wurstscript.jassIm.ImMethod bridge = classes.get("C").getMethods().stream() + .filter(m -> m.getImplementation() == baseM.getImplementation()).findFirst().orElseThrow(); + org.testng.Assert.assertTrue(bridge.getSubMethods().stream().anyMatch(s -> s.attrClass() == classes.get("D")), + "C's method for Base's m is linked to D's override: " + bridge.getSubMethods()); + } } From e8baa23997af66f90cb2a8c8a1d3d87ee94f0019 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 20:27:56 +0200 Subject: [PATCH 17/19] Local merger: replace a dead assignment by flat statements It put what a dead assignment's value does in a statement expression, so the passes after it saw non-flat IM: a call (as before) and now a division which may stop the thread inside an expression. The effects become the statements a flatten makes of them; when that makes a local (the division), the liveness is computed again before the merge. --- .../optimizer/LocalMerger.java | 33 ++++++++--- .../wurstscript/tests/OptimizerTests.java | 58 +++++++++++++++++++ 2 files changed, 82 insertions(+), 9 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java index 388d8a53d..5bfec2ce1 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java @@ -57,9 +57,12 @@ void optimizeFunc(ImFunction func) { } liveness = analyzeLiveness(func); } - Map> livenessInfo = liveness.liveOut; - eliminateDeadCode(livenessInfo); - mergeLocals(livenessInfo, liveness.liveAtEntry, func); + if (eliminateDeadCode(liveness.liveOut, func)) { + // a flatten of an effect which replaced a dead assignment made a local, and an assignment to it which the + // liveness has not seen + liveness = analyzeLiveness(func); + } + mergeLocals(liveness.liveOut, liveness.liveAtEntry, func); } void optimizeFunc(ImFunction func, LocalPlayerContextAnalyzer analyzer) { @@ -281,7 +284,15 @@ private static List definedLocals(ImStmt stmt) { return Collections.emptyList(); } - private void eliminateDeadCode(Map> livenessInfo) { + /** + * Removes the assignments to locals which are dead after them, and puts what their value and target do besides + * that in their place. + * + * @return whether the statements which replaced an assignment assign a local which the flatten of an effect made + * (the assignment of a division which may stop the thread) + */ + private boolean eliminateDeadCode(Map> livenessInfo, ImFunction func) { + int locals = func.getLocals().size(); for (ImStmt s : livenessInfo.keySet()) { if (!(s instanceof ImSet set)) continue; @@ -312,16 +323,20 @@ private void eliminateDeadCode(Map> livenessInfo) { if (raw.isEmpty()) { AstEdits.deleteStmt(s); // remove the dead assignment entirely } else { - ImStmts block = JassIm.ImStmts(); - for (int i = 0; i < raw.size(); i++) { - ImExpr e = raw.get(i); - // wrap expression as a statement; add a *copy* to avoid re-parenting conflicts - block.add(ImHelper.statementExprVoid(e.copy())); + // The effects become the statements a flatten makes of them (a call statement, an assignment of + // a division which may stop the thread, an if for and/or), not statement expressions: the local + // optimisations work on flat IM (AGENTS.md section 7). A copy, to avoid re-parenting conflicts. + List statements = new ArrayList<>(); + for (ImExpr e : raw) { + e.copy().flatten(translator, func).intoStatements(statements, translator, func); } + ImStmts block = JassIm.ImStmts(); + block.addAll(statements); AstEdits.replaceStmtWithMany(s, block); // removes 's', then inserts the new stmts } } } + return func.getLocals().size() != locals; } private void collectLhsSideEffects(ImLExpr lhs, List out) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 1b9e7254f..763b64134 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -2471,6 +2471,64 @@ public void visit(ImVarAccess access) { assertTrue(f.getLocals().isEmpty(), "the local nothing reads any more: " + f.getLocals()); } + /** + * The local merger replaces a dead assignment by what its value does besides producing it, and the IM stays flat + * while the local optimisations run: a call becomes a call statement and a division which may stop the thread + * inside an expression an assignment of it, not a statement expression. + */ + @Test + public void localMergerKeepsTheEffectsOfADeadAssignmentAsFlatStatements() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, new RunArgs()); + ImProg prog = translator.getImProg(); + ImFunction tick = JassIm.ImFunction(model, "tick", JassIm.ImTypeVars(), JassIm.ImVars(), TypesHelper.imInt(), + JassIm.ImVars(), JassIm.ImStmts(), Collections.singletonList(FunctionFlagEnum.IS_NATIVE)); + ImVar d = JassIm.ImVar(model, TypesHelper.imInt(), "d", false); + ImVar x = JassIm.ImVar(model, TypesHelper.imInt(), "x", false); + ImVar y = JassIm.ImVar(model, TypesHelper.imInt(), "y", false); + ImExpr division = JassIm.ImOperatorCall(de.peeeq.wurstscript.WurstOperator.DIV_INT, + JassIm.ImExprs(JassIm.ImIntVal(10), JassIm.ImVarAccess(d))); + ImFunction f = JassIm.ImFunction(model, "f", JassIm.ImTypeVars(), JassIm.ImVars(d), TypesHelper.imInt(), + JassIm.ImVars(x, y), + JassIm.ImStmts( + JassIm.ImSet(model, JassIm.ImVarAccess(x), JassIm.ImOperatorCall(de.peeeq.wurstscript.WurstOperator.PLUS, + JassIm.ImExprs(JassIm.ImIntVal(1), division))), + JassIm.ImSet(model, JassIm.ImVarAccess(y), + JassIm.ImFunctionCall(model, tick, JassIm.ImTypeArguments(), JassIm.ImExprs(), false, CallType.NORMAL)), + JassIm.ImReturn(model, JassIm.ImIntVal(0))), + Collections.emptyList()); + prog.getFunctions().add(tick); + prog.getFunctions().add(f); + + new LocalMerger().optimize(translator, new LocalPlayerContextAnalyzer(prog)); + + List statementExprs = new ArrayList<>(); + boolean[] divides = {false}; + boolean[] calls = {false}; + f.accept(new ImFunction.DefaultVisitor() { + @Override + public void visit(ImStatementExpr e) { + super.visit(e); + statementExprs.add(e); + } + + @Override + public void visit(ImOperatorCall e) { + super.visit(e); + divides[0] |= e.getOp() == de.peeeq.wurstscript.WurstOperator.DIV_INT; + } + + @Override + public void visit(ImFunctionCall e) { + super.visit(e); + calls[0] |= e.getFunc() == tick; + } + }); + assertTrue(statementExprs.isEmpty(), "the IM stays flat: " + f.getBody()); + assertTrue(divides[0], "the division which may stop the thread stays: " + f.getBody()); + assertTrue(calls[0], "the call stays: " + f.getBody()); + } + @Test public void aFlattenLeavesTheFunctionsWhichWereNotModifiedSinceTheLastOne() { WurstModel model = Ast.WurstModel(); From ecfc95e0134f06948bd65d65a6e3a05a15382af4 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 20:50:37 +0200 Subject: [PATCH 18/19] Interfaces: a class's own method runs the default another interface gives it C extends Base implements Abstract, Default, with Abstract's m abstract and Default's m a default: a default beats an inherited method, and Lua binds one implementation for C to the calls through both interfaces, so with C's own method running Base's m a call through Default ran Base's m on Lua and the default on Jass. C's own method runs the default now, so every backend runs it through either interface. --- .../imtranslation/InterfaceTranslator.java | 60 +++++++++++++++++-- .../wurstscript/tests/InterfaceTests.java | 31 ++++++++++ 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java index 60f98790a..bcf4567d7 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/InterfaceTranslator.java @@ -105,7 +105,9 @@ private void translateInterfaceFuncDef(FuncDef f) { ImClass mClass = translator.getClassFor(subC); if (f.attrHasEmptyBody() && !subClasses.contains(subM.attrNearestClassDef()) && mClass.getTypeVariables().isEmpty() && m.attrClass().getTypeVariables().isEmpty()) { - m = methodOfItsOwn(mClass, m); + FuncDef interfaceDefault = defaultOf(subCT, f); + m = methodOfItsOwn(mClass, m, interfaceDefault == null + ? m.getImplementation() : translator.getFuncFor(interfaceDefault)); } OverrideUtils.addOverride(translator, f, mClass, m, subM, typeBinding); } @@ -118,19 +120,65 @@ private void translateInterfaceFuncDef(FuncDef f) { * over the implementations of the interface method follows the classes below the interface and takes at each the * method declared in that class, and so does the interpreter: Base is not on that path, so C gets a method of its * own with Base's implementation, which its subclasses inherit. The overrides below C are its sub-methods, as they - * are Base's ({@link ImTranslator#linkOverridesBelow}). Not for generic classes: a method of a generic class is - * specialised with the functions the class owns, and a generic class with an override in a non-generic subclass - * does not compile yet (EliminateGenerics.adaptSubmethods), so those keep what they did. + * are Base's ({@link ImTranslator#linkOverridesBelow}). Where another interface of C gives m a default, the + * default is the implementation: a default beats an inherited method (a call through that interface runs it on + * every backend), and Lua binds one implementation for C to both. Not for generic classes: a method of a generic + * class is specialised with the functions the class owns, and a generic class with an override in a non-generic + * subclass does not compile yet (EliminateGenerics.adaptSubmethods), so those keep what they did. */ - private ImMethod methodOfItsOwn(ImClass imClass, ImMethod inherited) { + private ImMethod methodOfItsOwn(ImClass imClass, ImMethod inherited, ImFunction implementation) { ImMethod own = JassIm.ImMethod(inherited.getTrace(), translator.selfType(imClass), inherited.getName(), - inherited.getImplementation(), Lists.newArrayList(), new java.util.ArrayList<>(), "", false); + implementation, Lists.newArrayList(), new java.util.ArrayList<>(), "", false); imClass.getMethods().add(own); translator.linkOverridesBelow(own, inherited); return own; } + /** + * The default which another interface of the class gives {@code abstractMethod}: a method of an interface which + * is not generic, with the same name and parameter types and a body, the nearest first (the class's own + * interfaces before its superclass's, as in {@link #typeBindingOf}); null if there is none. + */ + private static @org.eclipse.jdt.annotation.Nullable FuncDef defaultOf(WurstTypeClass classType, FuncDef abstractMethod) { + ArrayDeque queue = new ArrayDeque<>(); + queue.add(classType); + while (!queue.isEmpty()) { + WurstTypeClassOrInterface type = queue.removeFirst(); + if (type instanceof WurstTypeInterface i && i.getDef().getTypeParameters().isEmpty()) { + for (FuncDef candidate : i.getDef().getMethods()) { + if (candidate != abstractMethod && !candidate.attrHasEmptyBody() + && candidate.getName().equals(abstractMethod.getName()) + && sameParameterTypes(candidate, abstractMethod)) { + return candidate; + } + } + } + if (type instanceof WurstTypeClass c) { + queue.addAll(c.implementedInterfaces()); + WurstTypeClass extended = c.extendedClass(); + if (extended != null) { + queue.add(extended); + } + } else { + queue.addAll(type.directSupertypes()); + } + } + return null; + } + + private static boolean sameParameterTypes(FuncDef a, FuncDef b) { + if (a.getParameters().size() != b.getParameters().size()) { + return false; + } + for (int i = 0; i < a.getParameters().size(); i++) { + if (!a.getParameters().get(i).attrTyp().equalsType(b.getParameters().get(i).attrTyp(), a)) { + return false; + } + } + return true; + } + /** * The type arguments a class gives this interface where it implements it: itself, through an interface which * extends this one, or through a superclass (the instances are the subclasses of an implementing class too). The diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java index e5be7b221..cc6064a53 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java @@ -697,4 +697,35 @@ public void theMethodAClassHasOfItsOwnKeepsTheOverridesBelowIt() { org.testng.Assert.assertTrue(bridge.getSubMethods().stream().anyMatch(s -> s.attrClass() == classes.get("D")), "C's method for Base's m is linked to D's override: " + bridge.getSubMethods()); } + + /** + * C gets m from Base and a default m from Default, and Abstract declares m without a body. A default beats an + * inherited method, so C's m is the default through either interface (Lua binds one implementation for C to + * both), and D's override below C beats both. Calls through both interfaces are in the program, so Lua keeps both. + */ + @Test + public void aDefaultBeatsTheInheritedMethodWhereAnotherInterfaceHasNoBody() { + test().testLua(true).luaOnly(false).executeProg().lines( + "package test", + "native testSuccess()", + "interface Abstract", + " function m() returns int", + "interface Default", + " function m() returns int", + " return 2", + "class Base", + " function m() returns int", + " return 1", + "class C extends Base implements Abstract, Default", + "class D extends C", + " override function m() returns int", + " return 4", + "function viaDefault(Default a) returns int", + " return a.m()", + "function viaAbstract(Abstract a) returns int", + " return a.m()", + "init", + " if viaDefault(new C()) == 2 and viaDefault(new D()) == 4 and viaAbstract(new C()) == 2 and viaAbstract(new D()) == 4", + " testSuccess()"); + } } From bc562498564c3a2f09fe6fa67d2b4121c5bbdff3 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 9 Oct 2026 20:50:38 +0200 Subject: [PATCH 19/19] Local merger: drop the assignment of a local to itself which a merge makes Merging two locals turns the copy between them into an assignment of the local to itself. The next run of the merger removed those, but nothing after the last run did: castle fight's optimised Lua had 103, zombie defense's 106. The merge removes them now. --- .../optimizer/LocalMerger.java | 22 ++++++++++++ .../wurstscript/tests/OptimizerTests.java | 35 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java index 5bfec2ce1..d57ca01a4 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java @@ -135,10 +135,32 @@ private void mergeLocals(Map> livenessInfo, Set liveAt } applyMerges(func, merges); + removeSelfAssignments(func, merges); int removed = removeUnusedLocals(func); totalLocalsMerged += removed; } + /** + * A copy between two locals which were merged is an assignment of the local to itself now, which does nothing. + * No pass after the last merge would remove it, so the script kept it. + */ + private static void removeSelfAssignments(ImFunction func, Map merges) { + if (merges.isEmpty()) return; + List selfAssignments = new ArrayList<>(); + func.accept(new ImFunction.DefaultVisitor() { + @Override public void visit(ImSet set) { + super.visit(set); + if (set.getLeft() instanceof ImVarAccess left && set.getRight() instanceof ImVarAccess right + && left.getVar() == right.getVar()) { + selfAssignments.add(set); + } + } + }); + for (ImSet set : selfAssignments) { + AstEdits.deleteStmt(set); + } + } + private static void applyMerges(ImFunction func, Map merges) { if (merges.isEmpty()) return; diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java index 763b64134..93e3c1b58 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/OptimizerTests.java @@ -2471,6 +2471,41 @@ public void visit(ImVarAccess access) { assertTrue(f.getLocals().isEmpty(), "the local nothing reads any more: " + f.getLocals()); } + /** + * Merging two locals makes the copy between them an assignment of the local to itself, which does nothing and + * goes: `a = tick(); b = a; return b` keeps no `a = a`. + */ + @Test + public void localMergerLeavesNoAssignmentOfALocalToItself() { + WurstModel model = Ast.WurstModel(); + ImTranslator translator = new ImTranslator(model, false, new RunArgs()); + ImProg prog = translator.getImProg(); + ImFunction tick = JassIm.ImFunction(model, "tick", JassIm.ImTypeVars(), JassIm.ImVars(), TypesHelper.imInt(), + JassIm.ImVars(), JassIm.ImStmts(), Collections.singletonList(FunctionFlagEnum.IS_NATIVE)); + ImVar a = JassIm.ImVar(model, TypesHelper.imInt(), "a", false); + ImVar b = JassIm.ImVar(model, TypesHelper.imInt(), "b", false); + ImFunction f = JassIm.ImFunction(model, "f", JassIm.ImTypeVars(), JassIm.ImVars(), TypesHelper.imInt(), + JassIm.ImVars(a, b), + JassIm.ImStmts( + JassIm.ImSet(model, JassIm.ImVarAccess(a), + JassIm.ImFunctionCall(model, tick, JassIm.ImTypeArguments(), JassIm.ImExprs(), false, CallType.NORMAL)), + JassIm.ImSet(model, JassIm.ImVarAccess(b), JassIm.ImVarAccess(a)), + JassIm.ImReturn(model, JassIm.ImVarAccess(b))), + Collections.emptyList()); + prog.getFunctions().add(tick); + prog.getFunctions().add(f); + + new LocalMerger().optimize(translator, new LocalPlayerContextAnalyzer(prog)); + + assertEquals(f.getLocals().size(), 1, "a and b are merged: " + f.getLocals()); + for (ImStmt s : f.getBody()) { + assertFalse(s instanceof ImSet set && set.getLeft() instanceof ImVarAccess left + && set.getRight() instanceof ImVarAccess right && left.getVar() == right.getVar(), + "an assignment of a local to itself is left: " + f.getBody()); + } + assertEquals(f.getBody().size(), 2, "the call and the return: " + f.getBody()); + } + /** * The local merger replaces a dead assignment by what its value does besides producing it, and the IM stays flat * while the local optimisations run: a call becomes a call statement and a division which may stop the thread