Merge pull request #24 from Giandonn/fix/count-literal-fold-side-effects --skip-tests

fix(optimizer): stop folding count() on unfoldable array literals

phpunit/code/count-literal-fold-safe.php

@@ -0,0 +1,16 @@
+<?php
+/**
+ * This file is part of TypePHP(AOT).
+ *
+ * @link     https://www.swoole.com/aot/
+ * @contact  service@swoole.com
+ */
+
+function main(): void
+{
+    echo count([1, 2, 3]), "\n";
+    echo count([[1, 2], [3]]), "\n";
+    echo count([1.5, 'text', true, false, null]), "\n";
+    echo count([-2, +3, -1.5]), "\n";
+    echo count([]), "\n";
+}

phpunit/code/count-literal-fold-unsafe.php

@@ -0,0 +1,47 @@
+<?php
+/**
+ * This file is part of TypePHP(AOT).
+ *
+ * @link     https://www.swoole.com/aot/
+ * @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";
+    return 1;
+}
+
+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";
+}

phpunit/src/CountLiteralFoldTest.php

@@ -0,0 +1,52 @@
+<?php
+/**
+ * This file is part of TypePHP(AOT).
+ *
+ * @link     https://www.swoole.com/aot/
+ * @contact  service@swoole.com
+ */
+
+namespace TypePhp\Tests;
+
+use PHPUnit\Framework\TestCase;
+use TypePhp\CompilerTest;
+
+/**
+ * @internal
+ * @coversNothing
+ */
+class CountLiteralFoldTest extends TestCase
+{
+    public function testUnfoldableArrayLiteralsKeepTheRuntimeCall(): void
+    {
+        $cpp = $this->compileToCpp('count-literal-fold-unsafe.php');
+
+        // 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);
+    }
+
+    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));
+    }
+}

src/Optimizer/FuncCallOptimizer.php

@@ -656,11 +656,67 @@ 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,
+            // 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)) {
+                return false;
+            }
+        }
+        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
+    {
+        // 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 $value->expr instanceof Node\Scalar\Int_ || $value->expr instanceof Node\Scalar\Float_;
+        }
+        if ($value instanceof Node\Expr\Array_) {
+            return $this->isCountFoldableArray($value);
+        }
+        return false;
+    }
+
     protected function doFoldKnownClass(Node\Expr\FuncCall $expr): string|false
     {
         // An explicit $autoload argument must still be evaluated, including

tests/compiler/array/count-literal-fold.phpt

@@ -0,0 +1,95 @@
+--TEST--
+count() on an array literal keeps spreads, duplicate keys and element side effects
+--FILE--
+<?php
+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";
+    return 1;
+}
+
+function main()
+{
+    // Element expressions must still run.
+    var_dump(count([bump(), bump()]));
+
+    // A repeated key collapses onto the first one.
+    var_dump(count(['a' => 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);
+
+    // An undefined constant must still raise the same Error PHP raises.
+    try {
+        var_dump(count([UNDEFINED_COUNT_LITERAL]));
+        echo "constant-error-not-thrown\n";
+    } catch (Error $e) {
+        echo "caught=", $e->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.
+    var_dump(count([1, 2, 3]));
+    var_dump(count([[1, 2], [3]]));
+    var_dump(count([1.5, 'text', true, false, null]));
+    var_dump(count([-2, +3, -1.5]));
+    var_dump(count([]));
+}
+?>
+--EXPECT--
+bump
+bump
+int(2)
+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(5)
+int(3)
+int(0)