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.

Author: Giandonn

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";
 }

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";
 }

phpunit/src/CountLiteralFoldTest.php

@@ -21,9 +21,11 @@ public function testUnfoldableArrayLiteralsKeepTheRuntimeCall(): void
     {
         $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);
     }

src/Optimizer/FuncCallOptimizer.php

@@ -674,9 +674,10 @@ 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) {
+            // 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 @@ protected function isCountFoldableArray(Node\Expr\Array_ $array): bool
         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

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--
 <?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";
@@ -25,11 +39,38 @@ function main()
     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.
-    $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)