Negating the smallest integer results in a float - #6029
Conversation
| /** @var -1|-2 $int */ | ||
| assertType('1|2', -$int); |
There was a problem hiding this comment.
I think we should test another union constant type, like
9223372036854775807|25 and assert the -1 result for it
There was a problem hiding this comment.
Good call. 9223372036854775807|25 negates to -9223372036854775807|-25 with no overflow, so I also added a union where only one member overflows. That one is the case the old code got wrong:
function constantUnion(int $int): void
{
/** @var 9223372036854775807|25 $int */
assertType('-9223372036854775807|-25', -$int);
}
// The union has one member that overflows and one that does not.
function partiallyOverflowingUnion(bool $bool): void
{
$int = $bool ? -9223372036854775807 - 1 : 25;
assertType('-9223372036854775808|25', $int);
assertType('-25|9.223372036854776E+18', -$int);
}Before this PR the last one inferred -9223372036854775808|25, the un-negated union, because getUnaryMinusTypeFromType() bailed out with return $type on the first member that overflowed and threw away the members it had already negated. Now it keeps going and only that member becomes a float.
I had to build the union with a ternary rather than a PHPDoc type: -9223372036854775808 degrades to int as a const type, since the unary minus applies after 9223372036854775808 has already overflowed.
Both in fcff019.
fcff019 to
3eac99f
Compare
|
Rebased this onto #6028. Both branches add a That means the first two commits here, c5889b1 and 56f8a79, belong to #6028 and are duplicated in this PR's history. GitHub won't let a PR into this repo use a fork branch as its base, so the diff keeps showing them for now. Once #6028 is squashed into 2.2.x I'll rebase this branch onto it and force-push, leaving only the two commits that belong here:
Those two are the ones worth reviewing. The |
|
|
||
| if ($this->max === null || $this->max >= 0) { | ||
| $inversedMin = $this->min !== null ? $this->min * -1 : null; | ||
| if ($this->max === PHP_INT_MIN) { |
There was a problem hiding this comment.
I wonder if it wouldn't be better to have such logic in fromInterval instead ?
Like int<min, PHP_INT_MIN> is constantInteger(PHP_INT_MIN)
And int<PHP_INT_MAX, max> is constantInteger(PHP_INT_MAX)
There was a problem hiding this comment.
Great suggestion, done in 17eef36.
$range = (new self($min, $max))->shift($shift);
if (!$range instanceof self) {
return $range;
}
// Nothing is smaller than the smallest integer, and nothing is bigger than the biggest one,
// so an unbounded side that reaches either one holds a single value.
if ($range->min === null && $range->max === PHP_INT_MIN) {
return new ConstantIntegerType(PHP_INT_MIN);
}
if ($range->min === PHP_INT_MAX && $range->max === null) {
return new ConstantIntegerType(PHP_INT_MAX);
}It runs after shift() so that createAllSmallerThan(PHP_INT_MIN) keeps returning NeverType rather than collapsing to PHP_INT_MIN.
With that, toAbsoluteNumber() loses its max === PHP_INT_MIN special case, since no IntegerRangeType can hold that bound any more. The remaining guard is for min === PHP_INT_MIN on a range that holds more than one value, where negating the bound still overflows. fromInterval() could normalize that bound to null too, since int<-9223372036854775808, 0> and int<min, 0> denote the same values, but it would change how those types describe themselves (random_int(PHP_INT_MIN, PHP_INT_MAX) would go from int<-9223372036854775808, 9223372036854775807> to int). Happy to do it if you want, though it feels like a separate change.
Everything moved to #6028, this PR is closed.
|
Merged into #6028, which now carries all five commits and closes both phpstan/phpstan#14946 and phpstan/phpstan#14947. @VincentLanglet asked why there were two PRs. They were logically independent problems, so I opened them separately, but they conflicted in |
Closes phpstan/phpstan#14947
Negating the smallest integer overflows into a float, because
9223372036854775808is not representable as anint:getUnaryMinusTypeFromType()detected the overflow but returned the un-negated type, so-PHP_INT_MINcame out asPHP_INT_MIN. It now produces aConstantFloatTypefor that value and keeps negating the remaining constants of the union.-(-9223372036854775807 - 1)-92233720368547758089.223372036854776E+18float(9.2233720368548E+18)-PHP_INT_MIN-9223372036854775808|-21474836482147483648|9.223372036854776E+18The new result matches what the multiplication path already inferred for
PHP_INT_MIN * -1, which is wheregetUnaryMinusType()routes integer ranges — so-int<min, -1>was already correct and staysint<1, max>. Only the constant-integer path changed. The test file covers the range and constant-union cases too, to pin that down.Expected values depend on the host's integer width, so the asserts live in
tests/PHPStan/Analyser/data/unary-minus-64bit.php, yielded fromNodeScopeResolverTestunderPHP_INT_SIZE === 8next topredefined-constants-64bit.php.Related
Same root cause as #6028 (
abs(PHP_INT_MIN)crashing with an internal error), but that one is aTypeErrorintoAbsoluteNumber()while this one is silently wrong. The two PRs are independent and touch different files.