Address review and drop BC shim for unions

Author: Girgias

Zend/tests/return_types/generators001.phpt

@@ -26,13 +26,18 @@ function test6() : object|callable {
     yield 6;
 }
 
+function test7() : iterable {
+    yield 7;
+}
+
 var_dump(
     test1(),
     test2(),
     test3(),
     test4(),
     test5(),
     test6(),
+    test7(),
 );
 ?>
 --EXPECTF--
@@ -48,3 +53,5 @@ object(Generator)#%d (%d) {
 }
 object(Generator)#%d (%d) {
 }
+object(Generator)#%d (%d) {
+}

Zend/tests/type_declarations/intersection_types/variance/invalid5.phpt

@@ -15,4 +15,4 @@ class Test2 extends Test {
 
 ?>
 --EXPECTF--
-Fatal error: Declaration of Test2::method(): X&Y must be compatible with Test::method(): iterable in %s on line %d
+Fatal error: Declaration of Test2::method(): X&Y must be compatible with Test::method(): Traversable|array in %s on line %d

Zend/tests/type_declarations/iterable_001.phpt renamed to Zend/tests/type_declarations/iterable/iterable_001.phpt

@@ -45,4 +45,4 @@ object(ArrayIterator)#1 (1) {
     int(3)
   }
 }
-test(): Argument #1 ($iterable) must be of type iterable, int given, called in %s on line %d
+test(): Argument #1 ($iterable) must be of type Traversable|array, int given, called in %s on line %d

Zend/tests/type_declarations/iterable_002.phpt renamed to Zend/tests/type_declarations/iterable/iterable_002.phpt

@@ -17,4 +17,4 @@ function baz(iterable $iterable = 1) {
 
 ?>
 --EXPECTF--
-Fatal error: Cannot use int as default value for parameter $iterable of type iterable in %s on line %d
+Fatal error: Cannot use int as default value for parameter $iterable of type Traversable|array in %s on line %d

Zend/tests/type_declarations/iterable_003.phpt renamed to Zend/tests/type_declarations/iterable/iterable_003.phpt

@@ -29,4 +29,4 @@ array(0) {
 }
 object(Generator)#2 (0) {
 }
-baz(): Return value must be of type iterable, int returned
+baz(): Return value must be of type Traversable|array, int returned

Zend/tests/type_declarations/iterable_004.phpt renamed to Zend/tests/type_declarations/iterable/iterable_004.phpt

