Deduplicate inheritance type check implementation

Make the check covariant (insofar as it is allowed now, i.e. nullability and iterable) and call it with appropriate argument order for both parameter and return types. This makes it simpler to extend to full variance support.
php · May 8, 2019 · 5c47401 · 5c47401
1 parent d5ef9c5
commit 5c47401
Showing 1 changed file with 16 additions and 33 deletions.
diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c
@@ -183,10 +183,16 @@ static zend_always_inline zend_bool zend_iterable_compatibility_check(zend_arg_i
 }
 /* }}} */
 
-static int zend_do_perform_type_hint_check(const zend_function *fe, zend_arg_info *fe_arg_info, const zend_function *proto, zend_arg_info *proto_arg_info) /* {{{ */
+static int zend_perform_covariant_type_check(
+		const zend_function *fe, zend_arg_info *fe_arg_info,
+		const zend_function *proto, zend_arg_info *proto_arg_info) /* {{{ */
 {
 	ZEND_ASSERT(ZEND_TYPE_IS_SET(fe_arg_info->type) && ZEND_TYPE_IS_SET(proto_arg_info->type));
 
+	if (ZEND_TYPE_ALLOW_NULL(fe_arg_info->type) && !ZEND_TYPE_ALLOW_NULL(proto_arg_info->type)) {
+		return 0;
+	}
+
 	if (ZEND_TYPE_IS_CLASS(fe_arg_info->type) && ZEND_TYPE_IS_CLASS(proto_arg_info->type)) {
 		zend_string *fe_class_name, *proto_class_name;
 		const char *class_name;
@@ -239,6 +245,10 @@ static int zend_do_perform_type_hint_check(const zend_function *fe, zend_arg_inf
 		zend_string_release(proto_class_name);
 		zend_string_release(fe_class_name);
 	} else if (ZEND_TYPE_CODE(fe_arg_info->type) != ZEND_TYPE_CODE(proto_arg_info->type)) {
+		if (ZEND_TYPE_CODE(proto_arg_info->type) == IS_ITERABLE) {
+			return zend_iterable_compatibility_check(fe_arg_info);
+		}
+
 		/* Incompatible built-in types */
 		return 0;
 	}
@@ -259,7 +269,9 @@ static int zend_do_perform_arg_type_hint_check(const zend_function *fe, zend_arg
 		return 0;
 	}
 
-	return zend_do_perform_type_hint_check(fe, fe_arg_info, proto, proto_arg_info);
+	/* Contravariant type check is performed as a covariant type check with swapped
+	 * argument order. */
+	return zend_perform_covariant_type_check(proto, proto_arg_info, fe, fe_arg_info);
 }
 /* }}} */
 
@@ -332,21 +344,6 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c
 		}
 
 		if (!zend_do_perform_arg_type_hint_check(fe, fe_arg_info, proto, proto_arg_info)) {
-			switch (ZEND_TYPE_CODE(fe_arg_info->type)) {
-				case IS_ITERABLE:
-					if (!zend_iterable_compatibility_check(proto_arg_info)) {
-						return 0;
-					}
-					break;
-
-				default:
-					return 0;
-			}
-		}
-
-		// This introduces BC break described at https://bugs.php.net/bug.php?id=72119
-		if (ZEND_TYPE_IS_SET(proto_arg_info->type) && ZEND_TYPE_ALLOW_NULL(proto_arg_info->type) && !ZEND_TYPE_ALLOW_NULL(fe_arg_info->type)) {
-			/* incompatible nullability */
 			return 0;
 		}
 
@@ -364,20 +361,7 @@ static zend_bool zend_do_perform_implementation_check(const zend_function *fe, c
 			return 0;
 		}
 
-		if (!zend_do_perform_type_hint_check(fe, fe->common.arg_info - 1, proto, proto->common.arg_info - 1)) {
-			switch (ZEND_TYPE_CODE(proto->common.arg_info[-1].type)) {
-				case IS_ITERABLE:
-					if (!zend_iterable_compatibility_check(fe->common.arg_info - 1)) {
-						return 0;
-					}
-					break;
-
-				default:
-					return 0;
-			}
-		}
-
-		if (ZEND_TYPE_ALLOW_NULL(fe->common.arg_info[-1].type) && !ZEND_TYPE_ALLOW_NULL(proto->common.arg_info[-1].type)) {
+		if (!zend_perform_covariant_type_check(fe, fe->common.arg_info - 1, proto, proto->common.arg_info - 1)) {
 			return 0;
 		}
 	}
@@ -645,8 +629,7 @@ static void do_inheritance_check_on_method(zend_function *child, zend_function *
 					error_verb = "must";
 				} else if ((parent->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) &&
 		                   (!(child->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
-				            !zend_do_perform_type_hint_check(child, child->common.arg_info - 1, parent, parent->common.arg_info - 1) ||
-				            (ZEND_TYPE_ALLOW_NULL(child->common.arg_info[-1].type) && !ZEND_TYPE_ALLOW_NULL(parent->common.arg_info[-1].type)))) {
+				            !zend_perform_covariant_type_check(child, child->common.arg_info - 1, parent, parent->common.arg_info - 1))) {
 					error_level = E_COMPILE_ERROR;
 					error_verb = "must";
 				} else {