Skip to content

Commit b2350cc

Browse files
nikicnikic
committed
Don't throw a warning for 1/-1 flips on equality comparisons
While the comparison result changes, the equality comparison is not going to care about it in the end.
1 parent 148d797 commit b2350cc

File tree

4 files changed

+74
-30
lines changed

4 files changed

+74
-30
lines changed

‎Zend/zend_operators.c‎

Lines changed: 60 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2019,7 +2019,7 @@ static void ZEND_FASTCALL convert_compare_result_to_long(zval *result) /* {{{ */
20192019
}
20202020
/* }}} */
20212021

2022-
static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */
2022+
static int compare_long_to_string(zend_long lval, zend_string *str, zend_bool equality_only) /* {{{ */
20232023
{
20242024
zend_long str_lval;
20252025
double str_dval;
@@ -2048,7 +2048,10 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */
20482048
str_cmp_result = ZEND_NORMALIZE_BOOL(str_cmp_result);
20492049
zend_string_release(lval_as_str);
20502050

2051-
if (str_cmp_result != num_cmp_result) {
2051+
/* Don't report a warning if we're using == and the comparison changed between 1/-1. */
2052+
zend_bool cmp_result_changed_observably = str_cmp_result != num_cmp_result &&
2053+
(!equality_only || str_cmp_result == 0 || num_cmp_result == 0);
2054+
if (cmp_result_changed_observably) {
20522055
zend_error(E_WARNING,
20532056
"Result of comparison between " ZEND_LONG_FMT " and \"%s\" will change (%d to %d)",
20542057
lval, ZSTR_VAL(str), num_cmp_result, str_cmp_result);
@@ -2059,7 +2062,7 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */
20592062
}
20602063
/* }}} */
20612064

2062-
static int compare_double_to_string(double dval, zend_string *str) /* {{{ */
2065+
static int compare_double_to_string(double dval, zend_string *str, zend_bool equality_only) /* {{{ */
20632066
{
20642067
zend_long str_lval;
20652068
double str_dval;
@@ -2091,7 +2094,10 @@ static int compare_double_to_string(double dval, zend_string *str) /* {{{ */
20912094
str_cmp_result = ZEND_NORMALIZE_BOOL(str_cmp_result);
20922095
zend_string_release(dval_as_str);
20932096

2094-
if (str_cmp_result != num_cmp_result) {
2097+
/* Don't report a warning if we're using == and the comparison changed between 1/-1. */
2098+
zend_bool cmp_result_changed_observably = str_cmp_result != num_cmp_result &&
2099+
(!equality_only || str_cmp_result == 0 || num_cmp_result == 0);
2100+
if (cmp_result_changed_observably) {
20952101
zend_error(E_WARNING,
20962102
"Result of comparison between %G and \"%s\" will change (%d to %d)",
20972103
dval, ZSTR_VAL(str), num_cmp_result, str_cmp_result);
@@ -2102,7 +2108,7 @@ static int compare_double_to_string(double dval, zend_string *str) /* {{{ */
21022108
}
21032109
/* }}} */
21042110

2105-
ZEND_API int ZEND_FASTCALL compare_function(zval *result, zval *op1, zval *op2) /* {{{ */
2111+
ZEND_API int ZEND_FASTCALL compare_function_ex(zval *result, zval *op1, zval *op2, zend_bool equality_only) /* {{{ */
21062112
{
21072113
int ret;
21082114
int converted = 0;
@@ -2135,7 +2141,7 @@ ZEND_API int ZEND_FASTCALL compare_function(zval *result, zval *op1, zval *op2)
21352141
return SUCCESS;
21362142

21372143
case TYPE_PAIR(IS_ARRAY, IS_ARRAY):
2138-
ZVAL_LONG(result, zend_compare_arrays(op1, op2));
2144+
ZVAL_LONG(result, zend_compare_arrays_ex(op1, op2, equality_only));
21392145
return SUCCESS;
21402146

21412147
case TYPE_PAIR(IS_NULL, IS_NULL):
@@ -2171,11 +2177,11 @@ ZEND_API int ZEND_FASTCALL compare_function(zval *result, zval *op1, zval *op2)
21712177
return SUCCESS;
21722178

21732179
case TYPE_PAIR(IS_LONG, IS_STRING):
2174-
ZVAL_LONG(result, compare_long_to_string(Z_LVAL_P(op1), Z_STR_P(op2)));
2180+
ZVAL_LONG(result, compare_long_to_string(Z_LVAL_P(op1), Z_STR_P(op2), equality_only));
21752181
return SUCCESS;
21762182

21772183
case TYPE_PAIR(IS_STRING, IS_LONG):
2178-
ZVAL_LONG(result, -compare_long_to_string(Z_LVAL_P(op2), Z_STR_P(op1)));
2184+
ZVAL_LONG(result, -compare_long_to_string(Z_LVAL_P(op2), Z_STR_P(op1), equality_only));
21792185
return SUCCESS;
21802186

21812187
case TYPE_PAIR(IS_DOUBLE, IS_STRING):
@@ -2184,7 +2190,7 @@ ZEND_API int ZEND_FASTCALL compare_function(zval *result, zval *op1, zval *op2)
21842190
return SUCCESS;
21852191
}
21862192

2187-
ZVAL_LONG(result, compare_double_to_string(Z_DVAL_P(op1), Z_STR_P(op2)));
2193+
ZVAL_LONG(result, compare_double_to_string(Z_DVAL_P(op1), Z_STR_P(op2), equality_only));
21882194
return SUCCESS;
21892195

21902196
case TYPE_PAIR(IS_STRING, IS_DOUBLE):
@@ -2193,8 +2199,8 @@ ZEND_API int ZEND_FASTCALL compare_function(zval *result, zval *op1, zval *op2)
21932199
return SUCCESS;
21942200
}
21952201

2196-
ZVAL_LONG(result, -compare_double_to_string(Z_DVAL_P(op2), Z_STR_P(op1)));
2197-
return SUCCESS;
2202+
ZVAL_LONG(result, -compare_double_to_string(Z_DVAL_P(op2), Z_STR_P(op1), equality_only));
2203+
return SUCCESS;
21982204

21992205
case TYPE_PAIR(IS_OBJECT, IS_NULL):
22002206
ZVAL_LONG(result, 1);
@@ -2242,7 +2248,7 @@ return SUCCESS;
22422248
if (Z_OBJ_HT_P(op1)->get) {
22432249
zval rv;
22442250
op_free = Z_OBJ_HT_P(op1)->get(op1, &rv);
2245-
ret = compare_function(result, op_free, op2);
2251+
ret = compare_function_ex(result, op_free, op2, equality_only);
22462252
zend_free_obj_get_result(op_free);
22472253
return ret;
22482254
} else if (Z_TYPE_P(op2) != IS_OBJECT && Z_OBJ_HT_P(op1)->cast_object) {
@@ -2252,7 +2258,7 @@ return SUCCESS;
22522258
zend_free_obj_get_result(&tmp_free);
22532259
return SUCCESS;
22542260
}
2255-
ret = compare_function(result, &tmp_free, op2);
2261+
ret = compare_function_ex(result, &tmp_free, op2, equality_only);
22562262
zend_free_obj_get_result(&tmp_free);
22572263
return ret;
22582264
}
@@ -2261,7 +2267,7 @@ return SUCCESS;
22612267
if (Z_OBJ_HT_P(op2)->get) {
22622268
zval rv;
22632269
op_free = Z_OBJ_HT_P(op2)->get(op2, &rv);
2264-
ret = compare_function(result, op1, op_free);
2270+
ret = compare_function_ex(result, op1, op_free, equality_only);
22652271
zend_free_obj_get_result(op_free);
22662272
return ret;
22672273
} else if (Z_TYPE_P(op1) != IS_OBJECT && Z_OBJ_HT_P(op2)->cast_object) {
@@ -2271,7 +2277,7 @@ return SUCCESS;
22712277
zend_free_obj_get_result(&tmp_free);
22722278
return SUCCESS;
22732279
}
2274-
ret = compare_function(result, op1, &tmp_free);
2280+
ret = compare_function_ex(result, op1, &tmp_free, equality_only);
22752281
zend_free_obj_get_result(&tmp_free);
22762282
return ret;
22772283
} else if (Z_TYPE_P(op1) == IS_OBJECT) {
@@ -2322,6 +2328,12 @@ return SUCCESS;
23222328
}
23232329
/* }}} */
23242330

2331+
ZEND_API int ZEND_FASTCALL compare_function(zval *result, zval *op1, zval *op2) /* {{{ */
2332+
{
2333+
return compare_function_ex(result, op1, op2, 0);
2334+
}
2335+
/* }}} */
2336+
23252337
static int hash_zval_identical_function(zval *z1, zval *z2) /* {{{ */
23262338
{
23272339
/* is_identical_function() returns 1 in case of identity and 0 in case
@@ -2380,7 +2392,7 @@ ZEND_API int ZEND_FASTCALL is_not_identical_function(zval *result, zval *op1, zv
23802392

23812393
ZEND_API int ZEND_FASTCALL is_equal_function(zval *result, zval *op1, zval *op2) /* {{{ */
23822394
{
2383-
if (compare_function(result, op1, op2) == FAILURE) {
2395+
if (compare_function_ex(result, op1, op2, 1) == FAILURE) {
23842396
return FAILURE;
23852397
}
23862398
ZVAL_BOOL(result, (Z_LVAL_P(result) == 0));
@@ -2390,7 +2402,7 @@ ZEND_API int ZEND_FASTCALL is_equal_function(zval *result, zval *op1, zval *op2)
23902402

23912403
ZEND_API int ZEND_FASTCALL is_not_equal_function(zval *result, zval *op1, zval *op2) /* {{{ */
23922404
{
2393-
if (compare_function(result, op1, op2) == FAILURE) {
2405+
if (compare_function_ex(result, op1, op2, 1) == FAILURE) {
23942406
return FAILURE;
23952407
}
23962408
ZVAL_BOOL(result, (Z_LVAL_P(result) != 0));
@@ -3076,15 +3088,44 @@ static int hash_zval_compare_function(zval *z1, zval *z2) /* {{{ */
30763088
}
30773089
/* }}} */
30783090

3091+
static int hash_zval_compare_function_equality(zval *z1, zval *z2) /* {{{ */
3092+
{
3093+
zval result;
3094+
3095+
if (compare_function_ex(&result, z1, z2, 1)==FAILURE) {
3096+
return 1;
3097+
}
3098+
return Z_LVAL(result);
3099+
}
3100+
/* }}} */
3101+
3102+
ZEND_API int ZEND_FASTCALL zend_compare_symbol_tables_ex(HashTable *ht1, HashTable *ht2, zend_bool equality_only) /* {{{ */
3103+
{
3104+
if (ht1 == ht2) {
3105+
return 0;
3106+
}
3107+
return zend_hash_compare(ht1, ht2,
3108+
equality_only
3109+
? (compare_func_t) hash_zval_compare_function_equality
3110+
: (compare_func_t) hash_zval_compare_function, 0);
3111+
}
3112+
/* }}} */
3113+
30793114
ZEND_API int ZEND_FASTCALL zend_compare_symbol_tables(HashTable *ht1, HashTable *ht2) /* {{{ */
30803115
{
3081-
return ht1 == ht2 ? 0 : zend_hash_compare(ht1, ht2, (compare_func_t) hash_zval_compare_function, 0);
3116+
return zend_compare_symbol_tables_ex(ht1, ht2, 0);
3117+
}
3118+
/* }}} */
3119+
3120+
ZEND_API int ZEND_FASTCALL zend_compare_arrays_ex(zval *a1, zval *a2, zend_bool equality_only) /* {{{ */
3121+
{
3122+
return zend_compare_symbol_tables_ex(Z_ARRVAL_P(a1), Z_ARRVAL_P(a2), equality_only);
30823123
}
30833124
/* }}} */
30843125

30853126
ZEND_API int ZEND_FASTCALL zend_compare_arrays(zval *a1, zval *a2) /* {{{ */
30863127
{
3087-
return zend_compare_symbol_tables(Z_ARRVAL_P(a1), Z_ARRVAL_P(a2));
3128+
return zend_compare_symbol_tables_ex(Z_ARRVAL_P(a1), Z_ARRVAL_P(a2), 0);
30883129
}
30893130
/* }}} */
30903131

‎Zend/zend_operators.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,7 @@ static zend_always_inline int i_zend_is_true(zval *op)
388388
return result;
389389
}
390390

391+
ZEND_API int ZEND_FASTCALL compare_function_ex(zval *result, zval *op1, zval *op2, zend_bool equality_only);
391392
ZEND_API int ZEND_FASTCALL compare_function(zval *result, zval *op1, zval *op2);
392393

393394
ZEND_API int ZEND_FASTCALL numeric_compare_function(zval *op1, zval *op2);
@@ -417,7 +418,9 @@ ZEND_API int ZEND_FASTCALL zend_binary_strncasecmp_l(const char *s1, size_t len1
417418

418419
ZEND_API int ZEND_FASTCALL zendi_smart_streq(zend_string *s1, zend_string *s2);
419420
ZEND_API int ZEND_FASTCALL zendi_smart_strcmp(zend_string *s1, zend_string *s2);
421+
ZEND_API int ZEND_FASTCALL zend_compare_symbol_tables_ex(HashTable *ht1, HashTable *ht2, zend_bool equality_only);
420422
ZEND_API int ZEND_FASTCALL zend_compare_symbol_tables(HashTable *ht1, HashTable *ht2);
423+
ZEND_API int ZEND_FASTCALL zend_compare_arrays_ex(zval *a1, zval *a2, zend_bool equality_only);
421424
ZEND_API int ZEND_FASTCALL zend_compare_arrays(zval *a1, zval *a2);
422425
ZEND_API int ZEND_FASTCALL zend_compare_objects(zval *o1, zval *o2);
423426

@@ -860,7 +863,7 @@ static zend_always_inline int fast_equal_check_function(zval *op1, zval *op2)
860863
return zend_fast_equal_strings(Z_STR_P(op1), Z_STR_P(op2));
861864
}
862865
}
863-
compare_function(&result, op1, op2);
866+
compare_function_ex(&result, op1, op2, 1);
864867
return Z_LVAL(result) == 0;
865868
}
866869

@@ -870,7 +873,7 @@ static zend_always_inline int fast_equal_check_long(zval *op1, zval *op2)
870873
if (EXPECTED(Z_TYPE_P(op2) == IS_LONG)) {
871874
return Z_LVAL_P(op1) == Z_LVAL_P(op2);
872875
}
873-
compare_function(&result, op1, op2);
876+
compare_function_ex(&result, op1, op2, 1);
874877
return Z_LVAL(result) == 0;
875878
}
876879

@@ -880,7 +883,7 @@ static zend_always_inline int fast_equal_check_string(zval *op1, zval *op2)
880883
if (EXPECTED(Z_TYPE_P(op2) == IS_STRING)) {
881884
return zend_fast_equal_strings(Z_STR_P(op1), Z_STR_P(op2));
882885
}
883-
compare_function(&result, op1, op2);
886+
compare_function_ex(&result, op1, op2, 1);
884887
return Z_LVAL(result) == 0;
885888
}
886889

