Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126011
Precedence: bulk
MIME-Version: 1.0
Date: Wed, 20 Nov 2024 22:22:23 +0100
Message-ID: <CAPyj-LD+X-XZoc5kZmW2qTCuu1-q5NtqEZyZhEs0rhvXa2AyJg@mail.gmail.com>
Subject: [PHP-DEV] opcache_compile_file() declares top-level functions
To: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
From: tovilo.ilija@gmail.com (Ilija Tovilo)

Hi everyone

We recently received a bug report regarding the behavior of
opcache_compile_file() [1]. The documentation specifies:

https://www.php.net/manual/en/function.opcache-compile-file.php

> This function compiles a PHP script and adds it to the opcode cache without executing it. This can be used to prime the cache after a Web server restart by pre-caching files that will be included in later requests.

Arguably, "without executing it" implies that, aside from putting the
script into opcache, there are no other observable side-effects. This
assumption is currently incorrect. To be more specific,
opcache_compile_file() differs from require in two ways:

* The "main" function of the script (containing all the code at the
top-level) is not executed.
* Classes will not be added to the class table.

Confusingly, top-level functions _will_ be added to the function
table. This has some weird consequences:

* opcache_compile_file() can be called multiple times on files
containing classes. However, the same is not true for files containing
functions. The second call will lead to a function redeclaration
error.

```php
// index.php
opcache_compile_file(__DIR__ . '/test.php');
opcache_compile_file(__DIR__ . '/test.php');

// test.php
class Foo {} // No problem
function foo() {} // Fatal error: Cannot redeclare function foo()
```

* Similarly, after calling opcache_compile_file() on files containing
classes, the same file may later be required without issues. This does
not work for files containing functions for the same reason.

```php
// index.php
opcache_compile_file(__DIR__ . '/test.php');
require __DIR__ . '/test.php';

// test.php
class Foo {} // No problem
function foo() {} // Fatal error: Cannot redeclare function foo()
```

* Mixing functions with classes is incompatible. An attempt to use one
of the classes from one of the functions will either error because of
an undeclared class, or trigger the autoloader and include the file
again, leading to a function redeclaration error.

```php
// index.php
spl_autoload_register(function ($name) {
    if ($name === 'Foo') {
        require __DIR__ . '/b.php';
    }
});
opcache_compile_file(__DIR__ . '/test.php');
foo();

// test.php
class Foo {}
function foo() {
    // Triggers the autoloader, the autoloader fails due to:
    // Fatal error: Cannot redeclare function foo()
    var_dump(new Foo());
}
```

* Functions that are conditionally declared will not be added to the
function table, since they are not top-level functions, even if the
condition always evaluates to true.

```php
// index.php
opcache_compile_file(__DIR__ . '/test.php');
foo(); // Uncaught Error: Call to undefined function foo()

// test.php
if (true) {
    function foo() {}
}
```

This behavior is inconsistent and confusing. Arguably, the correct
behavior is to never register functions in opcache_compile_file() to
begin with, since opcache_compile_file() exists to prime the cache,
rather than execute code. I created a PR with this change [2]. This is
breaking, since code using opcache_compile_file() might currently
depend on functions being declared and then calling them directly.
Such code would have to be adjusted to use require instead.

Are there any concerns with making this change for 8.5? Are there any
use-cases this would break?

Of note is that it may be beneficial to provide a related function
that allows compiling files and declaring symbols without executing
the "main" function. This could be useful static analysis tools that
want to reflect on files without executing them. That said, there are
existing solutions in userland that solve this problem in a better
way, like BetterReflection [3].

Ilija

[1] https://github.com/php/php-src/issues/16668
[2] https://github.com/php/php-src/pull/16862
[3] https://github.com/Roave/BetterReflection