Local-player analysis: build it only for the passes which use it

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/FunctionSplitter.java

@@ -38,8 +38,11 @@ private void optimize() {
         Preconditions.checkArgument(func.getReturnType() instanceof ImVoid, "func must return void");
         // run some basic optimizations first:
         func.flatten(tr);
-        LocalPlayerContextAnalyzer localPlayerContextAnalyzer =
-            new LocalPlayerContextAnalyzer(tr.getImProg());
+        // Both passes ask the locality analysis only about locals and the values assigned to them (the function has
+        // no parameters), and the compile-time state usually has none, so the analysis is made only when it has some.
+        LocalPlayerContextAnalyzer localPlayerContextAnalyzer = func.getLocals().isEmpty()
+            ? null
+            : new LocalPlayerContextAnalyzer(tr.getImProg());
         new ConstantAndCopyPropagation().optimizeFunc(func, localPlayerContextAnalyzer);
         new LocalMerger().optimizeFunc(func, localPlayerContextAnalyzer, tr);
         Set<ImVar> usedVars = UsedVariables.calculate(func);

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalMerger.java

@@ -119,6 +119,8 @@ private void mergeLocals(Map<ImStmt, Set<ImVar>> livenessInfo, Set<ImVar> liveAt
                 if (!canMerge(color.getType(), v.getType())) {
                     continue;
                 }
+                // A merged local would hold both values while the analysis, made before this pass, still answers for
+                // each: keeping them apart keeps it true for the passes which use it after this one.
                 if (localPlayerContextAnalyzer != null
                     && localPlayerContextAnalyzer.isLocalPlayerDependent(v)
                     != localPlayerContextAnalyzer.isLocalPlayerDependent(color)) {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/intermediatelang/optimizer/LocalPlayerContextAnalyzer.java

@@ -215,7 +215,8 @@ public boolean isLocalPlayerDependent(ImVar variable) {
         return variable != null && variableIsDependent(variable);
     }
 
-    public boolean isLocalPlayerSource(ImFunction function) {
+    /** Whether {@code function} is a native whose value may differ between clients; a fixed list, no analysis. */
+    public static boolean isLocalPlayerSource(ImFunction function) {
         return isClientLocalValueSource(function);
     }
 

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImInliner.java

@@ -433,7 +433,7 @@ private static boolean isStoreTarget(ImLExpr target) {
     private boolean isEffectFree(Element e) {
         if (e instanceof ImFunctionCall call) {
             ImFunction target = call.getFunc();
-            if (!target.isNative() || localPlayerAnalysis().isLocalPlayerSource(target)
+            if (!target.isNative() || LocalPlayerContextAnalyzer.isLocalPlayerSource(target)
                 || !(SideEffectFreeNatives.isFunctionWithoutSideEffect(target.getName())
                     || translator.isLuaTableRead(target))) {
                 return false;

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

@@ -1,7 +1,6 @@
 package de.peeeq.wurstscript.translation.imtranslation;
 
 import de.peeeq.wurstscript.intermediatelang.optimizer.LocalMerger;
-import de.peeeq.wurstscript.intermediatelang.optimizer.LocalPlayerContextAnalyzer;
 import de.peeeq.wurstscript.jassIm.*;
 import de.peeeq.wurstscript.types.TypesHelper;
 
@@ -24,6 +23,9 @@
  * got some are passed through the local merger once more: the temporaries share a slot with each
  * other and with dead locals of their type, and a function with many casts does not grow towards
  * Lua's limit of 200 locals. Merging only renames variables, so the operands stay variables.
+ * The merger runs here without a locality analysis, so a client-local value and a synchronized one
+ * may share a local. Elsewhere it keeps them apart only so that the analysis made before it stays
+ * true for the passes which use it afterwards; this is the last pass, so nothing uses one after it.
  */
 public final class LuaOldGenericsCasts {
 
@@ -72,9 +74,8 @@ public void visit(ImCast cast) {
                 replacement));
         }
         prog.flatten(translator);
-        LocalPlayerContextAnalyzer analyzer = new LocalPlayerContextAnalyzer(prog);
         for (ImFunction function : touched) {
-            new LocalMerger().optimizeFunc(function, analyzer, translator);
+            new LocalMerger().optimizeFunc(function, null, translator);
         }
     }
 }

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

@@ -1311,6 +1311,41 @@ public void oldGenericsCastTemporariesAreMerged() {
         assertFalse("every cast is still inlined:\n" + readMany, readMany.contains("__wurst_oldGenericsFromInt"));
     }
 
+    /**
+     * The merge after the cast operands are hoisted is the last pass, so nothing reads a locality analysis
+     * after it, and a client-local local may share its slot with a synchronized one which is dead by then.
+     */
+    @Test
+    public void oldGenericsCastMergeIgnoresLocality() {
+        String compiled = compileOptimizedLua("oldGenericsCastMergeIgnoresLocality",
+            "type player extends handle",
+            "package Test",
+            "@extern native GetLocalPlayer() returns player",
+            "@extern native GetPlayerId(player p) returns integer",
+            "native readInt() returns integer",
+            "native consume(integer value)",
+            "int array slots",
+            "@inline function load<T>(int key) returns T",
+            "    return slots[key] castTo T",
+            "@noinline function show(int key)",
+            "    let id = GetPlayerId(GetLocalPlayer())",
+            "    consume(id)",
+            "    consume(id)",
+            "    let other = readInt()",
+            "    consume(other)",
+            "    consume(other)",
+            "    consume(load<int>(key))",
+            "init",
+            "    show(1)");
+        String show = topLevelFunctionBodyWithPrefix(compiled, "show");
+        java.util.regex.Matcher clientLocal =
+            java.util.regex.Pattern.compile("(\\w+) = GetPlayerId\\(GetLocalPlayer\\(\\)\\)").matcher(show);
+        assertTrue("the client-local value is kept in a local:\n" + show, clientLocal.find());
+        String local = clientLocal.group(1);
+        assertTrue("that local is reused for a synchronized value:\n" + show,
+            show.contains(local + " = readInt()") || show.contains(local + " = Test_slots[key]"));
+    }
+
     /**
      * A class-typed caller uses an old-generics result as is, so a missing entry has to come back
      * as nil: TimedLoop looks its timer up in a HashMap and creates one when the lookup is null.

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

@@ -3934,35 +3934,12 @@ public void clientLocalNativeValuesAreLocalitySources() {
             "BlzIsSelectionEnabled"
         ));
         Element trace = Ast.NoExpr();
-        ImFunctions functions = JassIm.ImFunctions();
-        java.util.Map<String, ImFunction> functionsByName = new java.util.LinkedHashMap<>();
-        for (String name : localValueSources) {
-            ImFunction nativeFunction = nativeIntFunction(trace, name);
-            functions.add(nativeFunction);
-            functionsByName.put(name, nativeFunction);
-        }
-        for (String name : intentionallyExcludedSources) {
-            ImFunction nativeFunction = nativeIntFunction(trace, name);
-            functions.add(nativeFunction);
-            functionsByName.put(name, nativeFunction);
-        }
-        ImProg prog = JassIm.ImProg(
-            trace,
-            JassIm.ImVars(),
-            functions,
-            JassIm.ImMethods(),
-            JassIm.ImClasses(),
-            JassIm.ImTypeClassFuncs(),
-            new java.util.HashMap<>()
-        );
-        LocalPlayerContextAnalyzer analyzer = new LocalPlayerContextAnalyzer(prog);
-
         for (String name : localValueSources) {
-            assertTrue(analyzer.isLocalPlayerSource(functionsByName.get(name)),
+            assertTrue(LocalPlayerContextAnalyzer.isLocalPlayerSource(nativeIntFunction(trace, name)),
                 name + " must be treated as a client-local value source");
         }
         for (String name : intentionallyExcludedSources) {
-            assertFalse(analyzer.isLocalPlayerSource(functionsByName.get(name)),
+            assertFalse(LocalPlayerContextAnalyzer.isLocalPlayerSource(nativeIntFunction(trace, name)),
                 name + " is synchronized event data or user-managed local state");
         }
     }