Hi
Am 2025-07-22 22:02, schrieb Larry Garfield:
It seems the discussion has quieted down and wasn't particularly
contentious to begin with (whew), so we're just about ready for a vote.
However, Arnaud went on vacation and didn't remember to tell me when
he'd be back. :-) So I'm going to wait a few more days just in case he
has any last minute comments, but start the vote either when he returns
or Monday the 28th, whichever comes first. (That gets the vote
complete before the deadline for 8.5.)
Besides my remarks about the contents of the RFC, I've also heard some
feedback of other core developers being concerned about the complexity
of the implementation, particularly at this point in the release cycle.
The current PR adds about 6000 lines in total (including tests).
Excluding tests it is a net addition of roughly 2000 lines of code deep
in the engine:
Zend/Optimizer/compact_literals.c | 1 +
Zend/Optimizer/optimize_func_calls.c | 6 +-
Zend/Optimizer/zend_call_graph.c | 2 +
Zend/Optimizer/zend_inference.c | 1 +
Zend/zend.c | 3 +
Zend/zend_API.h | 3 +-
Zend/zend_ast.c | 144 ++++++++++++++++-
Zend/zend_ast.h | 17 ++
Zend/zend_builtin_functions.c | 4 +
Zend/zend_closures.c | 50 ++++--
Zend/zend_closures.h | 3 +
Zend/zend_compile.c | 220
++++++++++++++++++++++---
Zend/zend_compile.h | 5 +-
Zend/zend_execute.c | 87 ++++++----
Zend/zend_execute.h | 19 +++
Zend/zend_fibers.c | 3 +
Zend/zend_language_parser.y | 27 ++--
Zend/zend_object_handlers.c | 12 +-
Zend/zend_object_handlers.h | 2 +-
Zend/zend_partial.c | 1237
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Zend/zend_partial.h | 67 ++++++++
Zend/zend_types.h | 4 +
Zend/zend_vm_def.h | 218
++++++++++++++++++++++++-
Zend/zend_vm_execute.h | Bin 2280557 -> 2293393
bytes
Zend/zend_vm_handlers.h | Bin 93176 -> 93469 bytes
Zend/zend_vm_opcodes.c | Bin 9505 -> 9667 bytes
Zend/zend_vm_opcodes.h | Bin 13859 -> 14051 bytes
configure.ac | 1 +
ext/opcache/jit/zend_jit.c | 3 +
ext/opcache/jit/zend_jit_helpers.c | 12 ++
ext/opcache/jit/zend_jit_ir.c | 15 ++
ext/opcache/jit/zend_jit_vm_helpers.c | 16 +-
ext/opcache/zend_file_cache.c | 4 +-
ext/opcache/zend_persist.c | 1 +
ext/opcache/zend_persist_calc.c | 2 +
ext/reflection/php_reflection.c | 58 ++++++-
ext/reflection/php_reflection.stub.php | 2 +
ext/reflection/php_reflection_arginfo.h | Bin 111516 -> 111497
bytes
ext/zend_test/fiber.c | 3 +
win32/build/config.w32 | 2 +-
40 files changed, 2152 insertions(+), 102 deletions(-)
For comparison: Clone-with was 700 lines, turning clone into a function
was 200 lines, #[\NoDiscard] was 1050 lines, the (void) cast was 150
lines (all *including* tests). All these RFCs together (including
tests!) are smaller than the implementation of PFA.
The current proof of concept PR for PFA at
https://github.com/arnaud-lb/php-src/pull/12 has 5 open ToDo items in
the initial PR description and CI is red.
I've tested the current version of the PR against Tideways and noticed
that calls to PFA Closures are completely invisible to observers and the
`zend_test` observers confirm this. For:
<?php
function foo(int $a, int $b, int $c, int $d): int
{
return array_sum([$a, $b, $c, $d]);
}
function main() {
$f = foo(1, ?, ?, 4);
$f = $f(2, ?);
var_dump($f(3));
}
main();
running `sapi/cli/php -d zend_test.observer.enabled=1 -d
zend_test.observer.observe_all=1 test.php` outputs:
<!-- init 'test.php' -->
<file 'test.php'>
<!-- init main() -->
<main>
<!-- init array_sum() -->
<array_sum>
</array_sum>
<!-- init var_dump() -->
<var_dump>
int(10)
</var_dump>
</main>
</file 'test.php'>
Erroneously indicating that `array_sum()` is directly called by
`main()`. The call to `foo()` is nowhere to be seen.
As of now the implementation very clearly is incomplete. Given the
complexity of the implementation, there is a significant risk that it
cannot be stabilized until hard freeze and that “amendments” to the RFC
will need to be made due to unexpected issues popping up during the
finalization of the implementation and review.
Best regards
Tim Düsterhus
