Fix incorrect lua type merging, loop var mutation error downgraded to warning (#1170)

Author: Frotty

README.markdown

@@ -2,8 +2,8 @@
 
 Wurstscript is a delicious programming language which compiles to Jass or Lua code that is used to power WarCraft III maps.
 
-[![Build Status](https://grill.wurstlang.org/hudson/job/Wurst/badge/icon)](http://grill.wurstlang.org/hudson/job/Wurst/)
-[![CircleCI](https://dl.circleci.com/status-badge/img/gh/wurstscript/WurstScript/tree/master.svg?style=svg)](https://dl.circleci.com/status-badge/redirect/gh/wurstscript/WurstScript/tree/master)
+[![Build](https://github.com/wurstscript/WurstScript/actions/workflows/build.yml/badge.svg)](https://github.com/wurstscript/WurstScript/actions/workflows/build.yml)
+[![Release](https://github.com/wurstscript/WurstScript/actions/workflows/release.yml/badge.svg)](https://github.com/wurstscript/WurstScript/actions/workflows/release.yml)
 [![GitHub issues](https://img.shields.io/github/issues/wurstscript/WurstScript.svg)]()
 [![GitHub pull requests](https://img.shields.io/github/issues-pr/wurstscript/WurstScript.svg)]()
 [![Coverage Status](https://coveralls.io/repos/github/wurstscript/WurstScript/badge.svg?branch=master)](https://coveralls.io/github/wurstscript/WurstScript?branch=master)
@@ -12,6 +12,7 @@ Wurstscript is a delicious programming language which compiles to Jass or Lua co
 ## User Documentation
 
 Using WurstScript to build a map is easy! Check out the [Setup Guide](https://wurstscript.github.io/start.html) on how to get started.
+For users, installation is automated and intended to work out of the box via the official setup.
 For a formal description of all language features, visit the [Manual](https://wurstscript.github.io/manual.html).
 
 Consider joining the WurstScript community on [Discord](https://discord.gg/mSHZpWcadz).
@@ -45,7 +46,10 @@ The source for the wurstscript website can be found here: https://github.com/wur
 
 ## Compiler Build Process
 
-Java 11+ is required to build the project. Clone the repository and open the `de.peeeq.wurstscript` folder which contains the compiler project.
+For **contributing/developing the compiler**, Java 25 is required.  
+End users normally do not need to install/configure Java manually for the standard Wurst setup flow.
+
+Clone the repository and open the `de.peeeq.wurstscript` folder which contains the compiler project.
 
 ### Using Gradle
 
@@ -78,6 +82,8 @@ To run the Test Suite, execute `AllTests.xml` with TestNG.
 
 ### Publishing a new release
 
-[Jenkins](http://peeeq.de/hudson/job/Wurst/) auto-releases versions as `major.minor.patch.hotfix-jenkins-Wurst-buildNumber` - e.g. `1.8.1.0-jenkins-Wurst-1248`.
+Releases are published via GitHub workflows and GitHub Releases:
+- Workflow: https://github.com/wurstscript/WurstScript/actions/workflows/release.yml
+- Releases: https://github.com/wurstscript/WurstScript/releases
 
 The version string can be updated in [build.gradle](https://github.com/wurstscript/WurstScript/blob/master/de.peeeq.wurstscript/build.gradle#L28).

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jurst/AntlrJurstParseTreeTransformer.java

@@ -783,7 +783,7 @@ private WStatement transformForLoop(StmtForLoopContext s) {
     private WStatement transformForRangeLoop(ForRangeLoopContext s) {
         WPos source = source(s);
         Expr start = transformExpr(s.start);
-        LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, true);
+        LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, false);
         Expr to = transformExpr(s.end);
         Expr step;
         if (s.step == null) {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/antlr/AntlrWurstParseTreeTransformer.java

@@ -947,7 +947,7 @@ private WStatement transformForLoop(StmtForLoopContext s) {
     private WStatement transformForRangeLoop(ForRangeLoopContext s) {
         WPos source = source(s);
         Expr start = transformExpr(s.start);
-        LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, true);
+        LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, false);
         Expr to = transformExpr(s.end);
         Expr step;
         if (s.step == null) {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateLocalTypes.java

@@ -6,6 +6,10 @@
 
 public class EliminateLocalTypes {
     private static final ImType localSimpleType = JassIm.ImSimpleType("localSimpleType");
+    private static final ImType localIntType = JassIm.ImSimpleType("localSimpleTypeInt");
+    private static final ImType localRealType = JassIm.ImSimpleType("localSimpleTypeReal");
+    private static final ImType localBoolType = JassIm.ImSimpleType("localSimpleTypeBool");
+    private static final ImType localStringType = JassIm.ImSimpleType("localSimpleTypeString");
 
     public static void eliminateLocalTypesProg(ImProg imProg, ImTranslator translator) {
         // While local types are still there, perform transformation, such that the lua translator does not need to know variable types
@@ -23,12 +27,29 @@ private static void eliminateLocalTypesFunc(ImFunction f, final ImTranslator tra
         for(ImVar local : f.getLocals()) {
             ImType t = local.getType();
             if(t instanceof ImSimpleType) {
-                // Simple types can always be merged.
-                local.setType(localSimpleType);
+                // Keep primitive domains separate so later local merging cannot
+                // unify e.g. number/bool/string temporaries into one slot.
+                local.setType(canonicalizeSimpleLocalType((ImSimpleType) t));
             }
         }
     }
 
+    private static ImType canonicalizeSimpleLocalType(ImSimpleType t) {
+        if (TypesHelper.isIntType(t)) {
+            return localIntType;
+        }
+        if (TypesHelper.isRealType(t)) {
+            return localRealType;
+        }
+        if (TypesHelper.isBoolType(t)) {
+            return localBoolType;
+        }
+        if (TypesHelper.isStringType(t)) {
+            return localStringType;
+        }
+        return localSimpleType;
+    }
+
     private static void transformProgram(ImProg imProg, ImTranslator translator) {
         imProg.accept(new Element.DefaultVisitor() {
             @Override

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/StackTraceInjector2.java

@@ -295,9 +295,14 @@ private int getStacktraceIndex(ImFunction f) {
      */
     private int getStacktraceIndex(ImFunctionCall c) {
         ImFunction f = c.getFunc();
-        int res = f.getParameters().size() - 1;
+        int res;
         if (f.hasFlag(FunctionFlagEnum.IS_VARARG)) {
-            res--;
+            // Keep stacktrace before vararg payload.
+            res = f.getParameters().size() - 2;
+        } else {
+            // Append for non-vararg calls based on actual call shape.
+            // This avoids relying on parameter-layout assumptions in lowered calls.
+            res = c.getArguments().size();
         }
         if (res < 0 || res > c.getArguments().size() + 1) {
             throw new CompileError(c, "Call " + c + " invalid index " + res + " for parameters " + f.getParameters() + " and isVararg = " + f.hasFlag(FunctionFlagEnum.IS_VARARG));
@@ -307,9 +312,16 @@ private int getStacktraceIndex(ImFunctionCall c) {
 
     private int getStacktraceIndex(ImMethodCall c) {
         ImFunction f = c.getMethod().getImplementation();
-        int res = f.getParameters().size() - 2; // subtract one for implicit parameter
+        int res;
         if (f.hasFlag(FunctionFlagEnum.IS_VARARG)) {
-            res--;
+            // For vararg methods keep the stacktrace argument before the vararg bucket.
+            // Method implementations normally include an implicit receiver as first parameter.
+            res = f.getParameters().size() - 3;
+        } else {
+            // For non-vararg methods append after existing explicit call arguments.
+            // This is robust even if a generated method implementation does not carry
+            // an implicit receiver parameter in its function signature.
+            res = c.getArguments().size();
         }
         if (res < 0 || res > c.getArguments().size() + 1) {
             throw new CompileError(c, "Call " + c + " invalid index " + res + " for parameters " + f.getParameters() + " and isVararg = " + f.hasFlag(FunctionFlagEnum.IS_VARARG));

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java

@@ -1333,6 +1333,12 @@ private void checkVarNotConstant(NameRef left, @Nullable NameLink link) {
             return;
         }
         NameDef var = link.getDef();
+        if (var != null && isForRangeLoopVar(var)) {
+            left.addWarning("Avoid mutating for-loop variable " + Utils.printElement(var)
+                + ": this can cause unexpected iteration side effects (skipped or repeated iterations). "
+                + "This will be an error in a future version.");
+            return;
+        }
         if (var != null && var.attrIsConstant()) {
             if (var instanceof GlobalVarDef) {
                 GlobalVarDef glob = (GlobalVarDef) var;
@@ -1345,6 +1351,13 @@ private void checkVarNotConstant(NameRef left, @Nullable NameLink link) {
         }
     }
 
+    private boolean isForRangeLoopVar(NameDef var) {
+        Element parent = var.getParent();
+        return parent instanceof StmtForRange
+            || parent instanceof StmtForRangeUp
+            || parent instanceof StmtForRangeDown;
+    }
+
     private boolean isInConstructor(Element e) {
         while (e != null) {
             if (e instanceof ConstructorDef) {

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/BugTests.java

@@ -919,8 +919,8 @@ public void forRangeStartReadsParameter() {
     }
 
     @Test
-    public void forRangeLoopVarIsImmutable() {
-        testAssertErrorsLines(false, "Cannot assign a new value to constant",
+    public void forRangeLoopVarMutationWarns() {
+        testAssertWarningsLines(false, "unexpected iteration side effects",
             "package test",
             "init",
             "    int stackPointer = 3",

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java

@@ -2,13 +2,21 @@
 
 import com.google.common.base.Charsets;
 import com.google.common.io.Files;
+import config.WurstProjectConfigData;
+import de.peeeq.wurstio.WurstCompilerJassImpl;
+import de.peeeq.wurstscript.RunArgs;
+import de.peeeq.wurstscript.ast.WurstModel;
+import de.peeeq.wurstscript.gui.WurstGui;
+import de.peeeq.wurstscript.gui.WurstGuiCliImpl;
+import de.peeeq.wurstscript.luaAst.LuaCompilationUnit;
 import de.peeeq.wurstscript.validation.GlobalCaches;
 import org.testng.annotations.Ignore;
 import org.testng.annotations.Test;
 
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -71,6 +79,27 @@ private void assertContainsRegex(String output, String regex) {
         assertTrue("Pattern must occur: " + regex, matcher.find());
     }
 
+    private String compileLuaWithRunArgs(String testName, boolean withStdLib, String... lines) {
+        RunArgs runArgs = new RunArgs().with("-lua", "-inline", "-localOptimizations", "-stacktraces");
+        WurstGui gui = new WurstGuiCliImpl();
+        WurstCompilerJassImpl compiler = new WurstCompilerJassImpl(null, gui, null, runArgs);
+        List<CU> inputs = Collections.singletonList(new CU(testName + ".wurst", String.join("\n", lines)));
+
+        WurstModel model = parseFiles(Collections.emptyList(), inputs, withStdLib, compiler);
+        assertNotNull("parse returned null model, errors = " + gui.getErrorList(), model);
+        assertTrue("unexpected parse/type errors: " + gui.getErrorList(), gui.getErrorList().isEmpty());
+        compiler.checkProg(model);
+        assertTrue("unexpected compile errors: " + gui.getErrorList(), gui.getErrorList().isEmpty());
+
+        compiler.translateProgToIm(model);
+        compiler.runCompiletime(new WurstProjectConfigData(), false, false);
+        LuaCompilationUnit luaCode = compiler.transformProgToLua();
+
+        StringBuilder sb = new StringBuilder();
+        luaCode.print(sb, 0);
+        return sb.toString();
+    }
+
     @Test
     public void testStdLib() throws IOException {
         test().testLua(true).withStdLib().lines(
@@ -87,6 +116,56 @@ public void testStdLib() throws IOException {
         assertFunctionBodyContains(compiled, "__wurst_LoadInteger", "return 0", true);
     }
 
+    @Test
+    public void stacktraceStringsNotInjectedIntoNumericComparisons() {
+        String compiled = compileLuaWithRunArgs(
+            "LuaTranslationTests_stacktraceStringsNotInjectedIntoNumericComparisons",
+            false,
+            "package Test",
+            "public tuple Vec2(real x, real y)",
+            "public tuple scanResult(boolean found, Vec2 pos)",
+            "class MyRect",
+            "    real minX",
+            "    real minY",
+            "    real maxX",
+            "    real maxY",
+            "    construct(real minX, real minY, real maxX, real maxY)",
+            "        this.minX = minX",
+            "        this.minY = minY",
+            "        this.maxX = maxX",
+            "        this.maxY = maxY",
+            "    function contains(Vec2 p) returns boolean",
+            "        return p.x > minX and p.x < maxX and p.y > minY and p.y < maxY",
+            "function gridAnchor(MyRect r, int dirX, int dirY, boolean isBehind) returns Vec2",
+            "    let ax = dirX >= 0 ? r.minX + (isBehind ? 0 : 64) : r.maxX + (isBehind ? 0 : 64)",
+            "    let ay = dirY >= 0 ? r.minY : r.maxY",
+            "    return Vec2(ax, ay)",
+            "function stepsAlongY(MyRect r, real startY, int dirY, real stepSize) returns int",
+            "    if dirY >= 0 and stepSize > 0 and r.maxY > startY",
+            "        return 1",
+            "    return 0",
+            "class BuildScanIterator",
+            "    private var scanWidth = 128.",
+            "    function tryRect(MyRect r, int dirX, int dirY) returns scanResult",
+            "        if r == null",
+            "            return scanResult(false, Vec2(0., 0.))",
+            "        let start = gridAnchor(r, dirX, dirY, true)",
+            "        if not r.contains(start)",
+            "            return scanResult(false, Vec2(0., 0.))",
+            "        let maxY = stepsAlongY(r, start.y, dirY, scanWidth)",
+            "        if maxY >= 0",
+            "            return scanResult(true, start)",
+            "        return scanResult(false, Vec2(0., 0.))",
+            "init",
+            "    let b = new BuildScanIterator",
+            "    b.tryRect(new MyRect(0., 0., 256., 256.), 1, 1)"
+        );
+        assertContainsRegex(compiled, "\"when calling contains");
+        assertDoesNotContainRegex(compiled, "\"when calling contains[^\"]*\"\\s*[<>]=?");
+        assertContainsRegex(compiled, "stepsAlongY\\(");
+        assertDoesNotContainRegex(compiled, "(?s)if not\\((\\w+)\\) then.*?stepsAlongY\\([^\\n]*,\\s*\\1\\s*,");
+    }
+
     @Test
     public void continueLoweringInLua() throws IOException {
         test().testLua(true).lines(