Skip to content

Commit 52e85af

Browse files
GirgiasGirgias
committed
Did I finally fix generic type binding?
1 parent ad0816d commit 52e85af

11 files changed

+181
-24
lines changed

‎Zend/tests/type_declarations/abstract_generics/abstract_generic_type_with_constraint.phpt renamed to ‎Zend/tests/type_declarations/abstract_generics/constraints/abstract_generic_type_with_constraint.phpt

File renamed without changes.

‎Zend/tests/type_declarations/abstract_generics/abstract_generic_type_with_constraint_failed.phpt renamed to ‎Zend/tests/type_declarations/abstract_generics/constraints/abstract_generic_type_with_constraint_failed.phpt

File renamed without changes.

‎Zend/tests/type_declarations/abstract_generics/constraints/errors/using_generic_type_as_constraint.phpt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--TEST--
2+
Cannot use generic type as a constraint
3+
--FILE--
4+
<?php
5+
6+
interface I<T1, T2 : T1> {
7+
public function foo(T $param): T;
8+
}
9+
10+
?>
11+
--EXPECTF--
12+
Fatal error: Cannot use generic parameter T1 to constrain generic parameter T2 in %s on line %d

‎Zend/tests/type_declarations/abstract_generics/constraints/errors/using_never_as_constraint.phpt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--TEST--
2+
Cannot use never as a constraint
3+
--FILE--
4+
<?php
5+
6+
interface I<T : never> {
7+
public function foo(T $param): T;
8+
}
9+
10+
?>
11+
--EXPECTF--
12+
Fatal error: Cannot use static, void, or never to constrain generic parameter T in %s on line %d

‎Zend/tests/type_declarations/abstract_generics/constraints/errors/using_static_as_constraint.phpt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--TEST--
2+
Cannot use void as a constraint
3+
--FILE--
4+
<?php
5+
6+
interface I<T : static> {
7+
public function foo(T $param): T;
8+
}
9+
10+
?>
11+
--EXPECTF--
12+
Fatal error: Cannot use static, void, or never to constrain generic parameter T in %s on line %d

‎Zend/tests/type_declarations/abstract_generics/constraints/errors/using_void_as_constraint.phpt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
--TEST--
2+
Cannot use void as a constraint
3+
--FILE--
4+
<?php
5+
6+
interface I<T : void> {
7+
public function foo(T $param): T;
8+
}
9+
10+
?>
11+
--EXPECTF--
12+
Fatal error: Cannot use static, void, or never to constrain generic parameter T in %s on line %d

‎Zend/tests/type_declarations/abstract_generics/extended_interface_abstract_generic_types_1_to_2.phpt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
Abstract generic type behaviour in extended interface 2 generic type to 1
3+
--FILE--
4+
<?php
5+
6+
interface I<T1, T2> {
7+
public function foo(T1 $param): T2;
8+
}
9+
10+
interface I2<S> extends I<S, S> {
11+
public function bar(int $o, S $param): S;
12+
}
13+
14+
class C implements I2<string> {
15+
public function foo(string $param): string {}
16+
public function bar(int $o, string $param): string {}
17+
}
18+
19+
?>
20+
DONE
21+
--EXPECT--
22+
DONE

‎Zend/tests/type_declarations/abstract_generics/extended_interface_abstract_generic_types_basic.phpt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
Abstract generic type behaviour in extended interface
3+
--FILE--
4+
<?php
5+
6+
interface IWG<T1> {
7+
public function foo(T1 $param): T1;
8+
}
9+
10+
interface I2<T2> extends IWG<T2> {
11+
public function bar(int $o, T2 $param): T2;
12+
}
13+
14+
class C implements I2<string> {
15+
public function foo(string $param): string {}
16+
public function bar(int $o, string $param): string {}
17+
}
18+
19+
?>
20+
DONE
21+
--EXPECT--
22+
DONE

‎Zend/tests/type_declarations/abstract_generics/extended_interface_new_abstract_generic_type2.phpt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
--TEST--
22
Abstract generic type behaviour in extended interface
3-
--XFAIL--
4-
Generic type is not properly bound (Declaration of C::foo(string $param): string must be compatible with I::foo(T $param): T)
53
--FILE--
64
<?php
75

