[DeepClone] Reference const-expr closures through engine ids on PHP 8.6#633
Open
nicolas-grekas wants to merge 6 commits into
Open
[DeepClone] Reference const-expr closures through engine ids on PHP 8.6#633nicolas-grekas wants to merge 6 commits into
nicolas-grekas wants to merge 6 commits into
Conversation
nicolas-grekas
force-pushed
the
deepclone-engine-ids
branch
from
June 11, 2026 16:22
d3bd802 to
ea28070
Compare
nicolas-grekas
force-pushed
the
deepclone-engine-ids
branch
from
June 11, 2026 17:25
ea28070 to
1d3ebac
Compare
…rank>" ids One wire format on every PHP version: [class, "<site>@<rank>", line], where the id names the declaring reflection element and the closure rank within just that element, matching ReflectionFunction::getConstExprId(). The per-class index enumerates one element at a time (attributes in declaration order, nested const-expr surfaces depth-first, then parameter defaults, then constant/property values, which extend the rank space past what the engine addresses); on PHP 8.6 anonymous closures short-circuit through getConstExprId(). Resolution parses the id, tries Closure::fromConstExpr() first on PHP 8.6 (with a line check), and falls back to evaluating only the named element. The 5-element site form and its attrIndex dimension are removed, and runtime closures now refuse with NotInstantiableException on every version.
…he declaring class An absolute line invalidated every cached reference in a file when anything above the class shifted (a new use import, another declaration). Anchoring the line to the declaring class (ReflectionClass::getStartLine) keeps the reference valid across such edits and matches the extension, so payloads stay byte-identical across both implementations.
…d by the engine code hash Match the engine, which embeds a code hash in the id and dropped the line field: the marker-1 payload is [class, id] (no line). On PHP 8.6 the engine id carries the hash and is verified by Closure::fromConstExpr; the polyfill cannot recompute it, so on 8.5, and for constant/property values, the id is hash-less and resolves positionally. Decode strips an unverifiable hash before the value-walk, and surfaces a hash-bearing id the engine rejects as stale.
…e()/unserialize() on PHP 8.6 Mirror the extension: read a const-expr closure's [class, id] declaration-site reference from Closure::__serialize() (for the attribute arguments and parameter defaults the engine addresses, falling back to the reflection walk for constant and property default values), and resolve it back by synthesizing the serialized form and unserialize()-ing it with the allowed classes. This replaces the reflection/reconstruction API (getConstExprClass, getConstExprId, fromConstExpr). The native resolution runs before the rank parse so the engine's name-keyed first-class-callable ids resolve, and a stale hash-bearing reference now surfaces the engine's own exception. Payloads interchange with the extension on 8.6 and resolve on 8.5 for anonymous closures.
Mirror the engine's marker-1 wire format: the reference carried under
mask 1 is now [class, site, key, hash] instead of [class, "<site>@<rank>"]
with an optional "#<hash>" suffix. key is an integer rank for an anonymous
closure or the callable's name for a first-class callable; hash is the
engine's 32-bit code hash (0 on PHP 8.5 and for the const/property value
walk, which cannot recompute it, and for first-class callables).
On PHP 8.6 the reference comes straight from Closure::__serialize() and is
resolved by synthesizing the engine's own a:4:{...} payload; the reflection
value-walk reads the four fields and resolves an anonymous rank
positionally, rejecting a string key it cannot place. Payloads stay
byte-identical to the extension on 8.6 and interchangeable with 8.5.
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
1 participant
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 the polyfill use those ids, gated on\PHP_VERSION_ID >= 80600, assuming the RFC lands.deepclone_to_array()emits[class, id, line]under the existing mask marker, from a singlegetConstExprId()call. Everything that existed only because userland could not see closure identity stops being needed for these sites: the per-class evaluated index, the name/file/line/signature keys, the token-level aliasing guard, and the documented refusals all become moot, since same-line literals get distinct ids and no source files are read. 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 (and, on 8.6, the index lookup ignores attribute and parameter-default candidates, which can only be stale line-mates there).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 throughClosure::fromConstExpr()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. The allow-list gates are unchanged on both sides, including the payload-class check before autoloading.Closure::__serialize()refusal (Serialization of 'Closure' is not allowed) instead ofNotInstantiableException, matching whatserialize()itself says on 8.6.Measured on the patched engine (release build): encoding an attribute closure goes from 3.2 us to 1.6 us with no more ~30 us cold per-class index, and decoding from 2.5 us to 1.2 us; both implementations now also outperform native
serialize()/unserialize()of the same closures, which carry the wrapping format.Payloads stay byte-identical and interchangeable with the extension, now covered by an in-suite interchange test that runs when the extension is loaded. The suite passes on PHP 8.5 (current behavior unchanged) and on the patched 8.6 build with the extension compiled in, on both the native and polyfill legs.
Companion extension implementation: symfony/php-ext-deepclone#24
Expected CI failures until dependencies ship: the 8.2-8.5 lanes with the extension enabled fail on the two new payload-validation tests because they install the latest tagged extension, which predates the engine-id form; they go green once symfony/php-ext-deepclone#24 is released (same sequence as #630/#632). The 8.6 lanes run php-src master, which does not include the engine patch yet, so
ReflectionFunction::getConstExprId()is undefined there; they go green once the engine implementation lands.