Skip to content

Commit 06be8c5

Browse files
zonuexezonuexe
committed
Do not report assigning a float zero as redundant
0.0 and -0.0 are identical, so ConstantFloatType::equals() does not tell them apart, but the sign stays observable: (string) -0.0 is "-0" and fdiv(1, -0.0) is -INF. `$f === 0.0` also narrows $f to 0.0 while it may still hold -0.0, so a sign-aware comparison would not help either. AssignHandler::redundant() now gives up when the value holds a float zero, directly or inside a constant array; the native twin does the same.
1 parent f30b81c commit 06be8c5

5 files changed

Lines changed: 127 additions & 2 deletions

File tree

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3027,11 +3027,31 @@ private static function redundant(ExpressionResult $rhs, Expr $target, Expressio
30273027
$nativeType = $nativeType->getOffsetValueType($nativeOffset);
30283028
}
30293029
$values = $type->getFiniteTypes();
3030-
if (count($values) !== 1 || !$values[0]->equals($rhs->getType())) {
3030+
if (count($values) !== 1 || !$values[0]->equals($rhs->getType()) || self::containsFloatZero($values[0])) {
30313031
return null;
30323032
}
30333033
$nativeValues = $nativeType->getFiniteTypes();
30343034
return count($nativeValues) === 1 && $nativeValues[0]->equals($rhs->getNativeType()) ? $rhs->getType() : null;
30353035
}
30363036

3037+
/**
3038+
* 0.0 and -0.0 are identical (`===`) but distinguishable (`(string)`, `fdiv()`),
3039+
* and `$f === 0.0` narrows $f to 0.0 while it may still hold -0.0, so a type
3040+
* holding a float zero cannot prove that assigning it again changes nothing.
3041+
*/
3042+
private static function containsFloatZero(Type $type): bool
3043+
{
3044+
if (in_array(0.0, $type->getConstantScalarValues(), true)) {
3045+
return true;
3046+
}
3047+
foreach ($type->getConstantArrays() as $array) {
3048+
foreach ($array->getValueTypes() as $valueType) {
3049+
if (self::containsFloatZero($valueType)) {
3050+
return true;
3051+
}
3052+
}
3053+
}
3054+
return false;
3055+
}
3056+
30373057
}

‎tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,14 @@ public function testRedundantAssignment(): void
201201
]);
202202
}
203203

