Skip to content

Commit 03d7d81

Browse files
GirgiasGirgias
committed
Stop relying on the generic type name to find bound type
Use the index that is now part of the zend_type This should reduce memory consumption as we are not doing a second deep copy of the types Fixes one of the implicit interface tests
1 parent fff15c9 commit 03d7d81

File tree

3 files changed

+17
-39
lines changed

3 files changed

+17
-39
lines changed

‎Zend/tests/type_declarations/abstract_generics/constraints/implicit_interface_same_bound_types.phpt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
--TEST--
22
Implicit interface inheritance with same bound types
3-
--XFAIL--
4-
This emits an error when it shouldn't
53
--FILE--
64
<?php
75

‎Zend/zend_compile.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1434,7 +1434,8 @@ static zend_string *add_intersection_type(zend_string *str,
14341434
return str;
14351435
}
14361436

1437-
static zend_string* resolve_bound_generic_type(zend_string *type_name, const zend_class_entry *scope, const HashTable *bound_types) {
1437+
static zend_string* resolve_bound_generic_type(const zend_type *type, const zend_class_entry *scope, const HashTable *bound_types) {
1438+
const zend_string *type_name = ZEND_TYPE_NAME(*type);
14381439
if (bound_types == NULL) {
14391440
const size_t len = ZSTR_LEN(type_name) + strlen("<>");
14401441
zend_string *result = zend_string_alloc(len, 0);
@@ -1445,7 +1446,7 @@ static zend_string* resolve_bound_generic_type(zend_string *type_name, const zen
14451446
return result;
14461447
}
14471448

1448-
const zend_type *constraint = zend_hash_find_ptr(bound_types, type_name);
1449+
const zend_type *constraint = zend_hash_index_find_ptr(bound_types, type->generic_param_index);
14491450
ZEND_ASSERT(constraint != NULL);
14501451

14511452
zend_string *constraint_type_str = zend_type_to_string_resolved(*constraint, scope, NULL);
@@ -1490,7 +1491,7 @@ zend_string *zend_type_to_string_resolved(const zend_type type, const zend_class
14901491
zend_string_release(resolved);
14911492
} ZEND_TYPE_LIST_FOREACH_END();
14921493
} else if (ZEND_TYPE_IS_GENERIC_PARAM_NAME(type)) {
1493-
str = resolve_bound_generic_type(ZEND_TYPE_NAME(type), scope, bound_types_to_scope);
1494+
str = resolve_bound_generic_type(&type, scope, bound_types_to_scope);
14941495
} else if (ZEND_TYPE_HAS_NAME(type)) {
14951496
str = resolve_class_name(ZEND_TYPE_NAME(type), scope);
14961497
}

‎Zend/zend_inheritance.c‎

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -696,21 +696,20 @@ static inheritance_status zend_is_type_subtype_of_generic_type(
696696
ZEND_ASSERT(bound_generic_types && "Have generic type");
697697
ZEND_ASSERT(ZEND_TYPE_IS_GENERIC_PARAM_NAME(generic_type));
698698

699-
const zend_type *bound_type_ptr = zend_hash_find_ptr(bound_generic_types, ZEND_TYPE_NAME(generic_type));
699+
const zend_type *bound_type_ptr = zend_hash_index_find_ptr(bound_generic_types, generic_type.generic_param_index);
700700
ZEND_ASSERT(bound_type_ptr != NULL);
701701
if (ZEND_TYPE_IS_GENERIC_PARAM_NAME(*bound_type_ptr)) {
702702
const zend_type concrete_type = *concrete_type_ptr;
703703
if (
704704
ZEND_TYPE_IS_GENERIC_PARAM_NAME(concrete_type)
705-
&& zend_string_equals(ZEND_TYPE_NAME(concrete_type), ZEND_TYPE_NAME(*bound_type_ptr))
705+
&& concrete_type.generic_param_index == bound_type_ptr->generic_param_index
706706
) {
707707
return INHERITANCE_SUCCESS;
708708
} else {
709709
return INHERITANCE_ERROR;
710710
}
711711
} else {
712712
/* Generic type must be invariant */
713-
// TODO Use zend_perform_contravariant_type_check()?
714713
const inheritance_status sub_type_status = zend_perform_covariant_type_check(
715714
concrete_scope, concrete_type_ptr, generic_type_scope, bound_type_ptr);
716715
const inheritance_status super_type_status = zend_perform_contravariant_type_check(
@@ -821,43 +820,34 @@ static inheritance_status zend_is_generic_type_subtype_of_generic_type(
821820
const zend_class_entry *fe_scope, const zend_type *fe_type_ptr,
822821
const zend_class_entry *proto_scope, const zend_type *proto_type_ptr
823822
) {
824-
const zend_type fe_type = *fe_type_ptr;
825-
const zend_type proto_type = *proto_type_ptr;
826-
827-
if (UNEXPECTED(!ZEND_TYPE_IS_GENERIC_PARAM_NAME(proto_type))) {
823+
if (UNEXPECTED(!ZEND_TYPE_IS_GENERIC_PARAM_NAME(*proto_type_ptr))) {
828824
/* A generic type cannot be a subtype of a concrete one */
829825
return INHERITANCE_ERROR;
830826
}
831-
ZEND_ASSERT(ZEND_TYPE_IS_GENERIC_PARAM_NAME(fe_type));
827+
ZEND_ASSERT(ZEND_TYPE_IS_GENERIC_PARAM_NAME(*fe_type_ptr));
832828
ZEND_ASSERT(fe_scope->bound_types);
833829

834830
const HashTable *bound_generic_types = zend_hash_find_ptr_lc(fe_scope->bound_types, proto_scope->name);
835831
ZEND_ASSERT(bound_generic_types && "Must have bound generic type");
836832

837-
zend_string *proto_type_name = ZEND_TYPE_NAME(proto_type);
838-
ZEND_ASSERT(proto_type_name);
839-
840-
const zend_type *bound_type_ptr = zend_hash_find_ptr(bound_generic_types, proto_type_name);
833+
const zend_type *bound_type_ptr = zend_hash_index_find_ptr(bound_generic_types, proto_type_ptr->generic_param_index);
841834
ZEND_ASSERT(bound_type_ptr != NULL);
842835

843836
const zend_type bound_type = *bound_type_ptr;
844837
ZEND_ASSERT(ZEND_TYPE_IS_GENERIC_PARAM_NAME(bound_type));
845838

846-
return zend_string_equals(ZEND_TYPE_NAME(bound_type), ZEND_TYPE_NAME(fe_type)) ? INHERITANCE_SUCCESS : INHERITANCE_ERROR;
839+
return bound_type_ptr->generic_param_index == fe_type_ptr->generic_param_index ? INHERITANCE_SUCCESS : INHERITANCE_ERROR;
847840
}
848841

849842
static inheritance_status zend_perform_covariant_type_check(
850843
zend_class_entry *fe_scope, const zend_type *fe_type_ptr,
851844
zend_class_entry *proto_scope, const zend_type *proto_type_ptr
852845
) {
853-
const zend_type fe_type = *fe_type_ptr;
854-
const zend_type proto_type = *proto_type_ptr;
855-
856846
/* If we check for concrete return type */
857-
if (ZEND_TYPE_IS_GENERIC_PARAM_NAME(proto_type)) {
847+
if (ZEND_TYPE_IS_GENERIC_PARAM_NAME(*proto_type_ptr)) {
858848
return zend_is_type_subtype_of_generic_type(
859849
fe_scope, fe_type_ptr, proto_scope, proto_type_ptr);
860-
} else if (UNEXPECTED(ZEND_TYPE_IS_GENERIC_PARAM_NAME(fe_type))) {
850+
} else if (UNEXPECTED(ZEND_TYPE_IS_GENERIC_PARAM_NAME(*fe_type_ptr))) {
861851
/* A generic type cannot be a subtype of a concrete one */
862852
return INHERITANCE_ERROR;
863853
}
@@ -1057,7 +1047,7 @@ static ZEND_COLD zend_string *zend_get_function_declaration(
10571047
const zend_generic_parameter param = scope->generic_parameters[i];
10581048
zend_string *constraint_type_str;
10591049
if (bound_types_to_scope) {
1060-
const zend_type *constraint = zend_hash_find_ptr(bound_types_to_scope, param.name);
1050+
const zend_type *constraint = zend_hash_index_find_ptr(bound_types_to_scope, i);
10611051
ZEND_ASSERT(constraint != NULL);
10621052
constraint_type_str = zend_type_to_string_resolved(*constraint, scope, NULL);
10631053
} else {
@@ -2351,23 +2341,19 @@ ZEND_ATTRIBUTE_NONNULL static void bind_generic_types_for_inherited_interfaces(z
23512341
false /* TODO depends on internals */
23522342
);
23532343
ZEND_HASH_FOREACH_KEY_PTR(interface_bound_types_for_inherited_iface, generic_param_index, generic_param_name, bound_type_ptr) {
2344+
ZEND_ASSERT(generic_param_name == NULL); // TODO Change foreach macro;
23542345
zend_type bound_type = *bound_type_ptr;
23552346
if (ZEND_TYPE_IS_GENERIC_PARAM_NAME(bound_type)) {
23562347
ZEND_ASSERT(ce_bound_types_for_direct_iface != NULL &&
23572348
"If a bound type is generic then we must have bound types for the current interface");
2358-
const zend_type *ce_bound_type_ptr = zend_hash_find_ptr(ce_bound_types_for_direct_iface, ZEND_TYPE_NAME(bound_type));
2349+
const zend_type *ce_bound_type_ptr = zend_hash_index_find_ptr(ce_bound_types_for_direct_iface, bound_type_ptr->generic_param_index);
23592350
ZEND_ASSERT(ce_bound_type_ptr != NULL);
23602351
bound_type = *ce_bound_type_ptr;
23612352
}
23622353

23632354
zend_type_copy_ctor(&bound_type, true, false /* TODO Depends on internal or not? */);
2364-
if (generic_param_name) {
2365-
zend_hash_add_mem(ce_bound_types_for_inherited_iface, generic_param_name,
2366-
&bound_type, sizeof(bound_type));
2367-
} else {
2368-
zend_hash_index_add_mem(ce_bound_types_for_inherited_iface, generic_param_index,
2369-
&bound_type, sizeof(bound_type));
2370-
}
2355+
zend_hash_index_add_mem(ce_bound_types_for_inherited_iface, generic_param_index,
2356+
&bound_type, sizeof(bound_type));
23712357
} ZEND_HASH_FOREACH_END();
23722358

23732359
const HashTable *existing_bound_types_for_inherited_iface = zend_hash_find_ptr(ce->bound_types, lc_inherited_iface_name);
@@ -2505,13 +2491,6 @@ static void do_interface_implementation(zend_class_entry *ce, zend_class_entry *
25052491
return;
25062492
}
25072493
}
2508-
/* Deep type copy */
2509-
zend_type bound_type = *bound_type_ptr;
2510-
zend_type_copy_ctor(&bound_type, true, false);
2511-
zend_hash_add_mem(bound_types, generic_parameter->name,
2512-
&bound_type, sizeof(bound_type));
2513-
///* Should we change the key from index to generic parameter name? */
2514-
//zend_hash_index_del(bound_types, i);
25152494
}
25162495
}
25172496
bind_generic_types_for_inherited_interfaces(ce, iface);

0 commit comments

Comments
 (0)