‎Zend/zend_vm_def.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ ZEND_VM_HELPER(zend_is_equal_helper, ANY, ANY, zval *op_1, zval *op_2)
498498
if (UNEXPECTED(Z_TYPE_INFO_P(op_2) == IS_UNDEF)) {
499499
op_2 = ZVAL_UNDEFINED_OP2();
500500
}
501-
compare_function(EX_VAR(opline->result.var), op_1, op_2);
501+
compare_function_ex(EX_VAR(opline->result.var), op_1, op_2, 1);
502502
if (OP1_TYPE & (IS_TMP_VAR|IS_VAR)) {
503503
zval_ptr_dtor_nogc(op_1);
504504
}
@@ -593,7 +593,7 @@ ZEND_VM_HELPER(zend_is_not_equal_helper, ANY, ANY, zval *op_1, zval *op_2)
593593
if (UNEXPECTED(Z_TYPE_INFO_P(op_2) == IS_UNDEF)) {
594594
op_2 = ZVAL_UNDEFINED_OP2();
595595
}
596-
compare_function(EX_VAR(opline->result.var), op_1, op_2);
596+
compare_function_ex(EX_VAR(opline->result.var), op_1, op_2, 1);
597597
if (OP1_TYPE & (IS_TMP_VAR|IS_VAR)) {
598598
zval_ptr_dtor_nogc(op_1);
599599
}
@@ -688,7 +688,7 @@ ZEND_VM_HELPER(zend_is_smaller_helper, ANY, ANY, zval *op_1, zval *op_2)
688688
if (UNEXPECTED(Z_TYPE_INFO_P(op_2) == IS_UNDEF)) {
689689
op_2 = ZVAL_UNDEFINED_OP2();
690690
}
691-
compare_function(EX_VAR(opline->result.var), op_1, op_2);
691+
compare_function_ex(EX_VAR(opline->result.var), op_1, op_2, 1);
692692
if (OP1_TYPE & (IS_TMP_VAR|IS_VAR)) {
693693
zval_ptr_dtor_nogc(op_1);
694694
}
@@ -768,7 +768,7 @@ ZEND_VM_HELPER(zend_is_smaller_or_equal_helper, ANY, ANY, zval *op_1, zval *op_2
768768
if (UNEXPECTED(Z_TYPE_INFO_P(op_2) == IS_UNDEF)) {
769769
op_2 = ZVAL_UNDEFINED_OP2();
770770
}
771-
compare_function(EX_VAR(opline->result.var), op_1, op_2);
771+
compare_function_ex(EX_VAR(opline->result.var), op_1, op_2, 1);
772772
if (OP1_TYPE & (IS_TMP_VAR|IS_VAR)) {
773773
zval_ptr_dtor_nogc(op_1);
774774
}

‎Zend/zend_vm_execute.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_is_equal_hel
587587
if (UNEXPECTED(Z_TYPE_INFO_P(op_2) == IS_UNDEF)) {
588588
op_2 = ZVAL_UNDEFINED_OP2();
589589
}
590-
compare_function(EX_VAR(opline->result.var), op_1, op_2);
590+
compare_function_ex(EX_VAR(opline->result.var), op_1, op_2, 1);
591591
if (opline->op1_type & (IS_TMP_VAR|IS_VAR)) {
592592
zval_ptr_dtor_nogc(op_1);
593593
}
@@ -619,7 +619,7 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_is_not_equal
619619
if (UNEXPECTED(Z_TYPE_INFO_P(op_2) == IS_UNDEF)) {
620620
op_2 = ZVAL_UNDEFINED_OP2();
621621
}
622-
compare_function(EX_VAR(opline->result.var), op_1, op_2);
622+
compare_function_ex(EX_VAR(opline->result.var), op_1, op_2, 1);
623623
if (opline->op1_type & (IS_TMP_VAR|IS_VAR)) {
624624
zval_ptr_dtor_nogc(op_1);
625625
}
@@ -651,7 +651,7 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_is_smaller_h
651651
if (UNEXPECTED(Z_TYPE_INFO_P(op_2) == IS_UNDEF)) {
652652
op_2 = ZVAL_UNDEFINED_OP2();
653653
}
654-
compare_function(EX_VAR(opline->result.var), op_1, op_2);
654+
compare_function_ex(EX_VAR(opline->result.var), op_1, op_2, 1);
655655
if (opline->op1_type & (IS_TMP_VAR|IS_VAR)) {
656656
zval_ptr_dtor_nogc(op_1);
657657
}
@@ -683,7 +683,7 @@ static zend_never_inline ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_is_smaller_o
683683
if (UNEXPECTED(Z_TYPE_INFO_P(op_2) == IS_UNDEF)) {
684684
op_2 = ZVAL_UNDEFINED_OP2();
685685
}
686-
compare_function(EX_VAR(opline->result.var), op_1, op_2);
686+
compare_function_ex(EX_VAR(opline->result.var), op_1, op_2, 1);
687687
if (opline->op1_type & (IS_TMP_VAR|IS_VAR)) {
688688
zval_ptr_dtor_nogc(op_1);
689689
}

0 commit comments

Comments
 (0)