Narrow division to int when modulo is known to be zero #5757
Narrow division to int when modulo is known to be zero #5757phpstan-bot wants to merge 3 commits into
Conversation
…s in `integerRangeMath` - Return benevolent union from `integerRangeMath` when division of two integer types produces `int<range>|float`, matching the existing behavior for plain `int / int` division - Propagate benevolence through union-combining code in both `integerRangeMath` (operand union handling) and `resolveCommonMath` (left number type union handling) by tracking whether any part returned a `BenevolentUnionType` - Simplify the cross-zero range-splitting blocks to use `TypeUtils::toBenevolentUnion()` instead of an equality check against `UnionType([int, float])`, which missed bounded integer range results - Update test expectations in integer-range-types.php, div-by-zero.php, math.php, and binary.php to reflect the corrected benevolent union types
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
Wouldn't it be instead possible to say that
0 === ($offset % $limit) imply $offset / $limit is an int ?
Instead of making all integer range divisions return benevolent unions, detect when the scope knows that `$a % $b === 0` and narrow `$a / $b` to just the integer type (removing float from the union). This is more precise: it only removes float when mathematically guaranteed, rather than broadly making all integer range divisions benevolent. The check works by constructing a synthetic modulo expression with the same operands and querying the scope for its type. When a condition like `0 === ($a % $b)` has narrowed the modulo to ConstantIntegerType(0), the division result is guaranteed to be int. Reverts the BenevolentUnionType propagation changes from the previous commit since the modulo narrowing is the correct fix. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pushed. Here's a summary of what changed: Approach change: Instead of making all integer range divisions return benevolent unions (the previous commit's approach), the fix now detects when How it works: In Changes:
|
VincentLanglet
changed the title
BenevolentUnionType for IntegerRangeType division results in integerRangeMath| return $this->getDivTypeFromTypes($left, $right, $leftType, $rightType); | ||
| $result = $this->getDivTypeFromTypes($left, $right, $leftType, $rightType); | ||
|
|
||
| if ($leftType->isInteger()->yes() && $rightType->isInteger()->yes()) { |
There was a problem hiding this comment.
Should this logic be in getDivTypeFromTypes instead ?
There was a problem hiding this comment.
Pushed. The change moves the modulo-based narrowing logic from getDivType into getDivTypeFromTypes, addressing VincentLanglet's review comment. getDivType now stays a thin wrapper (consistent with getMulType, getModType, etc.) — it resolves the mod type via the callback and passes it to getDivTypeFromTypes as an optional ?Type $modType = null parameter. The actual narrowing (TypeCombinator::remove($result, new FloatType())) happens inside getDivTypeFromTypes.
All tests pass (12154 tests, 79829 assertions) and make phpstan reports no errors.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Fixes phpstan/phpstan#9724