Skip to content

Commit 2dbba0c

Browse files
committed
Leave a forwarded vararg placeholder on the packed path
A vararg constructor is reached through a generated new_C wrapper that passes its own vararg parameter to construct_C. When a call above the bound keeps that wrapper as the retained original, its body still holds the forwarding call, and the placeholder is one node standing for however many arguments the caller passed. Counting nodes turned it into an arity, so construct_C_1 was generated and the call rewritten to it: every argument after the first was dropped and the constructor ran on one value. Node count is only an arity when no node is a placeholder, so calls that forward one are now left alone. This is Lua-only in effect. The forwarding call survives in the body of a vararg original, and a copy has its placeholder expanded into real parameters long before anything counts them again, so only originals match - which Jass removes and Lua retains. Both the generation and the rewrite loop consult the same predicate. Skipping generation alone would not have been enough: another call can produce a copy at the same node count, and the rewrite would then redirect the forwarding call to it regardless. The existing above-the-bound test used a plain function, which has no wrapper and so never forwards. The new test uses a constructor and fails on the constructed value before this change.
1 parent 99f5dae commit 2dbba0c

2 files changed

Lines changed: 72 additions & 4 deletions

File tree

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

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public void run() {
5858
while (generated) {
5959
generated = false;
6060
for (ImFunctionCall c : collectVarargCalls()) {
61-
if (c.getFunc().hasFlag(IS_VARARG) && shouldSpecialise(c)
61+
if (c.getFunc().hasFlag(IS_VARARG) && shouldSpecialise(c) && !forwardsAVarargParameter(c.getArguments())
6262
&& !varargFuncs.contains(c.getFunc(), c.getArguments().size())) {
6363
generateVarargFunc(c);
6464
generated = true;
@@ -72,7 +72,7 @@ public void run() {
7272
for (ImMethodCall c : collectMonomorphicVarargMethodCalls()) {
7373
ImFunction implementation = c.getMethod().getImplementation();
7474
List<ImExpr> arguments = receiverAndArguments(c);
75-
if (shouldSpecialise(arguments)
75+
if (shouldSpecialise(arguments) && !forwardsAVarargParameter(arguments)
7676
&& !varargFuncs.contains(implementation, arguments.size())) {
7777
generateVarargFunc(implementation, arguments, c);
7878
generated = true;
@@ -90,21 +90,56 @@ public void run() {
9090
// (need to collect vararg calls again, because first phase can create copies of calls)
9191
for (ImFunctionCall call : collectVarargCalls()) {
9292
ImFunction newFunc = varargFuncs.get(call.getFunc(), call.getArguments().size());
93-
if (newFunc != null) {
93+
if (newFunc != null && !forwardsAVarargParameter(call.getArguments())) {
9494
redirectCall(call, newFunc);
9595
}
9696
}
9797
if (luaTarget) {
9898
for (ImMethodCall call : collectMonomorphicVarargMethodCalls()) {
9999
ImFunction implementation = call.getMethod().getImplementation();
100100
ImFunction newFunc = varargFuncs.get(implementation, 1 + call.getArguments().size());
101-
if (newFunc != null) {
101+
if (newFunc != null && !forwardsAVarargParameter(receiverAndArguments(call))) {
102102
redirectMethodCall(call, newFunc);
103103
}
104104
}
105105
}
106106
}
107107

108+
109+
/**
110+
* Whether a call passes a vararg placeholder straight through, which is what the generated
111+
* `new_C` wrapper of a vararg constructor does with its own parameter. The placeholder is a
112+
* single node standing for however many arguments the caller actually passed, so the call's node
113+
* count is not an arity: specialising by it would produce a fixed-arity callee and drop every
114+
* argument after the first.
115+
*
116+
* <p>Only reachable on Lua. The forwarding call lives in the body of a vararg original, and a
117+
* copy has its placeholder expanded into real parameters before anything looks at it again, so
118+
* this matches only originals - which Jass removes and Lua retains.
119+
*
120+
* <p>Both the generation and the rewrite loop consult this. Skipping generation alone would not
121+
* be enough: another call could have produced a copy at the same node count, and the rewrite
122+
* would then redirect the forwarding call to it.
123+
*/
124+
private static boolean forwardsAVarargParameter(List<ImExpr> arguments) {
125+
for (ImExpr argument : arguments) {
126+
if (argument instanceof ImVarAccess access && isVarargPlaceholder(access.getVar())) {
127+
return true;
128+
}
129+
}
130+
return false;
131+
}
132+
133+
/** The trailing parameter of a function still marked vararg, as opposed to a local or a copy's. */
134+
private static boolean isVarargPlaceholder(ImVar variable) {
135+
if (variable.getParent() == null
136+
|| !(variable.getParent().getParent() instanceof ImFunction function)
137+
|| !function.hasFlag(IS_VARARG)) {
138+
return false;
139+
}
140+
List<ImVar> parameters = function.getParameters();
141+
return !parameters.isEmpty() && parameters.get(parameters.size() - 1) == variable;
142+
}
108143
/** A method call which can only ever reach one implementation, and that implementation is vararg. */
109144
private Collection<ImMethodCall> collectMonomorphicVarargMethodCalls() {
110145
final Collection<ImMethodCall> calls = new ArrayList<>();

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2096,6 +2096,39 @@ public void varargCallAboveTheLuaArityBoundKeepsThePackedPath() throws IOExcepti
20962096
compiled.contains("table.pack"));
20972097
}
20982098

2099+
/**
2100+
* A vararg constructor is reached through a compiler-generated `new_C` wrapper which forwards its
2101+
* vararg placeholder to `construct_C`. When a call above the bound keeps that wrapper as the
2102+
* retained original, its body still holds the forwarding call, and the placeholder is one node
2103+
* standing for however many arguments the caller passed. Specialising by node count would rewrite
2104+
* it to a fixed-arity constructor and silently drop every argument after the first.
2105+
*/
2106+
@Test
2107+
public void varargConstructorAboveTheLuaArityBoundKeepsThePackedPath() {
2108+
StringBuilder args = new StringBuilder();
2109+
int n = 70;
2110+
for (int i = 1; i <= n; i++) {
2111+
if (i > 1) {
2112+
args.append(", ");
2113+
}
2114+
args.append(i);
2115+
}
2116+
test().testLua(true).executeProg().lines(
2117+
"package Test",
2118+
"native testSuccess()",
2119+
"class Tally",
2120+
" int total = 0",
2121+
" construct(vararg int xs)",
2122+
" for x in xs",
2123+
" total += x",
2124+
"init",
2125+
" let big = new Tally(" + args + ")",
2126+
" let small = new Tally(1, 2)",
2127+
" if big.total == " + (n * (n + 1) / 2) + " and small.total == 3",
2128+
" testSuccess()"
2129+
);
2130+
}
2131+
20992132
@Test
21002133
public void virtuallyDispatchedVarargMethodKeepsThePackedPath() throws IOException {
21012134
test().testLua(true).executeProg().lines(

0 commit comments

Comments
 (0)