Emit more node callbacks after their expressions' results are stored - #6278
Conversation
…essed The dimensions loop in doPrepareTarget() fired an intermediate chain link's node callback before evaluating its dimension and before the link's write-flavoured result existed, so callback-side consumers (NonexistentOffsetInArrayDimFetchRule, DependencyResolver) re-walked the yet-unstored sub-expressions. The callback now fires at the end of the iteration, with the link's entry scope, after its result is stored.
InvalidKeyInArrayItemRule asks about the item's key at the ArrayItem callback - firing the callback after the key and value walks lets it answer from the storage instead of re-walking them.
…their expressions These statements' rules (UnsetRule, ValueAssignedToClassConstantRule, ValueAssignedToGlobalConstantRule, WhileLoopAlwaysFalseConditionRule) read the statement's child expressions, so the statement callback now fires inside the handler after those expressions are processed - with the entry scope, joining the existing deferred set (Return_, Expression, Echo_, If_, Switch_, Foreach_). The per-constant Const node callbacks move after their value walks for the same reason.
ClassAttributesRule fires on InClassNode and reads the attribute constructor arguments - walking the attribute groups first lets it answer from the storage.
…nal condition walk
|
Verified - this fixes phpstan/phpstan#15112, and the mechanism it closes is exactly the one I measured there. All numbers below: M-series mac, PHP 8.5.8, no turbo extension, interleaved A/B of 1. The regression is gone.
-21.7%, rstdev <=1.5%. In a separate interleaved run earlier today 2. It closes the specific gap, not just the symptom. Counters on that file:
The two extra full re-prices of the 15,000-element literal are gone - the 3. No cost elsewhere. Ten subjects measured; 4. Independent census, on the bench corpus rather than the test suite. 185,350 stored-ask hits against 1,338 misses (0.7%), and I classified each miss by whether the same node object is stored later in the run:
That matches your own classification of the leftovers as fabrications, clones and forward asks. Worth keeping in mind that a never-stored miss still re-prices the expression, so it is cheap only because these shapes are cheap - a literal the size of 5. The riskiest hunk holds up. For "store the AssignOp result before
One suggestion, not a defect: fabricating a partial |
2 participants
Extracted from
resolve-type-rewrite-2(extraction series, follows #6274/#6275). Continues the post-order callback emission that #6248 established (ae03ab3ec1,1dc598762f,8bb4d4d352): each reorder makes rule-sidegetType()asks answer from the storage instead of re-walking yet-unstored sub-expressions on demand.Six reorders, each gated individually with the full test suite — zero expectation churn at every step:
processArgsarms, emission scope unchanged).$a[$x][$y] = …) emit after their dimension was processed, with the link's entry scope — defer-only on mainline, matching the whole-target idiom from1dc598762f.applyWrite()emits assignment nodes, soPropertyAssignNoderules asking about the whole$lvalue OP= valueexpression answer from the storage. The stored entry is the same before-scope-anchored result the engine stores after the handler returns, just visible earlier.Unset_,ClassConst,Const_andWhile_statement callbacks are deferred past their expressions, joiningReturn_/Expression/Echo_/If_/Switch_/Foreach_. InWhileHandlerthe deferred emission sits in both arms of the Replay the recorded fixpoint pass instead of repeating the final loop walk #6249 replay split: after the condition's replay, or after its real walk, before the body either way.InClassNodeemission, so rules firing on it (e.g.ClassAttributesRule) read the attribute arguments from the storage.Gates: full suite 21155 tests / 96002 assertions green after each commit,
make phpstanclean,make csclean. None of the touched files have turbo mirrors; only existingExpressionResultStorageAPI calls are added.🤖 Generated with Claude Code
https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
Round 2 — miss census. A temporary census guard in
NodeCallbackScope(log every stored-result miss with the emitting callback node, the asked expression, and the asking rule) over the full test suite + self-analysis found three more real emission-order gaps, fixed in the follow-up commits:Switch_fired before its case conditions were walked —MatchingTypeInSwitchCaseConditionRule(strict-rules) re-priced every case condition on demand (~250 walk-scope re-pricings on self-analysis). The deferred emission now sits after the cases.Do_was not deferred at all, andDoWhileLoopConditionNodefired before the final condition walk — both now emit after the condition's results are stored.PropertyAssignNoderules asking about the whole$x++missed the storage; all four handlers now store a before-scope-anchored result before the virtual assign runs.Everything else the census surfaced is classified non-actionable: rule-fabricated synthetic nodes (including
DataProviderDataRule's simulated test-method calls, which carry a realstartLinebut areTypeExpr-based fabrications), aggregate-node forward asks (ClassPropertiesNodescanning bodies), O(1) variable state reads, and a handful ofArgumentsNormalizerclones.Gates re-run after the instrumentation was removed: full suite 21155/96002 green,
make phpstanclean,make csclean — zero churn.