Skip to content

Fix UPGRADING and #9556 "iterable" alias "array|Traversable" breaks PHP 8.1 code #9558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ PHP 8.2 UPGRADE NOTES

- OpenSSL:
. openssl_cipher_key_length(): Returns a key length for the supplied
cipher.
cipher.

- Reflection:
. ReflectionFunction::isAnonymous()
Expand Down Expand Up @@ -400,6 +400,10 @@ PHP 8.2 UPGRADE NOTES
. CURL_VERSION_UNICODE (libcurl >= 7.72.0)
. CURL_VERSION_ZSTD (libcurl >= 7.72.0)

- DBA
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated?

. DBA_LMDB_USE_SUB_DIR
. DBA_LMDB_NO_SUB_DIR

- Filter
. FILTER_FLAG_GLOBAL_RANGE

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
A DNF type which contains object is redundant
--FILE--
<?php

interface A {}
interface B {}

function test(): (A&B)|object {}

?>
===DONE===
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

===DONE=== is kinda pointless, since Fatal error indicates termination. It's also used inconsistently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That ===DONE=== has been used in the past to easily see segfaults (which may not have been reported), but this is indeed obsolete since we now report any unexpected exit ("Termsig") on all platforms. However, according to https://qa.php.net/write-test.php#lastbit, there is another use case, and it seems this is still supported by run-tests.php. Still, without any code following ===DONE===, there's no need to use it. (And frankly, I doubt that ===DONE=== is actually useful for that purpose.)

--EXPECTF--
Fatal error: Type (A&B)|object contains both object and a class type, which is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
A DNF type which contains object is redundant 2
--FILE--
<?php

interface A {}
interface B {}

function test(): object|(A&B) {}

?>
===DONE===
--EXPECTF--
Fatal error: Type (A&B)|object contains both object and a class type, which is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
iterable type with array should be redundant
--FILE--
<?php

function bar(): array|iterable|null {
return null;
}

?>
--EXPECTF--
Fatal error: Duplicate type array is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
iterable type with array should be redundant
--FILE--
<?php

function bar(): iterable|array|null {
return null;
}

?>
--EXPECTF--
Fatal error: Duplicate type array is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
iterable type with second iterable should be redundant
--FILE--
<?php

function bar(): iterable|iterable|null {
return null;
}

?>
--EXPECTF--
Fatal error: Duplicate type array is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
--TEST--
iterable type with object should NOT be redundant 1
--FILE--
<?php

function bar(): iterable|object|null {
return null;
}

?>
===DONE===
--EXPECT--
===DONE===
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
--TEST--
iterable type with object should NOT be redundant 2
--FILE--
<?php

function bar(): object|iterable|null {
return null;
}

?>
===DONE===
--EXPECT--
===DONE===
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
iterable type with object and class T should be redundant
--FILE--
<?php

function bar(): object|iterable|T|null {
return null;
}

?>
--EXPECTF--
Fatal error: Type Traversable|T|object|array|null contains both object and a class type, which is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
iterable type with object and class T should be redundant
--FILE--
<?php

function bar(): iterable|object|T|null {
return null;
}

?>
--EXPECTF--
Fatal error: Type Traversable|T|object|array|null contains both object and a class type, which is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
iterable type with object and class T should be redundant
--FILE--
<?php

function bar(): T|object|iterable|null {
return null;
}

?>
--EXPECTF--
Fatal error: Type T|Traversable|object|array|null contains both object and a class type, which is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
iterable type with object and class T should be redundant
--FILE--
<?php

function bar(): T|iterable|object|null {
return null;
}

?>
--EXPECTF--
Fatal error: Type T|Traversable|object|array|null contains both object and a class type, which is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
--TEST--
iterable type with object should be allowed in variance checks
--FILE--
<?php

class A {
public object|iterable $x;
public object|array $y;
}
class B extends A {
public object|array $x;
public object|iterable $y;
}

?>
===DONE===
--EXPECT--
===DONE===
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
--TEST--
Using both object and a class type 2
--FILE--
<?php

function test(): Test|object {
}

?>
--EXPECTF--
Fatal error: Type Test|object contains both object and a class type, which is redundant in %s on line %d
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--TEST--
object and static are redundant 2
--FILE--
<?php

class Test {
public function foo(): object|static {}
}

