Skip to content

Apply specified types in place on a batched working copy - #6168

Open
ondrejmirtes wants to merge 6 commits into
2.2.xfrom
apply-specified-types-in-place
Open

Apply specified types in place on a batched working copy#6168
ondrejmirtes wants to merge 6 commits into
2.2.xfrom
apply-specified-types-in-place

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Member

Applying a SpecifiedTypes batch derived one whole scope — a full holder-map copy plus scope construction — per specification, and one more per array-dim level of the parent-intersection cascade inside specifyExpressionType(). This PR restructures the apply side to write in place:

  • specifyExpressionType() opens a single unpublished copy (openSpecificationScope()) and writes holders straight into its maps (specifyExpressionTypeInPlace()). Every lazily-derived view (resolvedTypes, truthy/falsey scopes, the promoted-native scope) is reset after each write, so every read answers exactly as it did on a per-specification fresh scope.
  • The application loop batches all consecutive specifications into one working copy, inlining the addTypeToExpression()/removeTypeFromExpression() math; operations that go through other scope derivations (isset certainty, overwriting assignment) publish it and the next specification opens a fresh one. The conditional-expressions tail moves to processConditionalExpressionsAfterSpecifying().
  • The type-specification list is now built inline at its only caller, with alternative-form entries sorted together with sure/sure-not entries — restoring the shortest-expression-first guarantee (parents before children) for them too. ScopeOps::buildTypeSpecifications() and its native mirror are removed (smoke differential and signature parity green).
  • filterBySpecifiedTypes() is renamed to applySpecifiedTypes() — the method does not filter, it applies computed narrowing. It is public but not @api.
  • The isset-certainty path keeps the held type instead of re-reading it via getType(), which broadens Maybe-certainty holders and would overwrite a co-applied narrowing.

Zero expectation churn: full test suite green in both turbo modes (17844 tests), self-analysis and CS clean, turbo smoke differential + signature parity green.

Benchmark note: the local machine is currently running a backup; I will post a commit-toggled ABBA user-CPU comparison as a comment once it quiets down.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7

ondrejmirtes and others added 6 commits July 31, 2026 21:39
specifyExpressionType() derived one whole scope per specification - and one
per array-dim level of the parent-intersection cascade. It now opens a single
unpublished copy (openSpecificationScope) and writes holders straight into its
maps (specifyExpressionTypeInPlace), resetting the lazily-derived views
(resolvedTypes, truthy/falsey scopes, promoted-native scope) after each write
so every read answers exactly as it did on a per-specification fresh scope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
filterBySpecifiedTypes() chained addTypeToExpression()/removeTypeFromExpression(),
deriving a whole scope per specification. The application loop now inlines their
math and writes every consecutive specification into a single unpublished working
copy; operations that go through other scope derivations (isset certainty,
overwriting assignment) publish it and the next specification opens a fresh one.
The conditional-expressions tail moves to processConditionalExpressionsAfterSpecifying(),
mirroring the eventual applySpecifiedTypes() shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
…tries in

The alternative-form entries of a SpecifiedTypes batch were appended after the
sorted sure/sure-not list; sorting all three kinds together restores the
shortest-expression-first guarantee (parents specified before their children)
for them too. With the building inlined at its only caller,
ScopeOps::buildTypeSpecifications() and its native mirror are removed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
The method does not filter - it applies computed narrowing to the scope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
The isset-certainty path of applySpecifiedTypes() re-read the expression's
type via getType() before re-specifying it with Maybe certainty. getType()
only reports the type of Yes-certainty holders, so for a maybe-defined
expression it broadens to the original type - which would overwrite a
co-applied narrowing (e.g. isset's $a -> null in the else branch).
Read the held type from the holder maps instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
@ondrejmirtes

Copy link
Copy Markdown
Member Author

Benchmark (machine now quiet): commit-toggled ABBA, same tree, no-turbo, bin/phpstan analyse src/Analyser src/Rules with cleared result cache, user CPU, 6 pairs.

  • base (2.2.x): median 49.63s — [48.63, 49.32, 49.37, 49.90, 50.26, 51.17]
  • this PR: median 49.73s — [47.49, 48.58, 48.82, 50.64, 50.80, 51.10]
  • delta +0.19% median; paired mean diff −0.20s (sd 2.16s)

Neutral within this run's noise floor — the value here is convergence (and the removed native mirror is a 1–3 element build+sort per application, so its deletion is micro). Happy to run a longer full self-analysis ABBA or a turbo-mode pair if wanted.

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