Keep a preserved name and a self reference on the vararg original

Two ways a generated copy could take over something that belongs only to the
original it was copied from.

A preserved name is part of the map's Warcraft-facing API, set by
@preserveName and by ExecuteFunc. The copy inherits the flag, and because it
also shares the original's trace, collectPredefinedNames resets both to the
same source name: the emitted Lua then defines that name twice and the later
definition wins. ExecuteFunc makes this easy to reach, since it marks its
target preserved and emits a zero-argument call, which is exactly what makes
a copy. The flag is now dropped from copies on Lua, where the original is
retained and keeps the name that external code calls.

The other is the reference case of the recursion fix. ReferenceRewritingCopy
retargets a function's own references, and the repair visitor undid that for
calls but not for function references, so a self reference inside a copy kept
naming the copy: registering it as a callback would invoke a fixed-arity body
at an arity nobody checked. Those two node types are the only ones the copy
retargets that name a function at all, so the visitor now covers the pair
rather than the reported half. Lua only, because nothing redirects a
reference afterwards and only that target keeps the original; on Jass it is
removed and the reference would dangle.

Jass keeps both flags and both reference kinds exactly as before.

Author: Frotty

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

@@ -13,6 +13,7 @@
 import java.util.stream.Collectors;
 
 import static de.peeeq.wurstscript.translation.imtranslation.FunctionFlagEnum.IS_VARARG;
+import static de.peeeq.wurstscript.translation.imtranslation.FunctionFlagEnum.PRESERVE_NAME;
 
 /**
  * Takes a program and eliminates vararg functions, replacing them with
@@ -207,10 +208,11 @@ private void generateVarargFunc(ImFunction func, List<ImExpr> arguments, Element
 
         // Create new function
         ImFunction newFunc = ReferenceRewritingCopy.copy(func);
-        // The copy retargets the function's own references, so a recursive call inside it now names
-        // the copy. That is wrong whenever the recursion uses a different argument count: the call
-        // must go back to naming the vararg original, so the rewrite below maps it to a copy of its
-        // own arity like any other call.
+        // ReferenceRewritingCopy retargets the function's own references - both call and reference
+        // nodes - so inside the copy they now name the copy. That is wrong for either kind. A
+        // recursive call must go back to naming the vararg original, so the rewrite below maps it to
+        // a copy of its own arity like any other call; a self reference must name the original too,
+        // because it is invoked at an arity this pass never sees.
         newFunc.accept(new Element.DefaultVisitor() {
             @Override
             public void visit(ImFunctionCall call) {
@@ -219,6 +221,17 @@ public void visit(ImFunctionCall call) {
                     call.setFunc(func);
                 }
             }
+
+            @Override
+            public void visit(ImFuncRef ref) {
+                super.visit(ref);
+                // Lua only: nothing redirects a reference afterwards, so it keeps naming whatever it
+                // is set to here, and only this target retains the original. On Jass the original is
+                // removed below and pointing at it would leave the reference dangling.
+                if (luaTarget && ref.getFunc() == newFunc) {
+                    ref.setFunc(func);
+                }
+            }
         });
         newFunc.setName(func.getName() + "_" + argumentSize);
         // replace vararg with special parameters:
@@ -266,12 +279,17 @@ public void visit(ImVarargLoop imLoop) {
         }
 
 
-        // Remove vararg flag
+        // Drop the vararg flag, and on Lua the name preservation with it. A preserved name is part
+        // of the map's Warcraft-facing API and belongs to the retained original, which is what
+        // external code calls at an arity this pass never sees. Since a copy shares the original's
+        // trace, and LuaTranslator.collectPredefinedNames() resets every preserved function to its
+        // trace's source name, an inherited flag would emit both under one name.
         List<FunctionFlag> list = new ArrayList<>();
         for (FunctionFlag flag : newFunc.getFlags()) {
-            if (flag != IS_VARARG) {
-                list.add(flag);
+            if (flag == IS_VARARG || (luaTarget && flag == PRESERVE_NAME)) {
+                continue;
             }
+            list.add(flag);
         }
         newFunc.setFlags(list);
         // Add new function to prog

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

@@ -2201,6 +2201,39 @@ public void varargParameterCannotBeForwardedToAMethod() {
     }
 
 
+    /**
+     * `@preserveName` and `ExecuteFunc` mark a function's emitted name as part of the map's
+     * WC3-facing API, and `LuaTranslator.collectPredefinedNames()` resets every function carrying
+     * that flag to its trace's source name. A generated copy shares the original's trace, so an
+     * inherited flag would emit the original and every copy under one name and let the last
+     * definition win. The preserved name belongs to the retained original: that is the one external
+     * code calls, at an arity this pass never gets to see.
+     */
+    @Test
+    public void preservedNameStaysOnTheVarargOriginalNotItsCopies() {
+        String compiled = compileOptimizedLua(
+            "preservedNameStaysOnTheVarargOriginalNotItsCopies",
+            "package Test",
+            "native consume(int i)",
+            "@preserveName @noinline public function tally(vararg int xs) returns int",
+            "    var sum = 0",
+            "    for x in xs",
+            "        sum += x",
+            "    return sum",
+            "init",
+            "    consume(tally(1, 2))"
+        );
+        assertTrue("the fixed-arity copy must keep its own suffixed name:\n" + compiled,
+            compiled.contains("function tally_2("));
+        int definitions = 0;
+        for (int at = compiled.indexOf("function tally("); at >= 0;
+             at = compiled.indexOf("function tally(", at + 1)) {
+            definitions++;
+        }
+        assertEquals("the preserved name must name exactly one function:\n" + compiled,
+            1, definitions);
+    }
+
     /**
      * The inliner used to refuse every function whose return fact the local-player analysis had
      * marked, and that fact fires for anything reachable from a client-local branch anywhere in the