Resolve isset/empty/?? chains from handler-built descriptors - #6144
Merged
Conversation
Rules\IssetCheck was a hand-maintained mirror of MutatingScope::issetCheck's chain walk (variable / offset / property / leaf), re-walking the AST and re-resolving types and property reflections with its own drifting copy of the logic. The chain-link handlers (Variable / ArrayDimFetch / PropertyFetch / StaticPropertyFetch) now attach an IssetabilityDescriptor to their ExpressionResult, built from the child results they already have in hand. IssetHandler, EmptyHandler, CoalesceHandler and AssignOpHandler (??=) emit virtual nodes carrying the subject ExpressionResults; IssetRule, EmptyRule and NullCoalesceRule listen on those nodes, and Rules\IssetCheck folds the resolved links of an IssetabilityResolution instead of walking the AST. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
ondrejmirtes
force-pushed
the
isset-resolution-rework
branch
from
July 28, 2026 20:15
e800ce8 to
6f9986a
Compare
ondrejmirtes
changed the title
… ??= read processAssignVar owned the whole assignment timeline - target sub-expression walk, value evaluation, write - so callers had to inject the value evaluation as a processExprCallback closure invoked once per target-shape branch. The invocation point is the same in every branch: after the target's sub-expressions are walked, before the write - PHP's own evaluation order. Splitting at that boundary gives the caller the timeline: prepareTarget() walks the target into a PreparedAssignTarget, the caller processes the assigned value inline on its scope, and applyWrite() performs the write. prepareTarget() takes an AssignTargetWalkMode; in the coalesce mode it also prices the whole target as a read with isset() semantics, composed from the just-walked child results through the chain-link handlers' new composeResult() methods (the pure tails of their processExpr, which routes through the same method) - so AssignOpHandler's ??= subject result for CoalesceExpressionNode no longer needs the separate noop-callback walk of the target chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
Member
Author
|
Second commit ports the AssignHandler |
Two leftover noop-callback walks over real AST: - A dynamic-name read-modify-write target (`$$name op= ...`) priced the whole variable through the walk machinery because the name expression was only walked later by the write flow. PHP evaluates the name before reading the old value, so prepareTarget() now walks it once - with the real node callback, at its actual evaluation point - composes the read from the result, and threads it on the PreparedAssignTarget; the write flow consumes it instead of walking the name again after the value. - The ExistingArrayDimFetch branch walked the cloned chain so its nodes had stored results, but the composition below prices the clones directly through the scope - the walks had no observable effect here. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
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.
Rules\IssetCheckwas a hand-maintained mirror ofMutatingScope::issetCheck()'s chain walk (variable / offset / property / leaf), re-walking the AST and re-resolving types and property reflections with its own drifting copy of the logic (the scope side even carried a// mirrored in ...comment).This lands the resolve-type-rewrite branch's architecture for the rule side, with
MutatingScopeuntouched:VariableHandler/ArrayDimFetchHandler/PropertyFetchHandler/StaticPropertyFetchHandler) attach anIssetabilityDescriptorto theirExpressionResult, built from the child results they already have in hand;IssetHandler,EmptyHandler,CoalesceHandlerandAssignOpHandler(??=) emit virtual nodes (IssetExpressionNode/EmptyExpressionNode/CoalesceExpressionNode) carrying the subjectExpressionResults;IssetRule/EmptyRule/NullCoalesceRulelisten on those nodes, andRules\IssetCheckfolds the resolved links of anIssetabilityResolutioninstead of walking the AST.IssetabilityDescriptor,IssetabilityLinkInfo, the three virtual nodes and the three rules are byte-identical to the branch;IssetabilityResolutionandRules\IssetCheckdiffer only by omitting branch-only leftovers (notEmpty()is dead until the branch'sEmptyHandlerconversion, plus a debug block). For??=,AssignOpHandlerprices the left side once as a Noop-callback read, mirroringCoalesceHandler's left-side processing — that is the branch's own shape for carrying the descriptor to the rule.The engine side (
MutatingScope::issetCheck()and the handlers'resolveType/specifyTypes) is deliberately untouched; it converges with the branch's sweep, at which pointissetCheck()is deleted and no duplicated chain-walk logic remains.Verification:
--error-format=rawoutput identical to 2.2.x on an isset-saturated stress file??/empty()chains (ABBA pairs, +0.6% on the stress file, within noise)🤖 Generated with Claude Code
https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7