Reference const-expr closures through engine ids on PHP 8.6#24
Open
nicolas-grekas wants to merge 1 commit into
Open
Reference const-expr closures through engine ids on PHP 8.6#24nicolas-grekas wants to merge 1 commit into
nicolas-grekas wants to merge 1 commit into
Conversation
130fc82 to
12688ad
Compare
12688ad to
d477057
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The proposed PHP 8.6 "Serializable closures" engine support (implementation at nicolas-grekas/php-src#4) gives every closure declared in an attribute argument or in a parameter default value a canonical per-class id, derived from a deterministic, non-evaluating walk over the class's constant expressions, exposed as
ReflectionFunction::getConstExprId()and resolved byClosure::fromConstExpr(). This PR makes deepclone use those ids, gated onPHP_VERSION_ID >= 80600, assuming the RFC lands.deepclone_to_array()calls the exportedzend_constexpr_closure_ref()and emits[class, id, line]under the existing mask marker. This replaces the per-call declaration-site scan, which had to evaluate every preceding site to count closures: on a 60-site class, encoding the last closure drops from 8.3 us to 1.2 us, and resolving it from 0.3 us scan-equivalents to a 17 ns/site pointer walk. Closures declared in class constant values and in property default values are evaluated in place by the engine and have no id; they keep the site-based 5-element form.deepclone_from_array()accepts both forms on every PHP version, discriminated by the type of element 1 (int id vs string site). Site-based payloads written on PHP 8.5 keep resolving on PHP 8.6, so caches survive the upgrade; engine-id payloads on older PHP fail with a message saying they need PHP 8.6. Resolution goes throughzend_constexpr_closure_site_by_id()plus the same staleness check as before (stale payloadwhen the declaration line moved). Crafted payloads addressing a first-class-callable site are rejected; FCCs keep the named-closure form.Closuremust be allowed before anything is evaluated on theto_arrayside, and the payload-named class is allow-list-checked beforezend_lookup_class()on thefrom_arrayside.Closure::__serialize()refusal (Serialization of 'Closure' is not allowed) instead ofNotInstantiableException, sinceClosuregains that method on 8.6.The new code only compiles on PHP >= 8.6, so the current CI matrix is unaffected; the 8.6 paths were validated locally against the patched engine (all phpts pass, and the polyfill suite passes against this build with byte-identical, cross-resolvable payloads).
deepclone_constexpr_closures_native.phptcovers the 8.6 behavior,deepclone_constexpr_closures_id_pre86.phptthe refusal on older PHP.Companion polyfill implementation: symfony/polyfill#633