?>
--EXPECTF--
Fatal error: Type static|object contains both object and a class type, which is redundant in %s on line %d
32 changes: 21 additions & 11 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -6363,6 +6363,7 @@ static zend_type zend_compile_typename(
zend_ast_list *list = zend_ast_get_list(ast);
zend_type_list *type_list;
bool is_composite = false;
bool has_only_iterable_class = true;
ALLOCA_FLAG(use_heap)

type_list = do_alloca(ZEND_TYPE_LIST_SIZE(list->children), use_heap);
Expand All @@ -6371,17 +6372,20 @@ static zend_type zend_compile_typename(
for (uint32_t i = 0; i < list->children; i++) {
zend_ast *type_ast = list->child[i];
zend_type single_type;
uint32_t type_mask = ZEND_TYPE_FULL_MASK(type);

if (type_ast->kind == ZEND_AST_TYPE_INTERSECTION) {
has_only_iterable_class = false;
is_composite = true;
/* The first class type can be stored directly as the type ptr payload. */
if (ZEND_TYPE_IS_COMPLEX(type) && !ZEND_TYPE_HAS_LIST(type)) {
/* Switch from single name to name list. */
type_list->num_types = 1;
type_list->types[0] = type;
ZEND_TYPE_FULL_MASK(type_list->types[0]) &= ~_ZEND_TYPE_MAY_BE_MASK;
ZEND_TYPE_SET_LIST(type, type_list);
}
/* Mark type as list type */
ZEND_TYPE_SET_LIST(type, type_list);

single_type = zend_compile_typename(type_ast, false);
ZEND_ASSERT(ZEND_TYPE_IS_INTERSECTION(single_type));
Expand All @@ -6406,6 +6410,9 @@ static zend_type zend_compile_typename(
if (single_type_mask == MAY_BE_ANY) {
zend_error_noreturn(E_COMPILE_ERROR, "Type mixed can only be used as a standalone type");
}
if (ZEND_TYPE_IS_COMPLEX(single_type) && !ZEND_TYPE_IS_ITERABLE_FALLBACK(single_type)) {
has_only_iterable_class = false;
}

uint32_t type_mask_overlap = ZEND_TYPE_PURE_MASK(type) & single_type_mask;
if (type_mask_overlap) {
Expand All @@ -6414,8 +6421,9 @@ static zend_type zend_compile_typename(
zend_error_noreturn(E_COMPILE_ERROR,
"Duplicate type %s is redundant", ZSTR_VAL(overlap_type_str));
}
if ( ((ZEND_TYPE_PURE_MASK(type) & MAY_BE_TRUE) && (single_type_mask == MAY_BE_FALSE))
|| ((ZEND_TYPE_PURE_MASK(type) & MAY_BE_FALSE) && (single_type_mask == MAY_BE_TRUE)) ) {

if ( ((type_mask & MAY_BE_TRUE) && (single_type_mask == MAY_BE_FALSE))
|| ((type_mask & MAY_BE_FALSE) && (single_type_mask == MAY_BE_TRUE)) ) {
zend_error_noreturn(E_COMPILE_ERROR,
"Type contains both true and false, bool should be used instead");
}
Expand Down Expand Up @@ -6455,6 +6463,15 @@ static zend_type zend_compile_typename(
}

free_alloca(type_list, use_heap);

uint32_t type_mask = ZEND_TYPE_FULL_MASK(type);
if ((type_mask & MAY_BE_OBJECT) &&
((!has_only_iterable_class && ZEND_TYPE_IS_COMPLEX(type)) || (type_mask & MAY_BE_STATIC))) {
zend_string *type_str = zend_type_to_string(type);
zend_error_noreturn(E_COMPILE_ERROR,
"Type %s contains both object and a class type, which is redundant",
ZSTR_VAL(type_str));
}
} else if (ast->kind == ZEND_AST_TYPE_INTERSECTION) {
zend_ast_list *list = zend_ast_get_list(ast);
zend_type_list *type_list;
Expand Down Expand Up @@ -6515,13 +6532,6 @@ static zend_type zend_compile_typename(
zend_error_noreturn(E_COMPILE_ERROR, "Type mixed cannot be marked as nullable since mixed already includes null");
}

if ((type_mask & MAY_BE_OBJECT) && (ZEND_TYPE_IS_COMPLEX(type) || (type_mask & MAY_BE_STATIC))) {
zend_string *type_str = zend_type_to_string(type);
zend_error_noreturn(E_COMPILE_ERROR,
"Type %s contains both object and a class type, which is redundant",
ZSTR_VAL(type_str));
}

if ((type_mask & MAY_BE_NULL) && is_marked_nullable) {
zend_error_noreturn(E_COMPILE_ERROR, "null cannot be marked as nullable");
}
Expand Down Expand Up @@ -8083,7 +8093,7 @@ static void zend_compile_use(zend_ast *ast) /* {{{ */

/* Check that we are not attempting to alias a built-in type */
if (type == ZEND_SYMBOL_CLASS && zend_is_reserved_class_name(old_name)) {
zend_error_noreturn(E_COMPILE_ERROR,
zend_error_noreturn(E_COMPILE_ERROR,
"Cannot alias '%s' as it is a built-in type", ZSTR_VAL(old_name));
}

Expand Down