Skip to content

Add support for tentative return types of internal methods #6971

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.

Sign up for GitHub

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

Add support for tentative return types of internal methods #6971

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b59df1
Add support for tentative return types of internal methods
kocsismate May 6, 2021
b96379f
Address new review comments
kocsismate May 13, 2021
2dc1264
Address new review comments
kocsismate May 14, 2021
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
4 changes: 2 additions & 2 deletions Zend/Optimizer/zend_func_info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ ZEND_API uint32_t zend_get_func_info(
#endif

ret = zend_get_return_info_from_signature_only(
callee_func, /* script */ NULL, ce, ce_is_instanceof);
callee_func, /* script */ NULL, ce, ce_is_instanceof, /* use_tentative_return_info */ 1);

#if ZEND_DEBUG
if (internal_ret) {
Expand Down Expand Up @@ -884,7 +884,7 @@ ZEND_API uint32_t zend_get_func_info(
}
if (!ret) {
ret = zend_get_return_info_from_signature_only(
callee_func, /* TODO: script */ NULL, ce, ce_is_instanceof);
callee_func, /* TODO: script */ NULL, ce, ce_is_instanceof, /* use_tentative_return_info */ 0);
}
}
return ret;
Expand Down
9 changes: 5 additions & 4 deletions Zend/Optimizer/zend_inference.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4001,9 +4001,11 @@ static int is_recursive_tail_call(const zend_op_array *op_array,

uint32_t zend_get_return_info_from_signature_only(
const zend_function *func, const zend_script *script,
zend_class_entry **ce, bool *ce_is_instanceof) {
zend_class_entry **ce, bool *ce_is_instanceof, bool use_tentative_return_info) {
uint32_t type;
if (func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE && !ZEND_ARG_TYPE_IS_TENTATIVE(func->common.arg_info - 1)) {
if (func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE &&
(use_tentative_return_info || !ZEND_ARG_TYPE_IS_TENTATIVE(func->common.arg_info - 1))
) {
zend_arg_info *ret_info = func->common.arg_info - 1;
type = zend_fetch_arg_info_type(script, ret_info, ce);
*ce_is_instanceof = ce != NULL;
Expand All @@ -4026,12 +4028,11 @@ ZEND_API void zend_init_func_return_info(
const zend_op_array *op_array, const zend_script *script, zend_ssa_var_info *ret)
{
ZEND_ASSERT((op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE));
ZEND_ASSERT(!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) op_array)->common.arg_info[-1]));

zend_ssa_range tmp_range = {0, 0, 0, 0};
bool is_instanceof = false;
ret->type = zend_get_return_info_from_signature_only(
(zend_function *) op_array, script, &ret->ce, &is_instanceof);
(zend_function *) op_array, script, &ret->ce, &is_instanceof, /* use_tentative_return_info */ 1);
ret->is_instanceof = is_instanceof;
ret->range = tmp_range;
ret->has_range = 0;
Expand Down
2 changes: 1 addition & 1 deletion Zend/Optimizer/zend_inference.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ ZEND_API void zend_init_func_return_info(
const zend_op_array *op_array, const zend_script *script, zend_ssa_var_info *ret);
uint32_t zend_get_return_info_from_signature_only(
const zend_function *func, const zend_script *script,
zend_class_entry **ce, bool *ce_is_instanceof);
zend_class_entry **ce, bool *ce_is_instanceof, bool use_tentative_return_info);
void zend_func_return_info(const zend_op_array *op_array,
const zend_script *script,
int recursive,
Expand Down
4 changes: 1 addition & 3 deletions Zend/Optimizer/zend_optimizer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1446,9 +1446,7 @@ ZEND_API int zend_optimize_script(zend_script *script, zend_long optimization_le
func_info = ZEND_FUNC_INFO(call_graph.op_arrays[i]);
if (func_info) {
func_info->call_map = zend_build_call_map(&ctx.arena, func_info, call_graph.op_arrays[i]);
if ((call_graph.op_arrays[i]->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) &&
!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) call_graph.op_arrays[i])->common.arg_info[-1])
) {
if (call_graph.op_arrays[i]->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
zend_init_func_return_info(call_graph.op_arrays[i], script, &func_info->return_info);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...ance/internal_parent/internal_parent.phpt → ...unresolvable_inheritance_check_param.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--TEST--
Internal class as parent
Test unresolvable inheritance check due to unavailable parameter type when the parent has a tentative return type.
--FILE--
<?php

Expand Down
2 changes: 1 addition & 1 deletion ...nce/internal_parent/internal_parent2.phpt → ...nresolvable_inheritance_check_return.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--TEST--
Internal class as parent
Test unresolvable inheritance check due to unavailable return type when the parent has a tentative return type.
--FILE--
<?php

Expand Down
13 changes: 13 additions & 0 deletions Zend/tests/type_declarations/variance/return_type_will_change_class_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
--TEST--
Test that the ReturnTypeWillChange attribute cannot target classes
--FILE--
<?php

#[ReturnTypeWillChange]
class Foo
{
}

?>
--EXPECTF--
Fatal error: Attribute "ReturnTypeWillChange" cannot target class (allowed targets: method) in %s on line %d
11 changes: 11 additions & 0 deletions Zend/tests/type_declarations/variance/return_type_will_change_function_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
--TEST--
Test that the ReturnTypeWillChange attribute cannot target functions
--FILE--
<?php

#[ReturnTypeWillChange]
function foo() {}

?>
--EXPECTF--
Fatal error: Attribute "ReturnTypeWillChange" cannot target function (allowed targets: method) in %s on line %d
14 changes: 14 additions & 0 deletions Zend/tests/type_declarations/variance/return_type_will_change_property_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Test that the ReturnTypeWillChange attribute cannot be used with functions
--FILE--
<?php

class Foo
{
#[ReturnTypeWillChange]
public int $bar;
}

?>
--EXPECTF--
Fatal error: Attribute "ReturnTypeWillChange" cannot target property (allowed targets: method) in %s on line %d
10 changes: 1 addition & 9 deletions Zend/zend_attributes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,6 @@ void validate_attribute(zend_attribute *attr, uint32_t target, zend_class_entry
}
}

void validate_return_type_will_change_attribute(zend_attribute *attr, uint32_t target, zend_class_entry *scope)
{
if (target != ZEND_ATTRIBUTE_TARGET_METHOD) {
zend_error(E_COMPILE_ERROR, "Only methods can be marked with #[ReturnTypeWillChange]");
}
}

ZEND_METHOD(Attribute, __construct)
{
zend_long flags = ZEND_ATTRIBUTE_TARGET_ALL;
Expand Down Expand Up @@ -293,8 +286,7 @@ void zend_register_attribute_ce(void)
zend_declare_class_constant_long(zend_ce_attribute, ZEND_STRL("IS_REPEATABLE"), ZEND_ATTRIBUTE_IS_REPEATABLE);

zend_ce_return_type_will_change_attribute = register_class_ReturnTypeWillChange();
attr = zend_internal_attribute_register(zend_ce_return_type_will_change_attribute, ZEND_ATTRIBUTE_TARGET_METHOD);
attr->validator = validate_return_type_will_change_attribute;
zend_internal_attribute_register(zend_ce_return_type_will_change_attribute, ZEND_ATTRIBUTE_TARGET_METHOD);
}

void zend_attributes_shutdown(void)
Expand Down
8 changes: 4 additions & 4 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1305,7 +1305,7 @@ static void zend_mark_function_as_generator() /* {{{ */
"The \"yield\" expression can only be used inside a function");
}