204+
public function testRedundantAssignmentOfSignedZero(): void
205+
{
206+
$this->analyse([__DIR__ . '/data/unused-variable-redundant-signed-zero.php'], [
207+
['Value assigned to variable $x is never read before being overwritten.', 12],
208+
['Variable $x is assigned value 1.5 but it already has that value.', 58],
209+
]);
210+
}
211+
204212
public function testByRefReturn(): void
205213
{
206214
$this->analyse([__DIR__ . '/data/unused-variable-by-ref-return.php'], [

‎tests/PHPStan/Rules/DeadCode/data/unused-variable-redundant-signed-zero.php‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace UnusedVariableRedundantSignedZero;
4+
5+
function f(float $f): float {
6+
if ($f === 0.0) {
7+
$f = 0.0;
8+
}
9+
return $f;
10+
}
11+
function g(): float {
12+
$x = -0.0;
13+
$x = 0.0;
14+
return $x;
15+
}
16+
17+
/** @param mixed $v */
18+
function sink($v): void
19+
{
20+
}
21+
22+
function negativeZeroNarrowing(float $f): void
23+
{
24+
if ($f === -0.0) {
25+
$f = -0.0;
26+
}
27+
sink($f);
28+
}
29+
30+
function looseZeroNarrowing(float $f): void
31+
{
32+
if ($f == 0) {
33+
$f = 0.0;
34+
}
35+
sink($f);
36+
}
37+
38+
function zeroInArray(): void
39+
{
40+
$a = [-0.0];
41+
sink($a);
42+
$a = [0.0];
43+
sink($a);
44+
}
45+
46+
function zeroAtOffset(): void
47+
{
48+
$a = ['k' => -0.0];
49+
sink($a);
50+
$a['k'] = 0.0;
51+
sink($a);
52+
}
53+
54+
function nonZeroFloatIsStillRedundant(): void
55+
{
56+
$x = 1.5;
57+
sink($x);
58+
$x = 1.5;
59+
sink($x);
60+
}

‎turbo-ext/src/AssignHandler.cpp‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4679,6 +4679,9 @@ class AssignHandler
46794679
bool equal;
46804680
AH_OK(ahEquals(zend_hash_index_find(Z_ARRVAL_P(values.raw()), 0), rhsType.raw(), equal));
46814681
if (!equal) return ahNull();
4682+
bool floatZero;
4683+
AH_OK(containsFloatZero(zend_hash_index_find(Z_ARRVAL_P(values.raw()), 0), floatZero));
4684+
if (floatZero) return ahNull();
46824685
}
46834686
AH_VAL(nativeValues, ahCall(nativeType.raw(), PT_LC("getfinitetypes"), 0, NULL));
46844687
if (zend_hash_num_elements(Z_ARRVAL_P(nativeValues.raw())) != 1) return ahNull();
@@ -4688,6 +4691,35 @@ class AssignHandler
46884691
if (!equal) return ahNull();
46894692
return pt_expression_result_get_type(rhs);
46904693
}
4694+
4695+
/* (twin 3042); false = pending exception */
4696+
static bool containsFloatZero(zval *type, bool &result)
4697+
{
4698+
result = false;
4699+
{
4700+
AH_VALB(scalarValues, ahTypeOp(type, PT_OP_GET_CONSTANT_SCALAR_VALUES, 0, NULL));
4701+
for (auto entry : zv::TableRef(Z_ARRVAL_P(scalarValues.raw()))) {
4702+
zval *value = entry.value().deref().raw();
4703+
// in_array(0.0, ..., true): -0.0 is identical to 0.0, NAN to nothing
4704+
if (Z_TYPE_P(value) == IS_DOUBLE && Z_DVAL_P(value) == 0.0) {
4705+
result = true;
4706+
return true;
4707+
}
4708+
}
4709+
}
4710+
AH_VALB(constantArrays, ahTypeOp(type, PT_OP_GET_CONSTANT_ARRAYS, 0, NULL));
4711+
for (auto arrayEntry : zv::TableRef(Z_ARRVAL_P(constantArrays.raw()))) {
4712+
AH_VALB(valueTypes, ahTypeOp(arrayEntry.value().deref().raw(), PT_OP_GET_VALUE_TYPES, 0, NULL));
4713+
for (auto entry : zv::TableRef(Z_ARRVAL_P(valueTypes.raw()))) {
4714+
zval *valueType = entry.value().deref().raw();
4715+
bool ok = true;
4716+
pt_engine_with_stack([&]() { ok = containsFloatZero(valueType, result); });
4717+
AH_OKB(ok);
4718+
if (result) return true;
4719+
}
4720+
}
4721+
return true;
4722+
}
46914723
};
46924724

46934725
} // namespace phpstanturbo

‎turbo-ext/src/generated/AssignHandler.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,9 @@ inline constexpr char strings[] =
227227
"array\0" /* 3789 */
228228
"hasArrayReference\0" /* 3795 */
229229
"rhs\0" /* 3813 */
230-
"redundant"; /* 3817 */
230+
"redundant\0" /* 3817 */
231+
"type\0" /* 3827 */
232+
"containsFloatZero"; /* 3832 */
231233
inline constexpr reg::PackedArg args[] = {
232234
reg::packed(0, 0, 25), /* __construct $templateArgumentObserver */
233235
reg::packed(76, 0, 99), /* __construct $varAnnotationProcessor */
@@ -473,6 +475,8 @@ inline constexpr reg::PackedArg args[] = {
473475
reg::packed(1975, 0, 1376), /* redundant $target */
474476
reg::packed(1521, 0, 1529), /* redundant $storage */
475477
reg::packed(1396, MAY_BE_NULL, 2038), /* redundant return */
478+
reg::packed(3827, 0, 2038), /* containsFloatZero $type */
479+
reg::packed(1396, MAY_BE_BOOL), /* containsFloatZero return */
476480
};
477481
using Sig = reg::Sig<strings, args>;
478482
} // namespace sigtab
@@ -519,6 +523,7 @@ inline constexpr sigtab::Sig getLiteralArrayKeyType = { { 3654 /* getLiteralArra
519523
inline constexpr sigtab::Sig getOriginalPropertyType = { { 3765 /* getOriginalPropertyType */, 3, 234, 3, 237, ZEND_ACC_PRIVATE } };
520524
inline constexpr sigtab::Sig hasArrayReference = { { 3795 /* hasArrayReference */, 1, 238, 1, 239, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
521525
inline constexpr sigtab::Sig redundant = { { 3817 /* redundant */, 3, 240, 3, 243, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
526+
inline constexpr sigtab::Sig containsFloatZero = { { 3832 /* containsFloatZero */, 1, 244, 1, 245, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
522527
} // namespace sig
523528

524529
} // namespace ptdecl::AssignHandler

0 commit comments

Comments
 (0)