Skip to content

Commit 5a482a1

Browse files
committed
Fix enum to bool comparison
The compiler compiles $value == true to ZEND_BOOL, which always returns true for objects (with the default cast_object handler). However, when compared to a statically unknown rhs $value == $true, the resulting opcode ZEND_IS_EQUAL would call the objects compare handler. The zend_objects_not_comparable() handler, which is installed for enums and other internal classes, blanketly returns false. This does not match the ZEND_BOOL semantics. Object to boolean comparison is now handled directly in zend_compare(), analogous to object to null comparison. It continuous to call the cast_object handler, but guarantees consistent behavior across ZEND_BOOL and ZEND_IS_EQUAL. Fixes GH-16954 Closes GH-17031
1 parent fbb97aa commit 5a482a1

File tree

5 files changed

+84
-11
lines changed

5 files changed

+84
-11
lines changed

UPGRADING

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ PHP 8.5 UPGRADE NOTES
2828
- Core:
2929
. It is no longer possible to use "array" and "callable" as class alias names
3030
in class_alias().
31+
. Loosely comparing uncomparable objects (e.g. enums, \CurlHandle and other
32+
internal classes) to booleans was previously inconsistent. If compared to a
33+
boolean literal $object == true, it would behave the same way as (bool)
34+
$object. If compared to a statically unknown value $object == $true, it
35+
would always return false. This behavior was consolidated to always follow
36+
the behavior of (bool) $object.
3137

3238
- Intl:
3339
. The extension now requires at least ICU 57.1.

Zend/tests/enum/comparison.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,5 @@ bool(false)
5353
bool(false)
5454
bool(false)
5555
bool(false)
56-
bool(false)
57-
bool(false)
56+
bool(true)
57+
bool(true)

Zend/tests/gh16954.phpt

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
--TEST--
2+
GH-16954: Enum to bool comparison is inconsistent
3+
--FILE--
4+
<?php
5+
6+
enum E {
7+
case C;
8+
}
9+
10+
$true = true;
11+
$false = false;
12+
13+
var_dump(E::C == true);
14+
var_dump(E::C == $true);
15+
var_dump(true == E::C);
16+
var_dump($true == E::C);
17+
18+
var_dump(E::C != true);
19+
var_dump(E::C != $true);
20+
var_dump(true != E::C);
21+
var_dump($true != E::C);
22+
23+
var_dump(E::C == false);
24+
var_dump(E::C == $false);
25+
var_dump(false == E::C);
26+
var_dump($false == E::C);
27+
28+
var_dump(E::C != false);
29+
var_dump(E::C != $false);
30+
var_dump(false != E::C);
31+
var_dump($false != E::C);
32+
33+
?>
34+
--EXPECT--
35+
bool(true)
36+
bool(true)
37+
bool(true)
38+
bool(true)
39+
bool(false)
40+
bool(false)
41+
bool(false)
42+
bool(false)
43+
bool(false)
44+
bool(false)
45+
bool(false)
46+
bool(false)
47+
bool(true)
48+
bool(true)
49+
bool(true)
50+
bool(true)

Zend/zend_object_handlers.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2063,8 +2063,9 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */
20632063
object_lhs = false;
20642064
}
20652065
ZEND_ASSERT(Z_TYPE_P(value) != IS_OBJECT);
2066-
uint8_t target_type = (Z_TYPE_P(value) == IS_FALSE || Z_TYPE_P(value) == IS_TRUE)
2067-
? _IS_BOOL : Z_TYPE_P(value);
2066+
uint8_t target_type = Z_TYPE_P(value);
2067+
/* Should be handled in zend_compare(). */
2068+
ZEND_ASSERT(target_type != IS_FALSE && target_type != IS_TRUE);
20682069
if (Z_OBJ_HT_P(object)->cast_object(Z_OBJ_P(object), &casted, target_type) == FAILURE) {
20692070
// TODO: Less crazy.
20702071
if (target_type == IS_LONG || target_type == IS_DOUBLE) {

Zend/zend_operators.c

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2333,13 +2333,29 @@ ZEND_API int ZEND_FASTCALL zend_compare(zval *op1, zval *op2) /* {{{ */
23332333
}
23342334

23352335
if (Z_TYPE_P(op1) == IS_OBJECT
2336-
&& Z_TYPE_P(op2) == IS_OBJECT
2337-
&& Z_OBJ_P(op1) == Z_OBJ_P(op2)) {
2338-
return 0;
2339-
} else if (Z_TYPE_P(op1) == IS_OBJECT) {
2340-
return Z_OBJ_HANDLER_P(op1, compare)(op1, op2);
2341-
} else if (Z_TYPE_P(op2) == IS_OBJECT) {
2342-
return Z_OBJ_HANDLER_P(op2, compare)(op1, op2);
2336+
|| Z_TYPE_P(op2) == IS_OBJECT) {
2337+
zval *object, *other;
2338+
if (Z_TYPE_P(op1) == IS_OBJECT) {
2339+
object = op1;
2340+
other = op2;
2341+
} else {
2342+
object = op2;
2343+
other = op1;
2344+
}
2345+
if (EXPECTED(Z_TYPE_P(other) == IS_OBJECT)) {
2346+
if (Z_OBJ_P(object) == Z_OBJ_P(other)) {
2347+
return 0;
2348+
}
2349+
} else if (Z_TYPE_P(other) == IS_TRUE || Z_TYPE_P(other) == IS_FALSE) {
2350+
zval casted;
2351+
if (Z_OBJ_HANDLER_P(object, cast_object)(Z_OBJ_P(object), &casted, _IS_BOOL) == FAILURE) {
2352+
return object == op1 ? 1 : -1;
2353+
}
2354+
int ret = object == op1 ? zend_compare(&casted, other) : zend_compare(other, &casted);
2355+
ZEND_ASSERT(!Z_REFCOUNTED_P(&casted));
2356+
return ret;
2357+
}
2358+
return Z_OBJ_HANDLER_P(object, compare)(op1, op2);
23432359
}
23442360

23452361
if (!converted) {

0 commit comments

Comments
 (0)