if (CG(active_op_array)->fn_flags & ZEND_ACC_HAS_RETURN_TYPE && !ZEND_ARG_TYPE_IS_TENTATIVE(&(CG(active_op_array))->arg_info[-1])) {
if (CG(active_op_array)->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
zend_type return_type = CG(active_op_array)->arg_info[-1].type;
bool valid_type = (ZEND_TYPE_FULL_MASK(return_type) & (MAY_BE_ITERABLE | MAY_BE_OBJECT)) != 0;
if (!valid_type) {
Expand Down Expand Up @@ -2424,7 +2424,7 @@ static void zend_emit_return_type_check(
znode *expr, zend_arg_info *return_info, bool implicit) /* {{{ */
{
zend_type type = return_info->type;
if (ZEND_TYPE_IS_SET(type) && !ZEND_ARG_TYPE_IS_TENTATIVE(return_info)) {
if (ZEND_TYPE_IS_SET(type)) {
zend_op *opline;

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

uint32_t arg_info_flags = _ZEND_ARG_INFO_FLAGS(is_ref, is_variadic, 0)
uint32_t arg_info_flags = _ZEND_ARG_INFO_FLAGS(is_ref, is_variadic, /* is_tentative */ 0)
| (visibility ? _ZEND_IS_PROMOTED_BIT : 0);
ZEND_TYPE_FULL_MASK(arg_info->type) |= arg_info_flags;
if (opcode == ZEND_RECV) {
Expand Down
19 changes: 9 additions & 10 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1294,19 +1294,18 @@ static ZEND_COLD void zend_verify_void_return_error(const zend_function *zf, con
static bool zend_verify_internal_return_type(zend_function *zf, zval *ret)
{
zend_internal_arg_info *ret_info = zf->internal_function.arg_info - 1;
if (!ZEND_ARG_TYPE_IS_TENTATIVE(ret_info)) {
if (ZEND_TYPE_FULL_MASK(ret_info->type) & MAY_BE_VOID) {
if (UNEXPECTED(Z_TYPE_P(ret) != IS_NULL)) {
zend_verify_void_return_error(zf, zend_zval_type_name(ret), "");
return 0;
}
return 1;
}

if (UNEXPECTED(!zend_check_type(&ret_info->type, ret, /* cache_slot */ NULL, NULL, 1, /* is_internal */ 1))) {
zend_verify_internal_return_error(zf, ret);
if (ZEND_TYPE_FULL_MASK(ret_info->type) & MAY_BE_VOID) {
if (UNEXPECTED(Z_TYPE_P(ret) != IS_NULL)) {
zend_verify_void_return_error(zf, zend_zval_type_name(ret), "");
return 0;
}
return 1;
}

if (UNEXPECTED(!zend_check_type(&ret_info->type, ret, /* cache_slot */ NULL, NULL, 1, /* is_internal */ 1))) {
zend_verify_internal_return_error(zf, ret);
return 0;
}

return 1;
Expand Down
8 changes: 4 additions & 4 deletions Zend/zend_inheritance.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,10 +869,10 @@ static void ZEND_COLD emit_incompatible_method_error(
ZSTR_VAL(child_prototype), ZSTR_VAL(parent_prototype), ZSTR_VAL(unresolved_class));
} else if (status == INHERITANCE_WARNING) {
zend_attribute *return_type_will_change_attribute = zend_get_attribute_str(
child->common.attributes,
"returntypewillchange",
sizeof("returntypewillchange")-1
);
child->common.attributes,
"returntypewillchange",
sizeof("returntypewillchange")-1
);

if (!return_type_will_change_attribute) {
zend_error_at(E_DEPRECATED, NULL, func_lineno(child),
Expand Down
6 changes: 2 additions & 4 deletions ext/opcache/ZendAccelerator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ static void accel_copy_permanent_strings(zend_new_interned_string_func_t new_int
if (Z_FUNC(p->val)->common.function_name) {
Z_FUNC(p->val)->common.function_name = new_interned_string(Z_FUNC(p->val)->common.function_name);
}
if ((Z_FUNC(p->val)->common.arg_info) &&
if (Z_FUNC(p->val)->common.arg_info &&
(Z_FUNC(p->val)->common.fn_flags & (ZEND_ACC_HAS_RETURN_TYPE|ZEND_ACC_HAS_TYPE_HINTS))) {
uint32_t i;
uint32_t num_args = Z_FUNC(p->val)->common.num_args + 1;
Expand Down Expand Up @@ -3885,9 +3885,7 @@ static bool preload_needed_types_known(zend_class_entry *ce) {
zend_string *lcname;
ZEND_HASH_FOREACH_STR_KEY_PTR(&ce->function_table, lcname, fptr) {
uint32_t i;
if ((fptr->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) &&
!ZEND_ARG_TYPE_IS_TENTATIVE(&fptr->common.arg_info[-1])
) {
if (fptr->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
if (!preload_is_type_known(ce, &fptr->common.arg_info[-1].type) &&
preload_is_method_maybe_override(ce, lcname)) {
return 0;
Expand Down
8 changes: 2 additions & 6 deletions ext/opcache/jit/zend_jit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3604,9 +3604,7 @@ static int zend_real_jit_func(zend_op_array *op_array, zend_script *script, cons
}
func_info = ZEND_FUNC_INFO(op_array);
func_info->call_map = zend_build_call_map(&CG(arena), func_info, op_array);
if ((op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) &&
!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) op_array)->common.arg_info[-1])
) {
if (op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
zend_init_func_return_info(op_array, script, &func_info->return_info);
}
}
Expand Down Expand Up @@ -3922,9 +3920,7 @@ ZEND_EXT_API int zend_jit_script(zend_script *script)
info = ZEND_FUNC_INFO(call_graph.op_arrays[i]);
if (info) {
info->call_map = zend_build_call_map(&CG(arena), info, call_graph.op_arrays[i]);
if ((call_graph.op_arrays[i]->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) &&
!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) call_graph.op_arrays[i])->common.arg_info[-1])
) {
if (call_graph.op_arrays[i]->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
zend_init_func_return_info(call_graph.op_arrays[i], script, &info->return_info);
}
}
Expand Down
4 changes: 1 addition & 3 deletions ext/opcache/jit/zend_jit_trace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,9 +489,7 @@ static zend_ssa *zend_jit_trace_build_ssa(const zend_op_array *op_array, zend_sc
break;
}
jit_extension->func_info.call_map = zend_build_call_map(&CG(arena), &jit_extension->func_info, op_array);
if (op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE &&
!ZEND_ARG_TYPE_IS_TENTATIVE(&((zend_function *) op_array)->common.arg_info[-1])
) {
if (op_array->fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
zend_init_func_return_info(op_array, script, &jit_extension->func_info.return_info);
}
}
Expand Down