From 044ba910940fffc3b2a5d499c505ff8fe57a5368 Mon Sep 17 00:00:00 2001 From: Giandonn Date: Sat, 29 Aug 2026 19:52:55 -0300 Subject: [PATCH 1/3] fix(optimizer): stop folding count() on unfoldable array literals doFoldCountLiteral replaced `count([...])` with the number of AST items and dropped the array literal entirely. That is only correct when the item count equals the runtime element count and no element carries an observable effect. Three common shapes break both assumptions: count([bump(), bump()]); // folded to 2, bump() never ran count(['a' => 1, 'a' => 2]); // folded to 2, PHP counts 1 count([...$rest, 9]); // folded to 2, PHP counts 6 The spread case is the most damaging: it silently yields a wrong number in ordinary code that compiles without any diagnostic. The fold now applies only when every item is unkeyed, is not a spread, and holds an expression whose evaluation cannot be observed - a scalar, a constant fetch, a unary sign over either, a nested literal that is itself foldable, or a variable already known to be defined. An undefined variable still reaches the dynamic path so it reports the same diagnostic as PHP. Everything else keeps the runtime php::fn::count() call, so `count([1, 2, 3])` and friends still fold as before. Covered by tests/compiler/array/count-literal-fold.phpt for the runtime semantics and by CountLiteralFoldTest for the fold/no-fold decision in the generated C++. --- phpunit/code/count-literal-fold-safe.php | 10 +++++ phpunit/code/count-literal-fold-unsafe.php | 17 +++++++ phpunit/src/CountLiteralFoldTest.php | 40 +++++++++++++++++ src/Optimizer/FuncCallOptimizer.php | 45 +++++++++++++++++++ tests/compiler/array/count-literal-fold.phpt | 47 ++++++++++++++++++++ 5 files changed, 159 insertions(+) create mode 100644 phpunit/code/count-literal-fold-safe.php create mode 100644 phpunit/code/count-literal-fold-unsafe.php create mode 100644 phpunit/src/CountLiteralFoldTest.php create mode 100644 tests/compiler/array/count-literal-fold.phpt diff --git a/phpunit/code/count-literal-fold-safe.php b/phpunit/code/count-literal-fold-safe.php new file mode 100644 index 00000000..66c558c0 --- /dev/null +++ b/phpunit/code/count-literal-fold-safe.php @@ -0,0 +1,10 @@ + 1, 'a' => 2]), "\n"; + echo count([...$rest, 9]), "\n"; + echo count([$i++, $i++]), "\n"; +} diff --git a/phpunit/src/CountLiteralFoldTest.php b/phpunit/src/CountLiteralFoldTest.php new file mode 100644 index 00000000..83785291 --- /dev/null +++ b/phpunit/src/CountLiteralFoldTest.php @@ -0,0 +1,40 @@ +compileToCpp('count-literal-fold-unsafe.php'); + + // Element side effects, a repeated key and a spread each make the + // number of AST items differ from the runtime element count. + self::assertSame(4, substr_count($cpp, 'php::fn::count(')); + self::assertStringContainsString('php_bump()', $cpp); + self::assertStringContainsString('i++', $cpp); + } + + public function testPlainArrayLiteralsStillFoldAtCompileTime(): void + { + $cpp = $this->compileToCpp('count-literal-fold-safe.php'); + + self::assertStringNotContainsString('php::fn::count(', $cpp); + } + + private function compileToCpp(string $file): string + { + global $translator; + + $compiler = CompilerTest::create(TYPEPHP_ROOT_PATH); + $translator = $compiler; + $source = TYPEPHP_ROOT_PATH . '/phpunit/code/' . $file; + $compiler->addFiles([$source]); + $compiler->prepareFile($source); + + return file_get_contents($compiler->convertFile($source)); + } +} diff --git a/src/Optimizer/FuncCallOptimizer.php b/src/Optimizer/FuncCallOptimizer.php index fe2ae690..3c15ca65 100644 --- a/src/Optimizer/FuncCallOptimizer.php +++ b/src/Optimizer/FuncCallOptimizer.php @@ -656,11 +656,56 @@ trait FuncCallOptimizer } $arg = $expr->args[0]->value; if ($arg instanceof Node\Expr\Array_) { + if (!$this->isCountFoldableArray($arg)) { + return false; + } return count($arg->items) . $this->getPlatform()->getIntegerLiteralSuffix(); } return $this->genStdContainerCount($arg); } + /** + * The number of AST items only equals the runtime element count when no + * item spreads another array, no key can collide with another key, and + * dropping the element expressions cannot lose an observable effect. + * Anything else keeps the runtime php::fn::count() call. + */ + protected function isCountFoldableArray(Node\Expr\Array_ $array): bool + { + foreach ($array->items as $item) { + // [...$other] contributes an element count only known at runtime, + // and a key may collapse onto an earlier one: ['a' => 1, 'a' => 2] + // counts as one element, not two. + if ($item->unpack || $item->key !== null) { + return false; + } + if (!$this->isCountFoldableItem($item->value)) { + return false; + } + } + return true; + } + + protected function isCountFoldableItem(Node\Expr $value): bool + { + // ConstFetch also covers true, false and null. + if ($this->isScalar($value) + || $value instanceof Node\Expr\ConstFetch + || $value instanceof Node\Expr\ClassConstFetch + ) { + return true; + } + if ($value instanceof Node\Expr\UnaryMinus || $value instanceof Node\Expr\UnaryPlus) { + return $this->isCountFoldableItem($value->expr); + } + if ($value instanceof Node\Expr\Array_) { + return $this->isCountFoldableArray($value); + } + // A defined variable is a plain read; an undefined one must reach the + // dynamic path so it still reports the same diagnostic as PHP. + return $this->isVarExpr($value) && is_string($value->name) && $this->hasVar($value->name); + } + protected function doFoldKnownClass(Node\Expr\FuncCall $expr): string|false { $cn = $expr->args[0]->value; diff --git a/tests/compiler/array/count-literal-fold.phpt b/tests/compiler/array/count-literal-fold.phpt new file mode 100644 index 00000000..ade723fc --- /dev/null +++ b/tests/compiler/array/count-literal-fold.phpt @@ -0,0 +1,47 @@ +--TEST-- +count() on an array literal keeps spreads, duplicate keys and element side effects +--FILE-- + 1, 'a' => 2])); + + // A spread contributes a count only known at runtime. + $rest = [1, 2, 3, 4, 5]; + var_dump(count([...$rest, 9])); + + // Side effects of the elements must be observable afterwards. + $i = 0; + var_dump(count([$i++, $i++])); + var_dump($i); + + // Plain literals stay eligible for the compile-time fold. + $a = 1; + var_dump(count([1, 2, 3])); + var_dump(count([[1, 2], [3]])); + var_dump(count([$a, -2, true, null])); + var_dump(count([])); +} +?> +--EXPECT-- +bump +bump +int(2) +int(1) +int(6) +int(2) +int(2) +int(3) +int(2) +int(4) +int(0) From 8328c4b50a24ab2914af4d7260821aff1fa1453a Mon Sep 17 00:00:00 2001 From: Giandonn Date: Sat, 29 Aug 2026 20:08:36 -0300 Subject: [PATCH 2/3] style: apply the project file header to the new test files --- phpunit/code/count-literal-fold-safe.php | 7 +++++++ phpunit/code/count-literal-fold-unsafe.php | 7 +++++++ phpunit/src/CountLiteralFoldTest.php | 10 ++++++++++ 3 files changed, 24 insertions(+) diff --git a/phpunit/code/count-literal-fold-safe.php b/phpunit/code/count-literal-fold-safe.php index 66c558c0..8c777ccf 100644 --- a/phpunit/code/count-literal-fold-safe.php +++ b/phpunit/code/count-literal-fold-safe.php @@ -1,4 +1,11 @@ Date: Sun, 30 Aug 2026 10:47:25 -0300 Subject: [PATCH 3/3] fix(optimizer): restrict the count() literal fold to provably inert items The first whitelist was too broad. ConstFetch, ClassConstFetch and the base Node\Scalar type all admit expressions PHP must still evaluate, so count([UNDEFINED_COUNT_LITERAL]), count([KnownClass::MISSING]) and count(["{$object->property}"]) folded to 1, dropping two Errors and a __get() call. The defined-variable check was not a purity proof either: hasVar() only reports a compiler slot, not that the variable is still initialized on every path after unset(). Narrow the fold to items whose evaluation cannot be observed: - literal Int_, Float_ and String_ (an interpolated string is a distinct InterpolatedString node, so String_ already excludes it); - the language constants true, false and null only; - unary plus/minus over a literal int or float; - recursively safe nested arrays. Variables, general constant and class constant fetches, interpolated strings and every other expression stay on the runtime path, and by-reference items are now rejected explicitly alongside keys and unpacking. Cover the three reported cases plus a by-reference item, a plain variable read and a defined class constant in both the fold-decision test and the PHPT. --- phpunit/code/count-literal-fold-safe.php | 5 +- phpunit/code/count-literal-fold-unsafe.php | 23 +++++++++ phpunit/src/CountLiteralFoldTest.php | 8 +-- src/Optimizer/FuncCallOptimizer.php | 33 ++++++++---- tests/compiler/array/count-literal-fold.phpt | 54 ++++++++++++++++++-- 5 files changed, 103 insertions(+), 20 deletions(-) diff --git a/phpunit/code/count-literal-fold-safe.php b/phpunit/code/count-literal-fold-safe.php index 8c777ccf..fdcfbf0e 100644 --- a/phpunit/code/count-literal-fold-safe.php +++ b/phpunit/code/count-literal-fold-safe.php @@ -8,10 +8,9 @@ function main(): void { - $a = 1; - echo count([1, 2, 3]), "\n"; echo count([[1, 2], [3]]), "\n"; - echo count([$a, -2, true, null]), "\n"; + echo count([1.5, 'text', true, false, null]), "\n"; + echo count([-2, +3, -1.5]), "\n"; echo count([]), "\n"; } diff --git a/phpunit/code/count-literal-fold-unsafe.php b/phpunit/code/count-literal-fold-unsafe.php index f9d51fcd..d78a4906 100644 --- a/phpunit/code/count-literal-fold-unsafe.php +++ b/phpunit/code/count-literal-fold-unsafe.php @@ -6,6 +6,20 @@ * @contact service@swoole.com */ +class KnownClass +{ + public const KNOWN = 1; +} + +class MagicHolder +{ + public function __get(string $name): int + { + echo "get-{$name}\n"; + return 1; + } +} + function bump(): int { echo "bump\n"; @@ -16,9 +30,18 @@ function main(): void { $rest = [1, 2, 3, 4, 5]; $i = 0; + $plain = 1; + $ref = 1; + $object = new MagicHolder(); echo count([bump(), bump()]), "\n"; echo count(['a' => 1, 'a' => 2]), "\n"; echo count([...$rest, 9]), "\n"; echo count([$i++, $i++]), "\n"; + echo count([$plain]), "\n"; + echo count([&$ref]), "\n"; + echo count([UNDEFINED_COUNT_LITERAL]), "\n"; + echo count([KnownClass::MISSING]), "\n"; + echo count(["{$object->property}"]), "\n"; + echo count([KnownClass::KNOWN]), "\n"; } diff --git a/phpunit/src/CountLiteralFoldTest.php b/phpunit/src/CountLiteralFoldTest.php index a68d30af..062a0ea9 100644 --- a/phpunit/src/CountLiteralFoldTest.php +++ b/phpunit/src/CountLiteralFoldTest.php @@ -21,9 +21,11 @@ class CountLiteralFoldTest extends TestCase { $cpp = $this->compileToCpp('count-literal-fold-unsafe.php'); - // Element side effects, a repeated key and a spread each make the - // number of AST items differ from the runtime element count. - self::assertSame(4, substr_count($cpp, 'php::fn::count(')); + // Every call in the fixture must stay on the runtime path: element + // side effects, a repeated key, a spread, a by-reference item, a + // plain variable read, a constant or class constant fetch that may + // be undefined, and an interpolated string that may call __get(). + self::assertSame(10, substr_count($cpp, 'php::fn::count(')); self::assertStringContainsString('php_bump()', $cpp); self::assertStringContainsString('i++', $cpp); } diff --git a/src/Optimizer/FuncCallOptimizer.php b/src/Optimizer/FuncCallOptimizer.php index 3c15ca65..019a4bf5 100644 --- a/src/Optimizer/FuncCallOptimizer.php +++ b/src/Optimizer/FuncCallOptimizer.php @@ -674,9 +674,10 @@ trait FuncCallOptimizer { foreach ($array->items as $item) { // [...$other] contributes an element count only known at runtime, - // and a key may collapse onto an earlier one: ['a' => 1, 'a' => 2] - // counts as one element, not two. - if ($item->unpack || $item->key !== null) { + // a key may collapse onto an earlier one (['a' => 1, 'a' => 2] + // counts as one element, not two), and a by-reference item binds + // its source variable instead of reading it. + if ($item->unpack || $item->key !== null || $item->byRef) { return false; } if (!$this->isCountFoldableItem($item->value)) { @@ -686,24 +687,34 @@ trait FuncCallOptimizer return true; } + /** + * Only expressions whose evaluation is provably free of observable effects + * may be discarded. Variables, general constant and class constant + * fetches, interpolated strings and every other expression stay on the + * runtime path: they can be undefined, autoload, throw or call __get(). + */ protected function isCountFoldableItem(Node\Expr $value): bool { - // ConstFetch also covers true, false and null. - if ($this->isScalar($value) - || $value instanceof Node\Expr\ConstFetch - || $value instanceof Node\Expr\ClassConstFetch + // Node\Scalar\String_ is the literal string only; an interpolated + // string is a distinct Node\Scalar\InterpolatedString node. + if ($value instanceof Node\Scalar\Int_ + || $value instanceof Node\Scalar\Float_ + || $value instanceof Node\Scalar\String_ ) { return true; } + // The language constants only. Any other name may be undefined and + // must still raise the same Error PHP raises. + if ($value instanceof Node\Expr\ConstFetch) { + return in_array(strtolower($value->name->toString()), ['true', 'false', 'null'], true); + } if ($value instanceof Node\Expr\UnaryMinus || $value instanceof Node\Expr\UnaryPlus) { - return $this->isCountFoldableItem($value->expr); + return $value->expr instanceof Node\Scalar\Int_ || $value->expr instanceof Node\Scalar\Float_; } if ($value instanceof Node\Expr\Array_) { return $this->isCountFoldableArray($value); } - // A defined variable is a plain read; an undefined one must reach the - // dynamic path so it still reports the same diagnostic as PHP. - return $this->isVarExpr($value) && is_string($value->name) && $this->hasVar($value->name); + return false; } protected function doFoldKnownClass(Node\Expr\FuncCall $expr): string|false diff --git a/tests/compiler/array/count-literal-fold.phpt b/tests/compiler/array/count-literal-fold.phpt index ade723fc..d8552362 100644 --- a/tests/compiler/array/count-literal-fold.phpt +++ b/tests/compiler/array/count-literal-fold.phpt @@ -2,6 +2,20 @@ count() on an array literal keeps spreads, duplicate keys and element side effects --FILE-- getMessage(), "\n"; + } + + // A missing class constant on a known class must also still throw. + try { + var_dump(count([KnownClass::MISSING])); + echo "class-constant-error-not-thrown\n"; + } catch (Error $e) { + echo "caught=", $e->getMessage(), "\n"; + } + + // An interpolated string may invoke __get(), which must still happen. + $object = new MagicHolder(); + var_dump(count(["{$object->property}"])); + + // A by-reference item binds the source variable instead of reading it. + $ref = 1; + var_dump(count([&$ref])); + + // A defined class constant is still evaluated, not discarded. + var_dump(count([KnownClass::KNOWN])); + // Plain literals stay eligible for the compile-time fold. - $a = 1; var_dump(count([1, 2, 3])); var_dump(count([[1, 2], [3]])); - var_dump(count([$a, -2, true, null])); + var_dump(count([1.5, 'text', true, false, null])); + var_dump(count([-2, +3, -1.5])); var_dump(count([])); } ?> @@ -41,7 +82,14 @@ int(1) int(6) int(2) int(2) +caught=Undefined constant "UNDEFINED_COUNT_LITERAL" +caught=Undefined constant KnownClass::MISSING +get-property +int(1) +int(1) +int(1) int(3) int(2) -int(4) +int(5) +int(3) int(0)