From 142ed2e4475ce9a450b0c863d31756ad26623667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8E=9F=E7=82=B9?= <467490186@qq.com> Date: Fri, 9 Oct 2026 13:43:11 +0800 Subject: [PATCH 1/3] fix(codegen): rebuild variadic aggregation array per statement --- src/Generator/CallArgumentGenerator.php | 13 +++++- .../compiler/variadic/loop-accumulation.phpt | 41 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 tests/compiler/variadic/loop-accumulation.phpt diff --git a/src/Generator/CallArgumentGenerator.php b/src/Generator/CallArgumentGenerator.php index c2fdd2ba..5c92a6df 100644 --- a/src/Generator/CallArgumentGenerator.php +++ b/src/Generator/CallArgumentGenerator.php @@ -305,8 +305,17 @@ protected function parseNativeCallArgs( } } - $variadicVar ??= $this->addTmpVar(Type::ARRAY); - if ($arg->unpack) { + if ($variadicVar === null) { + // Declare the aggregation array inline instead of hoisting it + // into the function preamble (addTmpVar). A preamble temp is + // constructed once and never reset, so a call site inside a + // loop would accumulate arguments across iterations. An inline + // declaration re-constructs an empty array every time the + // statement executes, matching ensureCallArrayArgs. + $variadicVar = $this->genTmpVarName(); + $this->context->beforeStmtLines[] = Type::ARRAY . ' ' . $variadicVar . '{};'; + } + if ($arg->unpack) { $method = $argInfo->byRef ? 'mergeReferences' : 'merge'; $this->context->beforeStmtLines[] = $variadicVar . '.' . $method . '(' . $this->parseArrayArg($arg) . ');'; diff --git a/tests/compiler/variadic/loop-accumulation.phpt b/tests/compiler/variadic/loop-accumulation.phpt new file mode 100644 index 00000000..deecc242 --- /dev/null +++ b/tests/compiler/variadic/loop-accumulation.phpt @@ -0,0 +1,41 @@ +--TEST-- +Variadic arguments reset on every loop iteration instead of accumulating +--FILE-- + */ + public array $k = []; + public function add(...$els): void { + foreach ($els as $e) { $this->k[] = $e; } + } + public function cnt(): int { return count($this->k); } +} + +function main() +{ + $b = new Box(); + + // Positional variadic inside a for loop: 3 iterations x 2 args = 6. + for ($i = 0; $i < 3; $i++) { + $b->add(new Leaf(), new Leaf()); + } + + // Positional variadic inside a while loop: 2 iterations x 1 arg = 2. + $i = 0; + while ($i < 2) { + $b->add(new Leaf()); + $i++; + } + + // Unpacked variadic inside a for loop: 2 iterations x 2 args = 4. + $arr = [new Leaf(), new Leaf()]; + for ($i = 0; $i < 2; $i++) { + $b->add(...$arr); + } + + var_dump($b->cnt()); +} +?> +--EXPECT-- +int(12) From 26d12b00649f93f2ec0fea50984a48f0453b5833 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8E=9F=E7=82=B9?= <467490186@qq.com> Date: Fri, 9 Oct 2026 16:57:23 +0800 Subject: [PATCH 2/3] fix(codegen): hoist variadic aggregation temp and reset per call A call-site inline declaration of the aggregation array broke forward goto: the generated C++ jump crossed the initialization of a php::Array local, which is ill-formed C++ (review finding). Register the temp via addTmpVar so its declaration stays in the function preamble, and emit an unset() before aggregating the arguments. The reset runs on every evaluation of the call site, so a call inside a loop still aggregates into an empty array every iteration. --- src/Generator/CallArgumentGenerator.php | 22 +++++++++---------- .../compiler/variadic/variadic-goto-skip.phpt | 19 ++++++++++++++++ 2 files changed, 30 insertions(+), 11 deletions(-) create mode 100644 tests/compiler/variadic/variadic-goto-skip.phpt diff --git a/src/Generator/CallArgumentGenerator.php b/src/Generator/CallArgumentGenerator.php index 5c92a6df..f7d68649 100644 --- a/src/Generator/CallArgumentGenerator.php +++ b/src/Generator/CallArgumentGenerator.php @@ -305,17 +305,17 @@ protected function parseNativeCallArgs( } } - if ($variadicVar === null) { - // Declare the aggregation array inline instead of hoisting it - // into the function preamble (addTmpVar). A preamble temp is - // constructed once and never reset, so a call site inside a - // loop would accumulate arguments across iterations. An inline - // declaration re-constructs an empty array every time the - // statement executes, matching ensureCallArrayArgs. - $variadicVar = $this->genTmpVarName(); - $this->context->beforeStmtLines[] = Type::ARRAY . ' ' . $variadicVar . '{};'; - } - if ($arg->unpack) { + if ($variadicVar === null) { + // Keep the aggregation array in the function preamble + // (addTmpVar) so a forward goto can never jump across a + // call-site C++ local declaration, and reset it instead. + // The reset runs on every evaluation of the call site, so a + // call inside a loop aggregates into an empty array every + // iteration instead of accumulating arguments. + $variadicVar = $this->addTmpVar(Type::ARRAY); + $this->context->beforeStmtLines[] = $variadicVar . '.unset();'; + } + if ($arg->unpack) { $method = $argInfo->byRef ? 'mergeReferences' : 'merge'; $this->context->beforeStmtLines[] = $variadicVar . '.' . $method . '(' . $this->parseArrayArg($arg) . ');'; diff --git a/tests/compiler/variadic/variadic-goto-skip.phpt b/tests/compiler/variadic/variadic-goto-skip.phpt new file mode 100644 index 00000000..e8a6c259 --- /dev/null +++ b/tests/compiler/variadic/variadic-goto-skip.phpt @@ -0,0 +1,19 @@ +--TEST-- +A forward goto may skip a by-value variadic call without crossing an initialization +--FILE-- + 0) { + goto finished; + } + count_values(1, 2); + finished: + echo "done\n"; +} +?> +--EXPECT-- +done From 86ce14b02c1158cf634034006e42f38e36ffdcd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8E=9F=E7=82=B9?= <467490186@qq.com> Date: Fri, 9 Oct 2026 16:57:23 +0800 Subject: [PATCH 3/3] test(variadic): cover mixed positional and unpack aggregation in loop A lone unpack of a known PHP array takes the single-unpack fast path and never enters the aggregation code, so the loop test did not exercise the reset. Aggregate f(1, ...$arr) per iteration instead. --- tests/compiler/variadic/loop-accumulation.phpt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/compiler/variadic/loop-accumulation.phpt b/tests/compiler/variadic/loop-accumulation.phpt index deecc242..daec51ba 100644 --- a/tests/compiler/variadic/loop-accumulation.phpt +++ b/tests/compiler/variadic/loop-accumulation.phpt @@ -34,8 +34,14 @@ function main() $b->add(...$arr); } + // Mixed positional + unpacked variadic enters the aggregation path + // (the single-unpack fast path does not): 2 iterations x 3 args = 6. + for ($i = 0; $i < 2; $i++) { + $b->add(new Leaf(), ...$arr); + } + var_dump($b->cnt()); } ?> --EXPECT-- -int(12) +int(18)