‎Zend/zend_compile.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ void init_compiler(void) /* {{{ */
465465
CG(delayed_autoloads) = NULL;
466466
CG(unlinked_uses) = NULL;
467467
CG(current_linking_class) = NULL;
468-
CG(bound_associated_types) = NULL;
468+
CG(bound_generic_types) = NULL;
469469
}
470470
/* }}} */
471471

@@ -495,10 +495,10 @@ void shutdown_compiler(void) /* {{{ */
495495
}
496496
CG(current_linking_class) = NULL;
497497
/* This can happen during a fatal error */
498-
if (CG(bound_associated_types)) {
499-
zend_hash_destroy(CG(bound_associated_types));
500-
FREE_HASHTABLE(CG(bound_associated_types));
501-
CG(bound_associated_types) = NULL;
498+
if (CG(bound_generic_types)) {
499+
zend_hash_destroy(CG(bound_generic_types));
500+
FREE_HASHTABLE(CG(bound_generic_types));
501+
CG(bound_generic_types) = NULL;
502502
}
503503
}
504504
/* }}} */
@@ -9180,9 +9180,18 @@ static void zend_compile_generic_params(zend_ast *params_ast)
91809180
}
91819181

91829182
if (param_ast->child[1]) {
9183-
// TODO Need to free this
9183+
// TODO Need to free this?
91849184
constraint_type = zend_compile_typename(param_ast->child[1]);
9185-
// TODO Validate that void, static, never are not used in the constraint?
9185+
if (ZEND_TYPE_IS_ASSOCIATED(constraint_type)) {
9186+
zend_error_noreturn(E_COMPILE_ERROR,
9187+
"Cannot use generic parameter %s to constrain generic parameter %s",
9188+
ZSTR_VAL(ZEND_TYPE_NAME(constraint_type)), ZSTR_VAL(name));
9189+
}
9190+
if (ZEND_TYPE_FULL_MASK(constraint_type) & (MAY_BE_STATIC|MAY_BE_VOID|MAY_BE_NEVER)) {
9191+
zend_error_noreturn(E_COMPILE_ERROR,
9192+
"Cannot use static, void, or never to constrain generic parameter %s",
9193+
ZSTR_VAL(name));
9194+
}
91869195
}
91879196

91889197
generic_params[i].name = zend_string_copy(name);

‎Zend/zend_inheritance.c