@@ -21,4 +21,4 @@ class Bar extends Foo {
 
 ?>
 --EXPECTF--
-Fatal error: Declaration of Bar::testScalar(iterable $iterable) must be compatible with Foo::testScalar(int $int) in %s on line %d
+Fatal error: Declaration of Bar::testScalar(Traversable|array $iterable) must be compatible with Foo::testScalar(int $int) in %s on line %d

Zend/tests/type_declarations/iterable_005.phpt renamed to Zend/tests/type_declarations/iterable/iterable_005.phpt

@@ -29,4 +29,4 @@ class TestScalar extends Test {
 
 ?>
 --EXPECTF--
-Fatal error: Declaration of TestScalar::method(): int must be compatible with Test::method(): iterable in %s on line %d
+Fatal error: Declaration of TestScalar::method(): int must be compatible with Test::method(): Traversable|array in %s on line %d

Zend/tests/type_declarations/iterable/or_null.phpt

@@ -0,0 +1,46 @@
+--TEST--
+Test "or null"/"or be null" in type-checking errors for userland functions with iterable
+--FILE--
+<?php
+
+// This should test every branch in zend_execute.c's `zend_verify_arg_type`, `zend_verify_return_type` and `zend_verify_missing_return_type` functions which produces an "or null"/"or be null" part in its error message
+
+function iterableF(?iterable $param) {}
+try {
+    iterableF(1);
+} catch (\TypeError $e) {
+    echo $e, PHP_EOL;
+}
+
+function returnIterable(): ?iterable {
+    return 1;
+}
+
+try {
+    returnIterable();
+} catch (\TypeError $e) {
+    echo $e, PHP_EOL;
+}
+
+function returnMissingIterable(): ?iterable {
+}
+
+try {
+    returnMissingIterable();
+} catch (\TypeError $e) {
+    echo $e, PHP_EOL;
+}
+?>
+--EXPECTF--
+TypeError: iterableF(): Argument #1 ($param) must be of type Traversable|array|null, int given, called in %s on line %d and defined in %s:%d
+Stack trace:
+#0 %s(%d): iterableF(1)
+#1 {main}
+TypeError: returnIterable(): Return value must be of type Traversable|array|null, int returned in %s:%d
+Stack trace:
+#0 %s(%d): returnIterable()
+#1 {main}
+TypeError: returnMissingIterable(): Return value must be of type Traversable|array|null, none returned in %s:%d
+Stack trace:
+#0 %s(%d): returnMissingIterable()
+#1 {main}

Zend/tests/typehints/or_null.phpt

@@ -57,14 +57,6 @@ try {
     echo $e, PHP_EOL;
 }
 
-function iterableF(?iterable $param) {}
-
-try {
-    iterableF(1);
-} catch (\TypeError $e) {
-    echo $e, PHP_EOL;
-}
-
 function intF(?int $param) {}
 
 try {
@@ -143,16 +135,6 @@ try {
     echo $e, PHP_EOL;
 }
 
-function returnIterable(): ?iterable {
-    return 1;
-}
-
-try {
-    returnIterable();
-} catch (\TypeError $e) {
-    echo $e, PHP_EOL;
-}
-
 function returnInt(): ?int {
     return new \StdClass;
 }
@@ -199,15 +181,6 @@ try {
     echo $e, PHP_EOL;
 }
 
-function returnMissingIterable(): ?iterable {
-}
-
-try {
-    returnMissingIterable();
-} catch (\TypeError $e) {
-    echo $e, PHP_EOL;
-}
-
 function returnMissingInt(): ?int {
 }
 
@@ -221,97 +194,85 @@ try {
 --EXPECTF--
 TypeError: unloadedClass(): Argument #1 ($param) must be of type ?I\Dont\Exist, stdClass given, called in %s:%d
 Stack trace:
-#0 %s(8): unloadedClass(Object(stdClass))
+#0 %s(%d): unloadedClass(Object(stdClass))
 #1 {main}
 TypeError: loadedClass(): Argument #1 ($param) must be of type ?RealClass, stdClass given, called in %s:%d
 Stack trace:
-#0 %s(20): loadedClass(Object(stdClass))
+#0 %s(%d): loadedClass(Object(stdClass))
 #1 {main}
 TypeError: loadedInterface(): Argument #1 ($param) must be of type ?RealInterface, stdClass given, called in %s:%d
 Stack trace:
-#0 %s(26): loadedInterface(Object(stdClass))
+#0 %s(%d): loadedInterface(Object(stdClass))
 #1 {main}
 TypeError: unloadedClass(): Argument #1 ($param) must be of type ?I\Dont\Exist, int given, called in %s:%d
 Stack trace:
-#0 %s(32): unloadedClass(1)
+#0 %s(%d): unloadedClass(1)
 #1 {main}
 TypeError: loadedClass(): Argument #1 ($param) must be of type ?RealClass, int given, called in %s:%d
 Stack trace:
-#0 %s(38): loadedClass(1)
+#0 %s(%d): loadedClass(1)
 #1 {main}
 TypeError: loadedInterface(): Argument #1 ($param) must be of type ?RealInterface, int given, called in %s:%d
 Stack trace:
-#0 %s(44): loadedInterface(1)
+#0 %s(%d): loadedInterface(1)
 #1 {main}
 TypeError: callableF(): Argument #1 ($param) must be of type ?callable, int given, called in %s:%d
 Stack trace:
-#0 %s(52): callableF(1)
-#1 {main}
-TypeError: iterableF(): Argument #1 ($param) must be of type ?iterable, int given, called in %s:%d
-Stack trace:
-#0 %s(60): iterableF(1)
+#0 %s(%d): callableF(1)
 #1 {main}
 TypeError: intF(): Argument #1 ($param) must be of type ?int, stdClass given, called in %s:%d
 Stack trace:
-#0 %s(68): intF(Object(stdClass))
+#0 %s(%d): intF(Object(stdClass))
 #1 {main}
 TypeError: returnUnloadedClass(): Return value must be of type ?I\Dont\Exist, stdClass returned in %s:%d
 Stack trace:
-#0 %s(78): returnUnloadedClass()
+#0 %s(%d): returnUnloadedClass()
 #1 {main}
 TypeError: returnLoadedClass(): Return value must be of type ?RealClass, stdClass returned in %s:%d
 Stack trace:
-#0 %s(88): returnLoadedClass()
+#0 %s(%d): returnLoadedClass()
 #1 {main}
 TypeError: returnLoadedInterface(): Return value must be of type ?RealInterface, stdClass returned in %s:%d
 Stack trace:
-#0 %s(98): returnLoadedInterface()
+#0 %s(%d): returnLoadedInterface()
 #1 {main}
 TypeError: returnUnloadedClassScalar(): Return value must be of type ?I\Dont\Exist, int returned in %s:%d
 Stack trace:
-#0 %s(108): returnUnloadedClassScalar()
+#0 %s(%d): returnUnloadedClassScalar()
 #1 {main}
 TypeError: returnLoadedClassScalar(): Return value must be of type ?RealClass, int returned in %s:%d
 Stack trace:
-#0 %s(118): returnLoadedClassScalar()
+#0 %s(%d): returnLoadedClassScalar()
 #1 {main}
 TypeError: returnLoadedInterfaceScalar(): Return value must be of type ?RealInterface, int returned in %s:%d
 Stack trace:
-#0 %s(128): returnLoadedInterfaceScalar()
+#0 %s(%d): returnLoadedInterfaceScalar()
 #1 {main}
 TypeError: returnCallable(): Return value must be of type ?callable, int returned in %s:%d
 Stack trace:
-#0 %s(138): returnCallable()
-#1 {main}
-TypeError: returnIterable(): Return value must be of type ?iterable, int returned in %s:%d
-Stack trace:
-#0 %s(148): returnIterable()
+#0 %s(%d): returnCallable()
 #1 {main}
 TypeError: returnInt(): Return value must be of type ?int, stdClass returned in %s:%d
 Stack trace:
-#0 %s(158): returnInt()
+#0 %s(%d): returnInt()
 #1 {main}
 TypeError: returnMissingUnloadedClass(): Return value must be of type ?I\Dont\Exist, none returned in %s:%d
 Stack trace:
-#0 %s(167): returnMissingUnloadedClass()
+#0 %s(%d): returnMissingUnloadedClass()
 #1 {main}
 TypeError: returnMissingLoadedClass(): Return value must be of type ?RealClass, none returned in %s:%d
 Stack trace:
-#0 %s(176): returnMissingLoadedClass()
+#0 %s(%d): returnMissingLoadedClass()
 #1 {main}
 TypeError: returnMissingLoadedInterface(): Return value must be of type ?RealInterface, none returned in %s:%d
 Stack trace:
-#0 %s(185): returnMissingLoadedInterface()
+#0 %s(%d): returnMissingLoadedInterface()
 #1 {main}
 TypeError: returnMissingCallable(): Return value must be of type ?callable, none returned in %s:%d
 Stack trace:
-#0 %s(194): returnMissingCallable()
-#1 {main}
-TypeError: returnMissingIterable(): Return value must be of type ?iterable, none returned in %s:%d
-Stack trace:
-#0 %s(203): returnMissingIterable()
+#0 %s(%d): returnMissingCallable()
 #1 {main}
 TypeError: returnMissingInt(): Return value must be of type ?int, none returned in %s:%d
 Stack trace:
-#0 %s(212): returnMissingInt()
+#0 %s(%d): returnMissingInt()
 #1 {main}

Zend/zend_compile.c

@@ -1192,29 +1192,11 @@ zend_string *zend_type_to_string_resolved(zend_type type, zend_class_entry *scop
 	uint32_t type_mask = ZEND_TYPE_PURE_MASK(type);
 	bool has_name = ZEND_TYPE_HAS_NAME(type);
 
-	/* BC for iterable type as a single type */
-	if (UNEXPECTED(ZEND_TYPE_IS_ITERABLE_FALLBACK(type))) {
-		str = add_type_string(str, ZSTR_KNOWN(ZEND_STR_ITERABLE), /* is_intersection */ false);
-		/* Remove array bit */
-		type_mask &= ~MAY_BE_ARRAY;
-		/* If it has a type name it must be Traversable, ignore it */
-		if (has_name) {
-			has_name = false;
-		}
-	}
-
 	if (ZEND_TYPE_HAS_LIST(type)) {
 		zend_type *list_type;
 		bool is_intersection = ZEND_TYPE_IS_INTERSECTION(type);
 		ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(type), list_type) {
 			zend_string *name = ZEND_TYPE_NAME(*list_type);
-			/* BC for iterable type in a union type */
-			if (UNEXPECTED(ZEND_TYPE_IS_ITERABLE_FALLBACK(*list_type) && zend_string_equals(name, ZSTR_KNOWN(ZEND_STR_TRAVERSABLE)))) {
-				/* Remove array bit */
-				type_mask &= ~MAY_BE_ARRAY;
-				str = add_type_string(str, ZSTR_KNOWN(ZEND_STR_ITERABLE), /* is_intersection */ false);
-				continue;
-			}
 			zend_string *resolved = resolve_class_name(name, scope);
 			str = add_type_string(str, resolved, is_intersection);
 			zend_string_release(resolved);

Zend/zend_types.h

@@ -268,7 +268,7 @@ typedef struct {
 
 #define ZEND_TYPE_INIT_CODE(code, allow_null, extra_flags) \
 	ZEND_TYPE_INIT_MASK(((code) == _IS_BOOL ? MAY_BE_BOOL : ( (code) == IS_ITERABLE ? _ZEND_TYPE_ITERABLE_BIT : ((code) == IS_MIXED ? MAY_BE_ANY : (1 << (code))))) \
-		| ((allow_null) ? _ZEND_TYPE_NULLABLE_BIT : 0) | (extra_flags)) \
+		| ((allow_null) ? _ZEND_TYPE_NULLABLE_BIT : 0) | (extra_flags))
 
 #define ZEND_TYPE_INIT_PTR(ptr, type_kind, allow_null, extra_flags) \
 	{ (void *) (ptr), \

ext/reflection/php_reflection.c

@@ -3064,7 +3064,6 @@ ZEND_METHOD(ReflectionUnionType, getTypes)
 	reflection_object *intern;
 	type_reference *param;
 	uint32_t type_mask;
-	bool has_iterable = false;
 
 	if (zend_parse_parameters_none() == FAILURE) {
 		RETURN_THROWS();
@@ -3075,10 +3074,6 @@ ZEND_METHOD(ReflectionUnionType, getTypes)
 	if (ZEND_TYPE_HAS_LIST(param->type)) {
 		zend_type *list_type;
 		ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(param->type), list_type) {
-			/* BC for iterable type */
-			if (UNEXPECTED(ZEND_TYPE_IS_ITERABLE_FALLBACK(*list_type))) {
-				has_iterable = true;
-			}
 			append_type(return_value, *list_type);
 		} ZEND_TYPE_LIST_FOREACH_END();
 	} else if (ZEND_TYPE_HAS_NAME(param->type)) {
@@ -3098,7 +3093,7 @@ ZEND_METHOD(ReflectionUnionType, getTypes)
 	if (type_mask & MAY_BE_OBJECT) {
 		append_type_mask(return_value, MAY_BE_OBJECT);
 	}
-	if ((type_mask & MAY_BE_ARRAY) && !has_iterable) {
+	if ((type_mask & MAY_BE_ARRAY)) {
 		append_type_mask(return_value, MAY_BE_ARRAY);
 	}
 	if (type_mask & MAY_BE_STRING) {

ext/reflection/tests/ReflectionType_001.phpt

@@ -80,7 +80,6 @@ class PropTypeTest {
     public int $int;
     public string $string;
     public array $arr;
-    public iterable $iterable;
     public stdClass $std;
     public OtherThing $other;
     public $mixed;
@@ -216,7 +215,6 @@ string(4) "Test"
 public int $int;
 public string $string;
 public array $arr;
-public iterable $iterable;
 public stdClass $std;
 public OtherThing $other;
 public $mixed;

ext/reflection/tests/ReflectionType_possible_types.phpt

@@ -11,7 +11,6 @@ $functions = [
     function(): bool {},
     function(): array {},
     function(): callable {},
-    function(): iterable {},
     function(): null {},
     function(): false {},
     function(): StdClass {}
@@ -31,7 +30,6 @@ string(6) "string"
 string(4) "bool"
 string(5) "array"
 string(8) "callable"
-string(8) "iterable"
 string(4) "null"
 string(5) "false"
 string(8) "StdClass"

ext/reflection/tests/bug72661.phpt

@@ -7,4 +7,4 @@ function test(iterable $arg) { }
 var_dump((string)(new ReflectionParameter("test", 0))->getType());
 ?>
 --EXPECT--
-string(8) "iterable"
+string(17) "Traversable|array"