Extract Type::truncateListToSize() from TypeSpecifier#5713
Merged
Conversation
ondrejmirtes
force-pushed
the
truncate-array-to-size
branch
from
eead4a2 to
0b52def
Compare
ondrejmirtes
changed the title
Type::truncateArrayToSize() from TypeSpecifierType::truncateListToSize() from TypeSpecifier
`specifyTypesForCountFuncCall` had ~100 lines of inline shape-specific
narrowing (rebuild as N-element list, build with required prefix +
optional middle, probe `hasOffsetValueType` for unbounded max,
intersect with `HasOffsetValueType` accessories for non-CAT lists,
plus several `LIMIT` bail-outs that returned the original array). The
five branches were interleaved with the same outer `count()`-call /
size-superType filters, making the per-shape logic hard to read.
Push the per-list rebuild into a new `Type::truncateListToSize(Type
$sizeType): Type`:
- `ConstantArrayType`: required prefix `[0, min)`, optional middle
`[min, max)` when `max` is set, or probe explicit offsets via
`hasOffsetValueType` until `no` when `max` is unbounded.
- `ArrayType`: same prefix/middle for bounded ranges; for unbounded
`max`, intersect with `HasOffsetValueType` accessories.
- Non-array types (`NonArrayTypeTrait`, `MaybeArrayTypeTrait`): return
`ErrorType`.
- `LateResolvableTypeTrait`: delegate to the resolved type.
- `UnionType` / `IntersectionType` / `StaticType`: dispatch.
- `NeverType` / `MixedType` / accessory types: identity-ish — the
accessories represent metadata orthogonal to size, so the narrowing
flows through `IntersectionType`'s dispatcher unchanged.
The method is named for its actual contract: each implementation
assumes a list shape. The call site (`TypeSpecifier`) is responsible
for gating on outer list-ness — a CAT inside a `non-empty-list<T>`
intersection may have its own `isList()` weakened to `Maybe` even
though the aggregate is definitely a list. Letting the call site
decide preserves the original behavior exactly.
A private static helper `ConstantArrayType::extractTruncateListBounds()`
peels `[min, max]` out of either a `ConstantIntegerType` (`[N, N]`) or
an `IntegerRangeType`, sparing both implementations the same two-line
shape check.
Behavior preserved: full test suite green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ondrejmirtes
force-pushed
the
truncate-array-to-size
branch
from
0b52def to
b3ead90
Compare
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.
Summary
TypeSpecifier::specifyTypesForCountFuncCallhad ~100 lines of inline shape-specific narrowing — rebuild as N-element list, build with required prefix + optional middle, probehasOffsetValueTypefor unbounded max, intersect withHasOffsetValueTypeaccessories for non-CAT lists, plus severalLIMITbail-outs that returned the original array. Five interleaved branches sharing the same outercount()-call / size-superType filters, hard to read.This PR extracts the per-array rebuild into
Type::truncateArrayToSize(Type \$sizeType): Type:ConstantArrayType: required prefix[0, min), optional middle[min, max)whenmaxis set, or probe explicit offsets viahasOffsetValueTypeuntilnowhenmaxis unbounded.ArrayType: same prefix/middle for bounded ranges; for unboundedmax, intersect withHasOffsetValueTypeaccessories.NonArrayTypeTrait,MaybeArrayTypeTrait): returnErrorType.LateResolvableTypeTrait: delegate to the resolved type.UnionType/IntersectionType/StaticType: dispatch.NeverType/MixedType/ accessory types: identity-ish — the accessories represent metadata orthogonal to size, so the narrowing flows throughIntersectionType's dispatcher unchanged.isList()gate stays at the call siteThe per-array methods deliberately do not check
isList(). The outer aggregate gate stays at the call site — a CAT inside anon-empty-list<T>intersection may have its ownisList()weakened toMaybeeven though the aggregate is definitely a list. Hoisting the check into the methods broke ~30 tests; restoring it at the call site preserved exact behavior.Helper
A private static helper
ConstantArrayType::extractTruncateBounds()peels[min, max]out of either aConstantIntegerType([N, N]) or anIntegerRangeType, sparing both implementations the same two-line shape check. TheTypesignature acceptsType \$sizeType(notIntegerRangeType) becauseIntegerRangeType::fromInterval(N, N)collapses toConstantIntegerType.Behavior preserved.
Test plan
make tests— all 12,170 tests pass, no regressions vs.origin/2.1.x🤖 Generated with Claude Code