Skip to content

Commit 8fff60d

Browse files
FrottyFrotty
committed
for loop warning for now, inlining fixes
1 parent 0a260f9 commit 8fff60d

7 files changed

Lines changed: 135 additions & 10 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ private WStatement transformForLoop(StmtForLoopContext s) {
783783
private WStatement transformForRangeLoop(ForRangeLoopContext s) {
784784
WPos source = source(s);
785785
Expr start = transformExpr(s.start);
786-
LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, true);
786+
LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, false);
787787
Expr to = transformExpr(s.end);
788788
Expr step;
789789
if (s.step == null) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -947,7 +947,7 @@ private WStatement transformForLoop(StmtForLoopContext s) {
947947
private WStatement transformForRangeLoop(ForRangeLoopContext s) {
948948
WPos source = source(s);
949949
Expr start = transformExpr(s.start);
950-
LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, true);
950+
LocalVarDef loopVar = transformLocalVarDef(s.loopVar, start, false);
951951
Expr to = transformExpr(s.end);
952952
Expr step;
953953
if (s.step == null) {

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66

77
public class EliminateLocalTypes {
88
private static final ImType localSimpleType = JassIm.ImSimpleType("localSimpleType");
9+
private static final ImType localIntType = JassIm.ImSimpleType("localSimpleTypeInt");
10+
private static final ImType localRealType = JassIm.ImSimpleType("localSimpleTypeReal");
11+
private static final ImType localBoolType = JassIm.ImSimpleType("localSimpleTypeBool");
12+
private static final ImType localStringType = JassIm.ImSimpleType("localSimpleTypeString");
913

1014
public static void eliminateLocalTypesProg(ImProg imProg, ImTranslator translator) {
1115
// 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
2327
for(ImVar local : f.getLocals()) {
2428
ImType t = local.getType();
2529
if(t instanceof ImSimpleType) {
26-
// Simple types can always be merged.
27-
local.setType(localSimpleType);
30+
// Keep primitive domains separate so later local merging cannot
31+
// unify e.g. number/bool/string temporaries into one slot.
32+
local.setType(canonicalizeSimpleLocalType((ImSimpleType) t));
2833
}
2934
}
3035
}
3136

37+
private static ImType canonicalizeSimpleLocalType(ImSimpleType t) {
38+
if (TypesHelper.isIntType(t)) {
39+
return localIntType;
40+
}
41+
if (TypesHelper.isRealType(t)) {
42+
return localRealType;
43+
}
44+
if (TypesHelper.isBoolType(t)) {
45+
return localBoolType;
46+
}
47+
if (TypesHelper.isStringType(t)) {
48+
return localStringType;
49+
}
50+
return localSimpleType;
51+
}
52+
3253
private static void transformProgram(ImProg imProg, ImTranslator translator) {
3354
imProg.accept(new Element.DefaultVisitor() {
3455
@Override

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -295,9 +295,14 @@ private int getStacktraceIndex(ImFunction f) {
295295
*/
296296
private int getStacktraceIndex(ImFunctionCall c) {
297297
ImFunction f = c.getFunc();
298-
int res = f.getParameters().size() - 1;
298+
int res;
299299
if (f.hasFlag(FunctionFlagEnum.IS_VARARG)) {
300-
res--;
300+
// Keep stacktrace before vararg payload.
301+
res = f.getParameters().size() - 2;
302+
} else {
303+
// Append for non-vararg calls based on actual call shape.
304+
// This avoids relying on parameter-layout assumptions in lowered calls.
305+
res = c.getArguments().size();
301306
}
302307
if (res < 0 || res > c.getArguments().size() + 1) {
303308
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) {
307312

308313
private int getStacktraceIndex(ImMethodCall c) {
309314
ImFunction f = c.getMethod().getImplementation();
310-
int res = f.getParameters().size() - 2; // subtract one for implicit parameter
315+
int res;
311316
if (f.hasFlag(FunctionFlagEnum.IS_VARARG)) {
312-
res--;
317+
// For vararg methods keep the stacktrace argument before the vararg bucket.
318+
// Method implementations normally include an implicit receiver as first parameter.
319+
res = f.getParameters().size() - 3;
320+
} else {
321+
// For non-vararg methods append after existing explicit call arguments.
322+
// This is robust even if a generated method implementation does not carry
323+
// an implicit receiver parameter in its function signature.
324+
res = c.getArguments().size();
313325
}
314326
if (res < 0 || res > c.getArguments().size() + 1) {
315327
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‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,6 +1333,12 @@ private void checkVarNotConstant(NameRef left, @Nullable NameLink link) {
13331333
return;
13341334
}
13351335
NameDef var = link.getDef();
1336+
if (var != null && isForRangeLoopVar(var)) {
1337+
left.addWarning("Avoid mutating for-loop variable " + Utils.printElement(var)
1338+
+ ": this can cause unexpected iteration side effects (skipped or repeated iterations). "
1339+
+ "This will be an error in a future version.");
1340+
return;
1341+
}
13361342
if (var != null && var.attrIsConstant()) {
13371343
if (var instanceof GlobalVarDef) {
13381344
GlobalVarDef glob = (GlobalVarDef) var;
@@ -1345,6 +1351,13 @@ private void checkVarNotConstant(NameRef left, @Nullable NameLink link) {
13451351
}
13461352
}
13471353

1354+
private boolean isForRangeLoopVar(NameDef var) {
1355+
Element parent = var.getParent();
1356+
return parent instanceof StmtForRange
1357+
|| parent instanceof StmtForRangeUp
1358+
|| parent instanceof StmtForRangeDown;
1359+
}
1360+
13481361
private boolean isInConstructor(Element e) {
13491362
while (e != null) {
13501363
if (e instanceof ConstructorDef) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -919,8 +919,8 @@ public void forRangeStartReadsParameter() {
919919
}
920920

921921
@Test
922-
public void forRangeLoopVarIsImmutable() {
923-
testAssertErrorsLines(false, "Cannot assign a new value to constant",
922+
public void forRangeLoopVarMutationWarns() {
923+
testAssertWarningsLines(false, "unexpected iteration side effects",
924924
"package test",
925925
"init",
926926
" int stackPointer = 3",

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,21 @@
22

33
import com.google.common.base.Charsets;
44
import com.google.common.io.Files;
5+
import config.WurstProjectConfigData;
6+
import de.peeeq.wurstio.WurstCompilerJassImpl;
7+
import de.peeeq.wurstscript.RunArgs;
8+
import de.peeeq.wurstscript.ast.WurstModel;
9+
import de.peeeq.wurstscript.gui.WurstGui;
10+
import de.peeeq.wurstscript.gui.WurstGuiCliImpl;
11+
import de.peeeq.wurstscript.luaAst.LuaCompilationUnit;
512
import de.peeeq.wurstscript.validation.GlobalCaches;
613
import org.testng.annotations.Ignore;
714
import org.testng.annotations.Test;
815

916
import java.io.File;
1017
import java.io.IOException;
1118
import java.util.ArrayList;
19+
import java.util.Collections;
1220
import java.util.List;
1321
import java.util.regex.Matcher;
1422
import java.util.regex.Pattern;
@@ -71,6 +79,27 @@ private void assertContainsRegex(String output, String regex) {
7179
assertTrue("Pattern must occur: " + regex, matcher.find());
7280
}
7381

82+
private String compileLuaWithRunArgs(String testName, boolean withStdLib, String... lines) {
83+
RunArgs runArgs = new RunArgs().with("-lua", "-inline", "-localOptimizations", "-stacktraces");
84+
WurstGui gui = new WurstGuiCliImpl();
85+
WurstCompilerJassImpl compiler = new WurstCompilerJassImpl(null, gui, null, runArgs);
86+
List<CU> inputs = Collections.singletonList(new CU(testName + ".wurst", String.join("\n", lines)));
87+
88+
WurstModel model = parseFiles(Collections.emptyList(), inputs, withStdLib, compiler);
89+
assertNotNull("parse returned null model, errors = " + gui.getErrorList(), model);
90+
assertTrue("unexpected parse/type errors: " + gui.getErrorList(), gui.getErrorList().isEmpty());
91+
compiler.checkProg(model);
92+
assertTrue("unexpected compile errors: " + gui.getErrorList(), gui.getErrorList().isEmpty());
93+
94+
compiler.translateProgToIm(model);
95+
compiler.runCompiletime(new WurstProjectConfigData(), false, false);
96+
LuaCompilationUnit luaCode = compiler.transformProgToLua();
97+
98+
StringBuilder sb = new StringBuilder();
99+
luaCode.print(sb, 0);
100+
return sb.toString();
101+
}
102+
74103
@Test
75104
public void testStdLib() throws IOException {
76105
test().testLua(true).withStdLib().lines(
@@ -87,6 +116,56 @@ public void testStdLib() throws IOException {
87116
assertFunctionBodyContains(compiled, "__wurst_LoadInteger", "return 0", true);
88117
}
89118

119+
@Test
120+
public void stacktraceStringsNotInjectedIntoNumericComparisons() {
121+
String compiled = compileLuaWithRunArgs(
122+
"LuaTranslationTests_stacktraceStringsNotInjectedIntoNumericComparisons",
123+
false,
124+
"package Test",
125+
"public tuple Vec2(real x, real y)",
126+
"public tuple scanResult(boolean found, Vec2 pos)",
127+
"class MyRect",
128+
" real minX",
129+
" real minY",
130+
" real maxX",
131+
" real maxY",
132+
" construct(real minX, real minY, real maxX, real maxY)",
133+
" this.minX = minX",
134+
" this.minY = minY",
135+
" this.maxX = maxX",
136+
" this.maxY = maxY",
137+
" function contains(Vec2 p) returns boolean",
138+
" return p.x > minX and p.x < maxX and p.y > minY and p.y < maxY",
139+
"function gridAnchor(MyRect r, int dirX, int dirY, boolean isBehind) returns Vec2",
140+
" let ax = dirX >= 0 ? r.minX + (isBehind ? 0 : 64) : r.maxX + (isBehind ? 0 : 64)",
141+
" let ay = dirY >= 0 ? r.minY : r.maxY",
142+
" return Vec2(ax, ay)",
143+
"function stepsAlongY(MyRect r, real startY, int dirY, real stepSize) returns int",
144+
" if dirY >= 0 and stepSize > 0 and r.maxY > startY",
145+
" return 1",
146+
" return 0",
147+
"class BuildScanIterator",
148+
" private var scanWidth = 128.",
149+
" function tryRect(MyRect r, int dirX, int dirY) returns scanResult",
150+
" if r == null",
151+
" return scanResult(false, Vec2(0., 0.))",
152+
" let start = gridAnchor(r, dirX, dirY, true)",
153+
" if not r.contains(start)",
154+
" return scanResult(false, Vec2(0., 0.))",
155+
" let maxY = stepsAlongY(r, start.y, dirY, scanWidth)",
156+
" if maxY >= 0",
157+
" return scanResult(true, start)",
158+
" return scanResult(false, Vec2(0., 0.))",
159+
"init",
160+
" let b = new BuildScanIterator",
161+
" b.tryRect(new MyRect(0., 0., 256., 256.), 1, 1)"
162+
);
163+
assertContainsRegex(compiled, "\"when calling contains");
164+
assertDoesNotContainRegex(compiled, "\"when calling contains[^\"]*\"\\s*[<>]=?");
165+
assertContainsRegex(compiled, "stepsAlongY\\(");
166+
assertDoesNotContainRegex(compiled, "(?s)if not\\((\\w+)\\) then.*?stepsAlongY\\([^\\n]*,\\s*\\1\\s*,");
167+
}
168+
90169
@Test
91170
public void continueLoweringInLua() throws IOException {
92171
test().testLua(true).lines(

0 commit comments

Comments
 (0)