Comparison cleanup:

- introduce zend_compare() that returns -1,0,1 dirctly (without intermediate zval)
- remove compare_objects() object handler, and keep only compare() handler

Author: dstogov

UPGRADING.INTERNALS

@@ -10,6 +10,7 @@ PHP 8.0 INTERNALS UPGRADE NOTES
   g. zend_free_op type and should_free argument of zend_get_zval_ptr()
   h. zend_value_error()
   i. get_closure() object handler
+  j. compare_objects() and compare() object handlers
 
 2. Build system changes
   a. Abstract
@@ -76,6 +77,10 @@ PHP 8.0 INTERNALS UPGRADE NOTES
      `check_only`. If it is true, the handler is called to check whether the
      object is callable; in this case the handler should not throw an exception.
 
+  j. compare_objects() handler was removed. Extensions should use compare() object
+     handler instead and check if both arguments are objects and have the same
+     compare hanldler, using ZEND_COMPARE_OBJECTS_FALLBACK() macro.
+
 ========================
 2. Build system changes
 ========================

Zend/zend_closures.c

@@ -355,9 +355,10 @@ static ZEND_COLD zend_function *zend_closure_get_constructor(zend_object *object
 }
 /* }}} */
 
-static int zend_closure_compare_objects(zval *o1, zval *o2) /* {{{ */
+static int zend_closure_compare(zval *o1, zval *o2) /* {{{ */
 {
-	return (Z_OBJ_P(o1) != Z_OBJ_P(o2));
+	ZEND_COMPARE_OBJECTS_FALLBACK(o1, o2);
+	return Z_OBJ_P(o1) != Z_OBJ_P(o2);
 }
 /* }}} */
 
@@ -639,7 +640,7 @@ void zend_register_closure_ce(void) /* {{{ */
 	closure_handlers.get_property_ptr_ptr = zend_closure_get_property_ptr_ptr;
 	closure_handlers.has_property = zend_closure_has_property;
 	closure_handlers.unset_property = zend_closure_unset_property;
-	closure_handlers.compare_objects = zend_closure_compare_objects;
+	closure_handlers.compare = zend_closure_compare;
 	closure_handlers.clone_obj = zend_closure_clone;
 	closure_handlers.get_debug_info = zend_closure_get_debug_info;
 	closure_handlers.get_closure = zend_closure_get_closure;

Zend/zend_iterators.c

