Skip to content

[DeepClone] Reference const-expr closures through engine ids on PHP 8.6#633

Open
nicolas-grekas wants to merge 1 commit into
1.xfrom
deepclone-engine-ids
Open

[DeepClone] Reference const-expr closures through engine ids on PHP 8.6#633
nicolas-grekas wants to merge 1 commit into
1.xfrom
deepclone-engine-ids

Conversation

@nicolas-grekas

@nicolas-grekas nicolas-grekas commented Jun 10, 2026

Copy link
Copy Markdown
Member
Q A
Branch? 1.x
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

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 by Closure::fromConstExpr(). This PR makes the polyfill use those ids, gated on \PHP_VERSION_ID >= 80600, assuming the RFC lands.

  • Encoding. On PHP 8.6, deepclone_to_array() emits [class, id, line] under the existing mask marker, from a single getConstExprId() 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).
  • Decoding. 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 through Closure::fromConstExpr() plus the same staleness check as before (stale payload when 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.
  • Runtime closures now surface the engine's own Closure::__serialize() refusal (Serialization of 'Closure' is not allowed) instead of NotInstantiableException, matching what serialize() 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant