Skip to content

Commit b96379f

Browse files
kocsismatekocsismate
committed
Address new review comments
1 parent 2b59df1 commit b96379f

16 files changed

+72
-52
lines changed

‎Zend/Optimizer/zend_func_info.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -844,7 +844,7 @@ ZEND_API uint32_t zend_get_func_info(
844844
#endif
845845

846846
ret = zend_get_return_info_from_signature_only(
847-
callee_func, /* script */ NULL, ce, ce_is_instanceof);
847+
callee_func, /* script */ NULL, ce, ce_is_instanceof, /* use_tentative_return_info */ 1);
848848

849849
#if ZEND_DEBUG
850850
if (internal_ret) {
@@ -884,7 +884,7 @@ ZEND_API uint32_t zend_get_func_info(
884884
}
885885
if (!ret) {
886886
ret = zend_get_return_info_from_signature_only(
887-
callee_func, /* TODO: script */ NULL, ce, ce_is_instanceof);
887+
callee_func, /* TODO: script */ NULL, ce, ce_is_instanceof, /* use_tentative_return_info */ 0);
888888
}
889889
}
890890
return ret;

‎Zend/Optimizer/zend_inference.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4001,9 +4001,11 @@ static int is_recursive_tail_call(const zend_op_array *op_array,
40014001

40024002
uint32_t zend_get_return_info_from_signature_only(
40034003
const zend_function *func, const zend_script *script,
4004-
zend_class_entry **ce, bool *ce_is_instanceof) {
4004+
zend_class_entry **ce, bool *ce_is_instanceof, bool use_tentative_return_info) {
40054005
uint32_t type;
4006-
if (func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE && !ZEND_ARG_TYPE_IS_TENTATIVE(func->common.arg_info - 1)) {
4006+
if (func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE &&
4007+
(use_tentative_return_info || !ZEND_ARG_TYPE_IS_TENTATIVE(func->common.arg_info - 1))
4008+
) {
40074009
zend_arg_info *ret_info = func->common.arg_info - 1;
40084010
type = zend_fetch_arg_info_type(script, ret_info, ce);
40094011
*ce_is_instanceof = ce != NULL;
@@ -4026,12 +4028,11 @@ ZEND_API void zend_init_func_return_info(
40264028
const zend_op_array *op_array, const zend_script *script, zend_ssa_var_info *ret)
40274029
{
40284030
ZEND_ASSERT((op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE));
4029-
ZEND_ASSERT(!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) op_array)->common.arg_info[-1]));
40304031

40314032
zend_ssa_range tmp_range = {0, 0, 0, 0};
40324033
bool is_instanceof = false;
40334034
ret->type = zend_get_return_info_from_signature_only(
4034-
(zend_function *) op_array, script, &ret->ce, &is_instanceof);
4035+
(zend_function *) op_array, script, &ret->ce, &is_instanceof, /* use_tentative_return_info */ 1);
40354036
ret->is_instanceof = is_instanceof;
40364037
ret->range = tmp_range;
40374038
ret->has_range = 0;

‎Zend/Optimizer/zend_inference.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ ZEND_API void zend_init_func_return_info(
271271
const zend_op_array *op_array, const zend_script *script, zend_ssa_var_info *ret);
272272
uint32_t zend_get_return_info_from_signature_only(
273273
const zend_function *func, const zend_script *script,
274-
zend_class_entry **ce, bool *ce_is_instanceof);
274+
zend_class_entry **ce, bool *ce_is_instanceof, bool use_tentative_return_info);
275275
void zend_func_return_info(const zend_op_array *op_array,
276276
const zend_script *script,
277277
int recursive,

‎Zend/Optimizer/zend_optimizer.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,9 +1446,7 @@ ZEND_API int zend_optimize_script(zend_script *script, zend_long optimization_le
14461446
func_info = ZEND_FUNC_INFO(call_graph.op_arrays[i]);
14471447
if (func_info) {
14481448
func_info->call_map = zend_build_call_map(&ctx.arena, func_info, call_graph.op_arrays[i]);
1449-
if ((call_graph.op_arrays[i]->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) &&
1450-
!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) call_graph.op_arrays[i])->common.arg_info[-1])
1451-
) {
1449+
if (call_graph.op_arrays[i]->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
14521450
zend_init_func_return_info(call_graph.op_arrays[i], script, &func_info->return_info);
14531451
}
14541452
}

‎Zend/tests/type_declarations/variance/internal_parent/internal_parent.phpt renamed to ‎Zend/tests/type_declarations/variance/internal_parent/unresolvable_inheritance_check_param.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
Internal class as parent
2+
Test unresolvable inheritance check due to unavailable parameter type when the parent has a tentative return type.
33
--FILE--
44
<?php
55

‎Zend/tests/type_declarations/variance/internal_parent/internal_parent2.phpt renamed to ‎Zend/tests/type_declarations/variance/internal_parent/unresolvable_inheritance_check_return.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
Internal class as parent
2+
Test unresolvable inheritance check due to unavailable return type when the parent has a tentative return type.
33
--FILE--
44
<?php
55

‎Zend/tests/type_declarations/variance/return_type_will_change_class_error.phpt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
--TEST--
2+
Test that the ReturnTypeWillChange attribute cannot target classes
3+
--FILE--
4+
<?php
5+
6+
#[ReturnTypeWillChange]
7+
class Foo
8+
{
9+
}
10+
11+
?>
12+
--EXPECTF--
13+
Fatal error: Attribute "ReturnTypeWillChange" cannot target class (allowed targets: method) in %s on line %d

‎Zend/tests/type_declarations/variance/return_type_will_change_function_error.phpt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--TEST--
2+
Test that the ReturnTypeWillChange attribute cannot target functions
3+
--FILE--
4+
<?php
5+
6+
#[ReturnTypeWillChange]
7+
function foo() {}
8+
9+
?>
10+
--EXPECTF--
11+
Fatal error: Attribute "ReturnTypeWillChange" cannot target function (allowed targets: method) in %s on line %d

‎Zend/tests/type_declarations/variance/return_type_will_change_property_error.phpt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--TEST--
2+
Test that the ReturnTypeWillChange attribute cannot be used with functions
3+
--FILE--
4+
<?php
5+
6+
class Foo
7+
{
8+
#[ReturnTypeWillChange]
9+
public int $bar;
10+
}
11+
12+
?>
13+
--EXPECTF--
14+
Fatal error: Attribute "ReturnTypeWillChange" cannot target property (allowed targets: method) in %s on line %d

‎Zend/zend_attributes.c

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,6 @@ void validate_attribute(zend_attribute *attr, uint32_t target, zend_class_entry
5656
}
5757
}
5858

59-
void validate_return_type_will_change_attribute(zend_attribute *attr, uint32_t target, zend_class_entry *scope)
60-
{
61-
if (target != ZEND_ATTRIBUTE_TARGET_METHOD) {
62-
zend_error(E_COMPILE_ERROR, "Only methods can be marked with #[ReturnTypeWillChange]");
63-
}
64-
}
65-
6659
ZEND_METHOD(Attribute, __construct)
6760
{
6861
zend_long flags = ZEND_ATTRIBUTE_TARGET_ALL;
@@ -293,8 +286,7 @@ void zend_register_attribute_ce(void)
293286
zend_declare_class_constant_long(zend_ce_attribute, ZEND_STRL("IS_REPEATABLE"), ZEND_ATTRIBUTE_IS_REPEATABLE);
294287

295288
zend_ce_return_type_will_change_attribute = register_class_ReturnTypeWillChange();
296-
attr = zend_internal_attribute_register(zend_ce_return_type_will_change_attribute, ZEND_ATTRIBUTE_TARGET_METHOD);
297-
attr->validator = validate_return_type_will_change_attribute;
289+
zend_internal_attribute_register(zend_ce_return_type_will_change_attribute, ZEND_ATTRIBUTE_TARGET_METHOD);
298290
}
299291

300292
void zend_attributes_shutdown(void)

‎Zend/zend_compile.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,7 +1305,7 @@ static void zend_mark_function_as_generator() /* {{{ */
13051305
"The \"yield\" expression can only be used inside a function");
13061306
}
13071307

1308-
if (CG(active_op_array)->fn_flags & ZEND_ACC_HAS_RETURN_TYPE && !ZEND_ARG_TYPE_IS_TENTATIVE(&(CG(active_op_array))->arg_info[-1])) {
1308+
if (CG(active_op_array)->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
13091309
zend_type return_type = CG(active_op_array)->arg_info[-1].type;
13101310
bool valid_type = (ZEND_TYPE_FULL_MASK(return_type) & (MAY_BE_ITERABLE | MAY_BE_OBJECT)) != 0;
13111311
if (!valid_type) {
@@ -2424,7 +2424,7 @@ static void zend_emit_return_type_check(
24242424
znode *expr, zend_arg_info *return_info, bool implicit) /* {{{ */
24252425
{
24262426
zend_type type = return_info->type;
2427-
if (ZEND_TYPE_IS_SET(type) && !ZEND_ARG_TYPE_IS_TENTATIVE(return_info)) {
2427+
if (ZEND_TYPE_IS_SET(type)) {
24282428
zend_op *opline;
24292429

24302430
/* `return ...;` is illegal in a void function (but `return;` isn't) */
@@ -6476,7 +6476,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall
64766476
arg_infos->type = zend_compile_typename(
64776477
return_type_ast, /* force_allow_null */ 0);
64786478
ZEND_TYPE_FULL_MASK(arg_infos->type) |= _ZEND_ARG_INFO_FLAGS(
6479-
(op_array->fn_flags & ZEND_ACC_RETURN_REFERENCE) != 0, /* is_variadic */ 0, 0);
6479+
(op_array->fn_flags & ZEND_ACC_RETURN_REFERENCE) != 0, /* is_variadic */ 0, /* is_tentative */ 0);
64806480
} else {
64816481
arg_infos->type = (zend_type) ZEND_TYPE_INIT_CODE(fallback_return_type, 0, 0);
64826482
}
@@ -6606,7 +6606,7 @@ void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32_t fall
66066606
zend_alloc_cache_slots(zend_type_get_num_classes(arg_info->type));
66076607
}
66086608

6609-
uint32_t arg_info_flags = _ZEND_ARG_INFO_FLAGS(is_ref, is_variadic, 0)
6609+
uint32_t arg_info_flags = _ZEND_ARG_INFO_FLAGS(is_ref, is_variadic, /* is_tentative */ 0)
66106610
| (visibility ? _ZEND_IS_PROMOTED_BIT : 0);
66116611
ZEND_TYPE_FULL_MASK(arg_info->type) |= arg_info_flags;
66126612
if (opcode == ZEND_RECV) {

‎Zend/zend_execute.c

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,19 +1294,18 @@ static ZEND_COLD void zend_verify_void_return_error(const zend_function *zf, con
12941294
static bool zend_verify_internal_return_type(zend_function *zf, zval *ret)
12951295
{
12961296
zend_internal_arg_info *ret_info = zf->internal_function.arg_info - 1;
1297-
if (!ZEND_ARG_TYPE_IS_TENTATIVE(ret_info)) {
1298-
if (ZEND_TYPE_FULL_MASK(ret_info->type) & MAY_BE_VOID) {
1299-
if (UNEXPECTED(Z_TYPE_P(ret) != IS_NULL)) {
1300-
zend_verify_void_return_error(zf, zend_zval_type_name(ret), "");
1301-
return 0;
1302-
}
1303-
return 1;
1304-
}
13051297

1306-
if (UNEXPECTED(!zend_check_type(&ret_info->type, ret, /* cache_slot */ NULL, NULL, 1, /* is_internal */ 1))) {
1307-
zend_verify_internal_return_error(zf, ret);
1298+
if (ZEND_TYPE_FULL_MASK(ret_info->type) & MAY_BE_VOID) {
1299+
if (UNEXPECTED(Z_TYPE_P(ret) != IS_NULL)) {
1300+
zend_verify_void_return_error(zf, zend_zval_type_name(ret), "");
13081301
return 0;
13091302
}
1303+
return 1;
1304+
}
1305+
1306+
if (UNEXPECTED(!zend_check_type(&ret_info->type, ret, /* cache_slot */ NULL, NULL, 1, /* is_internal */ 1))) {
1307+
zend_verify_internal_return_error(zf, ret);
1308+
return 0;
13101309
}
13111310

13121311
return 1;

‎Zend/zend_inheritance.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -869,10 +869,10 @@ static void ZEND_COLD emit_incompatible_method_error(
869869
ZSTR_VAL(child_prototype), ZSTR_VAL(parent_prototype), ZSTR_VAL(unresolved_class));
870870
} else if (status == INHERITANCE_WARNING) {
871871
zend_attribute *return_type_will_change_attribute = zend_get_attribute_str(
872-
child->common.attributes,
873-
"returntypewillchange",
874-
sizeof("returntypewillchange")-1
875-
);
872+
child->common.attributes,
873+
"returntypewillchange",
874+
sizeof("returntypewillchange")-1
875+
);
876876

877877
if (!return_type_will_change_attribute) {
878878
zend_error_at(E_DEPRECATED, NULL, func_lineno(child),

‎ext/opcache/ZendAccelerator.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ static void accel_copy_permanent_strings(zend_new_interned_string_func_t new_int
591591
if (Z_FUNC(p->val)->common.function_name) {
592592
Z_FUNC(p->val)->common.function_name = new_interned_string(Z_FUNC(p->val)->common.function_name);
593593
}
594-
if ((Z_FUNC(p->val)->common.arg_info) &&
594+
if (Z_FUNC(p->val)->common.arg_info &&
595595
(Z_FUNC(p->val)->common.fn_flags & (ZEND_ACC_HAS_RETURN_TYPE|ZEND_ACC_HAS_TYPE_HINTS))) {
596596
uint32_t i;
597597
uint32_t num_args = Z_FUNC(p->val)->common.num_args + 1;
@@ -3885,9 +3885,7 @@ static bool preload_needed_types_known(zend_class_entry *ce) {
38853885
zend_string *lcname;
38863886
ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->function_table, lcname, fptr) {
38873887
uint32_t i;
3888-
if ((fptr->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) &&
3889-
!ZEND_ARG_TYPE_IS_TENTATIVE(&fptr->common.arg_info[-1])
3890-
) {
3888+
if (fptr->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
38913889
if (!preload_is_type_known(ce, &fptr->common.arg_info[-1].type) &&
38923890
preload_is_method_maybe_override(ce, lcname)) {
38933891
return 0;

‎ext/opcache/jit/zend_jit.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3604,9 +3604,7 @@ static int zend_real_jit_func(zend_op_array *op_array, zend_script *script, cons
36043604
}
36053605
func_info = ZEND_FUNC_INFO(op_array);
36063606
func_info->call_map = zend_build_call_map(&CG(arena), func_info, op_array);
3607-
if ((op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) &&
3608-
!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) op_array)->common.arg_info[-1])
3609-
) {
3607+
if (op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
36103608
zend_init_func_return_info(op_array, script, &func_info->return_info);
36113609
}
36123610
}
@@ -3922,9 +3920,7 @@ ZEND_EXT_API int zend_jit_script(zend_script *script)
39223920
info = ZEND_FUNC_INFO(call_graph.op_arrays[i]);
39233921
if (info) {
39243922
info->call_map = zend_build_call_map(&CG(arena), info, call_graph.op_arrays[i]);
3925-
if ((call_graph.op_arrays[i]->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) &&
3926-
!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) call_graph.op_arrays[i])->common.arg_info[-1])
3927-
) {
3923+
if (call_graph.op_arrays[i]->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
39283924
zend_init_func_return_info(call_graph.op_arrays[i], script, &info->return_info);
39293925
}
39303926
}

‎ext/opcache/jit/zend_jit_trace.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -489,9 +489,7 @@ static zend_ssa *zend_jit_trace_build_ssa(const zend_op_array *op_array, zend_sc
489489
break;
490490
}
491491
jit_extension->func_info.call_map = zend_build_call_map(&CG(arena), &jit_extension->func_info, op_array);
492-
if (op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE &&
493-
!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) op_array)->common.arg_info[-1])
494-
) {
492+
if (op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
495493
zend_init_func_return_info(op_array, script, &jit_extension->func_info.return_info);
496494
}
497495
}

0 commit comments

Comments
 (0)