Lines changed: 73 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -685,12 +685,14 @@ static inheritance_status zend_is_type_subtype_of_generic_type(
685685
zend_class_entry *generic_type_scope,
686686
const zend_type *generic_type_ptr
687687
) {
688+
ZEND_ASSERT(concrete_scope->bound_types);
688689
const zend_type generic_type = *generic_type_ptr;
690+
const HashTable *bound_generic_types = zend_hash_find_ptr_lc(concrete_scope->bound_types, generic_type_scope->name);
689691

690-
ZEND_ASSERT(CG(bound_generic_types) && "Have generic type");
692+
ZEND_ASSERT(bound_generic_types && "Have generic type");
691693
ZEND_ASSERT(ZEND_TYPE_HAS_NAME(generic_type));
692694

693-
const zend_type *bound_type_ptr = zend_hash_find_ptr(CG(bound_generic_types), ZEND_TYPE_NAME(generic_type));
695+
const zend_type *bound_type_ptr = zend_hash_find_ptr(bound_generic_types, ZEND_TYPE_NAME(generic_type));
694696
ZEND_ASSERT(bound_type_ptr != NULL);
695697
/* Generic type must be invariant */
696698
const inheritance_status sub_type_status = zend_perform_covariant_type_check(
@@ -1638,6 +1640,57 @@ static inline void do_implement_interface(zend_class_entry *ce, zend_class_entry
16381640
}
16391641
/* }}} */
16401642

1643+
// TODO Merge with the one in zend_compile
1644+
static void zend_types_ht_dtor(zval *ptr) {
1645+
zend_type *type = Z_PTR_P(ptr);
1646+
// TODO Figure out persistency?
1647+
zend_type_release(*type, false);
1648+
efree(type);
1649+
}
1650+
1651+
static void interface_bind_generic_types_for_interfaces(zend_class_entry *ce, const zend_class_entry *iface) {
1652+
zend_string *iface_lc_name = zend_string_tolower(iface->name);
1653+
const HashTable *ce_bound_types = ce->bound_types ? zend_hash_find_ptr(ce->bound_types, iface_lc_name) : NULL;
1654+
for (uint32_t i = 0; i < iface->num_interfaces; i++) {
1655+
zend_class_entry *entry = iface->interfaces[i];
1656+
/* Bind generic types */
1657+
/* We need to propagate the bound generic parameters to the inherited interfaces */
1658+
if (entry->num_generic_parameters == 0) {
1659+
continue;
1660+
}
1661+
zend_string *inherited_iface_lc_name = zend_string_tolower(entry->name);
1662+
const HashTable *interface_bound_types = zend_hash_find_ptr(iface->bound_types, inherited_iface_lc_name);
1663+
HashTable *ce_bound_types_to_inherited_iface = zend_hash_find_ptr(ce->bound_types, inherited_iface_lc_name);
1664+
ZEND_ASSERT(interface_bound_types != NULL && "This must exist at this point");
1665+
if (ce_bound_types_to_inherited_iface == NULL) {
1666+
ALLOC_HASHTABLE(ce_bound_types_to_inherited_iface);
1667+
zend_hash_init(ce_bound_types_to_inherited_iface, entry->num_generic_parameters, NULL, zend_types_ht_dtor, false /* todo depend on internal or not */);
1668+
zend_hash_add_new_ptr(ce->bound_types, inherited_iface_lc_name, ce_bound_types_to_inherited_iface);
1669+
}
1670+
for (
1671+
uint32_t inherited_iface_generic_param_index = 0;
1672+
inherited_iface_generic_param_index < entry->num_generic_parameters;
1673+
inherited_iface_generic_param_index++
1674+
) {
1675+
const zend_generic_parameter *inherited_generic_parameter = &entry->generic_parameters[inherited_iface_generic_param_index];
1676+
const zend_type *iface_bound_type_ptr = zend_hash_index_find_ptr(interface_bound_types, inherited_iface_generic_param_index);
1677+
ZEND_ASSERT(iface_bound_type_ptr != NULL);
1678+
zend_type bound_type;
1679+
if (ZEND_TYPE_IS_ASSOCIATED(*iface_bound_type_ptr)) {
1680+
memcpy(&bound_type, zend_hash_find_ptr(ce_bound_types, ZEND_TYPE_NAME(*iface_bound_type_ptr)), sizeof(zend_type));
1681+
} else {
1682+
bound_type = *iface_bound_type_ptr;
1683+
}
1684+
/* Deep type copy */
1685+
zend_type_copy_ctor(&bound_type, true, false);
1686+
zend_hash_add_mem(ce_bound_types_to_inherited_iface, inherited_generic_parameter->name,
1687+
&bound_type, sizeof(bound_type));
1688+
}
1689+
zend_string_release_ex(inherited_iface_lc_name, false);
1690+
}
1691+
zend_string_release_ex(iface_lc_name, false);
1692+
}
1693+
16411694
static void zend_do_inherit_interfaces(zend_class_entry *ce, const zend_class_entry *iface) /* {{{ */
16421695
{
16431696
/* expects interface to be contained in ce's interface list already */
@@ -1656,6 +1709,7 @@ static void zend_do_inherit_interfaces(zend_class_entry *ce, const zend_class_en
16561709
zend_class_entry *entry = iface->interfaces[if_num];
16571710
for (i = 0; i < ce_num; i++) {
16581711
if (ce->interfaces[i] == entry) {
1712+
// TODO Check bound generic types match? Or is this done before?
16591713
break;
16601714
}
16611715
}
@@ -2222,7 +2276,7 @@ static void do_interface_implementation(zend_class_entry *ce, zend_class_entry *
22222276
ZSTR_VAL(iface->name)
22232277
);
22242278
}
2225-
const HashTable *bound_types = zend_hash_find_ptr_lc(ce->bound_types, iface->name);
2279+
HashTable *bound_types = zend_hash_find_ptr_lc(ce->bound_types, iface->name);
22262280
if (UNEXPECTED(bound_types == NULL)) {
22272281
zend_error_noreturn(E_COMPILE_ERROR,
22282282
"Cannot implement %s as it has generic parameters which are not specified",
@@ -2238,9 +2292,6 @@ static void do_interface_implementation(zend_class_entry *ce, zend_class_entry *
22382292
iface->num_generic_parameters
22392293
);
22402294
}
2241-
HashTable *ht = emalloc(sizeof(HashTable));
2242-
zend_hash_init(ht, num_bound_types, NULL, NULL, false);
2243-
CG(bound_generic_types) = ht;
22442295
for (uint32_t i = 0; i < num_bound_types; i++) {
22452296
const zend_generic_parameter *generic_parameter = &iface->generic_parameters[i];
22462297
const zend_type* generic_constraint = &generic_parameter->constraint;
@@ -2260,7 +2311,14 @@ static void do_interface_implementation(zend_class_entry *ce, zend_class_entry *
22602311
}
22612312
const zend_type *current_ce_generic_type_constraint = &current_ce_generic_parameter->constraint;
22622313
ZEND_ASSERT(current_ce_generic_type_constraint != NULL);
2263-
if (zend_perform_covariant_type_check(ce, current_ce_generic_type_constraint, iface, generic_constraint) != INHERITANCE_SUCCESS) {
2314+
if (
2315+
zend_perform_covariant_type_check(
2316+
ce,
2317+
current_ce_generic_type_constraint,
2318+
iface,
2319+
generic_constraint
2320+
) != INHERITANCE_SUCCESS
2321+
) {
22642322
zend_string *current_ce_constraint_type_str = zend_type_to_string(*current_ce_generic_type_constraint);
22652323
zend_string *constraint_type_str = zend_type_to_string(generic_parameter->constraint);
22662324
zend_error_noreturn(E_COMPILE_ERROR,
@@ -2276,8 +2334,6 @@ static void do_interface_implementation(zend_class_entry *ce, zend_class_entry *
22762334
zend_string_release(constraint_type_str);
22772335
return;
22782336
}
2279-
/* Loosing const qualifier here is OK because this hashtable never frees or does anything with the value */
2280-
zend_hash_add_new_ptr(ht, generic_parameter->name, (void*)current_ce_generic_type_constraint);
22812337
break;
22822338
}
22832339
} else {
@@ -2295,10 +2351,17 @@ static void do_interface_implementation(zend_class_entry *ce, zend_class_entry *
22952351
zend_string_release(constraint_type_str);
22962352
return;
22972353
}
2298-
zend_hash_add_new_ptr(ht, generic_parameter->name, bound_type_ptr);
2354+
/* Change key from index to generic parameter name */
2355+
/* Deep type copy */
2356+
zend_type bound_type = *bound_type_ptr;
2357+
zend_type_copy_ctor(&bound_type, true, false);
2358+
zend_hash_add_mem(bound_types, generic_parameter->name,
2359+
&bound_type, sizeof(bound_type));
2360+
//zend_hash_index_del(bound_types, i);
22992361
}
23002362
}
23012363
}
2364+
interface_bind_generic_types_for_interfaces(ce, iface);
23022365
ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&iface->constants_table, key, c) {
23032366
do_inherit_iface_constant(key, c, ce, iface);
23042367
} ZEND_HASH_FOREACH_END();
@@ -2320,11 +2383,6 @@ static void do_interface_implementation(zend_class_entry *ce, zend_class_entry *
23202383
if (iface->num_interfaces) {
23212384
zend_do_inherit_interfaces(ce, iface);
23222385
}
2323-
if (CG(bound_generic_types)) {
2324-
zend_hash_destroy(CG(bound_generic_types));
2325-
efree(CG(bound_generic_types));
2326-
CG(bound_generic_types) = NULL;
2327-
}
23282386
}
23292387
/* }}} */
23302388

0 commit comments

Comments
 (0)