Re: opcache_compile_file() declares top-level functions

Hi Ilija, I believe the removal of function caching is a bit of an overreaction: it was always known, to those who used it, that preloading will cache functions, and requires an include guard to avoid function redeclaration errors, and in fact it is a very useful feature. There is nothing wrong with this behaviour, and it does what you expect it to do. Regards, Daniil Gentili > On 20 Nov 2024, at 22:22, Ilija Tovilo <tovilo.ilija@gmail.com> wrote: > > 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