@@ -44,7 +44,6 @@ static const zend_object_handlers iterator_object_handlers = {
 	NULL, /* method get */
 	NULL, /* get ctor */
 	NULL, /* get class name */
-	NULL, /* compare */
 	NULL, /* cast */
 	NULL, /* count */
 	NULL, /* get_debug_info */

Zend/zend_object_handlers.c

@@ -1546,6 +1546,33 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */
 {
 	zend_object *zobj1, *zobj2;
 
+	if (Z_TYPE_P(o1) != Z_TYPE_P(o2)) {
+		/* Object and non-object */
+		zval casted;
+		if (Z_TYPE_P(o1) == IS_OBJECT) {
+			ZEND_ASSERT(Z_TYPE_P(o2) != IS_OBJECT);
+			if (Z_OBJ_HT_P(o1)->cast_object) {
+				if (Z_OBJ_HT_P(o1)->cast_object(Z_OBJ_P(o1), &casted, ((Z_TYPE_P(o2) == IS_FALSE || Z_TYPE_P(o2) == IS_TRUE) ? _IS_BOOL : Z_TYPE_P(o2))) == FAILURE) {
+					return 1;
+				}
+				int ret = zend_compare(&casted, o2);
+				zval_ptr_dtor(&casted);
+				return ret;
+			}
+		} else {
+			ZEND_ASSERT(Z_TYPE_P(o2) == IS_OBJECT);
+			if (Z_OBJ_HT_P(o2)->cast_object) {
+				if (Z_OBJ_HT_P(o2)->cast_object(Z_OBJ_P(o2), &casted, ((Z_TYPE_P(o1) == IS_FALSE || Z_TYPE_P(o1) == IS_TRUE) ? _IS_BOOL : Z_TYPE_P(o1))) == FAILURE) {
+					return -1;
+				}
+				int ret = zend_compare(o1, &casted);
+				zval_ptr_dtor(&casted);
+				return ret;
+			}
+		}
+		return 1;
+	}
+
 	zobj1 = Z_OBJ_P(o1);
 	zobj2 = Z_OBJ_P(o2);
 
@@ -1582,15 +1609,12 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */
 
 			if (Z_TYPE_P(p1) != IS_UNDEF) {
 				if (Z_TYPE_P(p2) != IS_UNDEF) {
-					zval result;
+					int ret;
 
-					if (compare_function(&result, p1, p2)==FAILURE) {
-						Z_UNPROTECT_RECURSION_P(o1);
-						return 1;
-					}
-					if (Z_LVAL(result) != 0) {
+					ret = zend_compare(p1, p2);
+					if (ret != 0) {
 						Z_UNPROTECT_RECURSION_P(o1);
-						return Z_LVAL(result);
+						return ret;
 					}
 				} else {
 					Z_UNPROTECT_RECURSION_P(o1);
@@ -1850,13 +1874,12 @@ ZEND_API const zend_object_handlers std_object_handlers = {
 	zend_std_get_method,					/* get_method */
 	zend_std_get_constructor,				/* get_constructor */
 	zend_std_get_class_name,				/* get_class_name */
-	zend_std_compare_objects,				/* compare_objects */
 	zend_std_cast_object_tostring,			/* cast_object */
 	NULL,									/* count_elements */
 	zend_std_get_debug_info,				/* get_debug_info */
 	zend_std_get_closure,					/* get_closure */
 	zend_std_get_gc,						/* get_gc */
 	NULL,									/* do_operation */
-	NULL,									/* compare */
+	zend_std_compare_objects,				/* compare */
 	NULL,									/* get_properties_for */
 };

Zend/zend_object_handlers.h

@@ -124,7 +124,6 @@ typedef zend_object* (*zend_object_clone_obj_t)(zend_object *object);
 typedef zend_string *(*zend_object_get_class_name_t)(const zend_object *object);
 
 typedef int (*zend_object_compare_t)(zval *object1, zval *object2);
-typedef int (*zend_object_compare_zvals_t)(zval *result, zval *op1, zval *op2);
 
 /* Cast an object to some other type.
  * readobj and retval must point to distinct zvals.
@@ -161,14 +160,13 @@ struct _zend_object_handlers {
 	zend_object_get_method_t				get_method;           /* required */
 	zend_object_get_constructor_t			get_constructor;      /* required */
 	zend_object_get_class_name_t			get_class_name;       /* required */
-	zend_object_compare_t					compare_objects;      /* optional */
 	zend_object_cast_t						cast_object;          /* optional */
 	zend_object_count_elements_t			count_elements;       /* optional */
 	zend_object_get_debug_info_t			get_debug_info;       /* optional */
 	zend_object_get_closure_t				get_closure;          /* optional */
 	zend_object_get_gc_t					get_gc;               /* required */
 	zend_object_do_operation_t				do_operation;         /* optional */
-	zend_object_compare_zvals_t				compare;              /* optional */
+	zend_object_compare_t					compare;              /* required */
 	zend_object_get_properties_for_t		get_properties_for;   /* optional */
 };
 
@@ -241,6 +239,16 @@ ZEND_API HashTable *zend_get_properties_for(zval *obj, zend_prop_purpose purpose
 		} \
 	} while (0)
 
+/* Fallback to default comparison implementation if the arguments aren't both objects
+ * and have the same compare() handler. You'll likely want to use this unless you
+ * explicitly wish to support comparisons between objects and non-objects. */
+#define ZEND_COMPARE_OBJECTS_FALLBACK(op1, op2) \
+	if (Z_TYPE_P(op1) != IS_OBJECT || \
+			Z_TYPE_P(op2) != IS_OBJECT || \
+			Z_OBJ_HT_P(op1)->compare != Z_OBJ_HT_P(op2)->compare) { \
+		return zend_std_compare_objects(op1, op2); \
+	}
+
 END_EXTERN_C()
 
 #endif