Skip to content

Fix param with hooks but no visibility not treated as cpp #15442

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

Merged
merged 1 commit into from
Aug 19, 2024
Merged
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
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ PHP NEWS
- Core:
. Fixed bug GH-15408 (MSan false-positve on zend_max_execution_timer).
(zeriyoshi)
. Fixed bug GH-15438 (Hooks on constructor promoted properties without
visibility are ignored). (ilutov)

15 Aug 2024, PHP 8.4.0beta3

Expand Down
20 changes: 20 additions & 0 deletions Zend/tests/property_hooks/gh15438_1.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
GH-15438: Promoted properties with hooks but no visibility
--FILE--
<?php

class C {
public function __construct(
$prop { set => $value * 2; }
) {}
}

$c = new C(42);
var_dump($c);

?>
--EXPECTF--
object(C)#%d (1) {
["prop"]=>
int(84)
}
20 changes: 20 additions & 0 deletions Zend/tests/property_hooks/gh15438_2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
GH-15438: Untyped promoted, hooked properties with no default value default to null
--FILE--
<?php

class C {
public function __construct(
public $prop { set => $value * 2; }
) {}
}

$c = new ReflectionClass(C::class)->newInstanceWithoutConstructor();
var_dump($c);

?>
--EXPECTF--
object(C)#%d (1) {
["prop"]=>
NULL
}
21 changes: 12 additions & 9 deletions Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -7570,6 +7570,7 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32
bool is_ref = (param_ast->attr & ZEND_PARAM_REF) != 0;
bool is_variadic = (param_ast->attr & ZEND_PARAM_VARIADIC) != 0;
uint32_t property_flags = param_ast->attr & (ZEND_ACC_PPP_MASK | ZEND_ACC_READONLY);
bool is_promoted = property_flags || hooks_ast;

znode var_node, default_node;
uint8_t opcode;
Expand Down Expand Up @@ -7626,14 +7627,14 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32
if (attributes_ast) {
zend_compile_attributes(
&op_array->attributes, attributes_ast, i + 1, ZEND_ATTRIBUTE_TARGET_PARAMETER,
property_flags ? ZEND_ATTRIBUTE_TARGET_PROPERTY : 0
is_promoted ? ZEND_ATTRIBUTE_TARGET_PROPERTY : 0
);
}

bool forced_allow_nullable = false;
if (type_ast) {
uint32_t default_type = *default_ast_ptr ? Z_TYPE(default_node.u.constant) : IS_UNDEF;
bool force_nullable = default_type == IS_NULL && !property_flags;
bool force_nullable = default_type == IS_NULL && !is_promoted;

op_array->fn_flags |= ZEND_ACC_HAS_TYPE_HINTS;
arg_info->type = zend_compile_typename_ex(type_ast, force_nullable, &forced_allow_nullable);
Expand Down Expand Up @@ -7696,14 +7697,14 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32
}

uint32_t arg_info_flags = _ZEND_ARG_INFO_FLAGS(is_ref, is_variadic, /* is_tentative */ 0)
| (property_flags ? _ZEND_IS_PROMOTED_BIT : 0);
| (is_promoted ? _ZEND_IS_PROMOTED_BIT : 0);
ZEND_TYPE_FULL_MASK(arg_info->type) |= arg_info_flags;
if (opcode == ZEND_RECV) {
opline->op2.num = type_ast ?
ZEND_TYPE_FULL_MASK(arg_info->type) : MAY_BE_ANY;
}

if (property_flags) {
if (is_promoted) {
zend_op_array *op_array = CG(active_op_array);
zend_class_entry *scope = op_array->scope;

Expand Down Expand Up @@ -7790,9 +7791,11 @@ static void zend_compile_params(zend_ast *ast, zend_ast *return_type_ast, uint32

for (i = 0; i < list->children; i++) {
zend_ast *param_ast = list->child[i];
zend_ast *hooks_ast = param_ast->child[5];
bool is_ref = (param_ast->attr & ZEND_PARAM_REF) != 0;
uint32_t flags = param_ast->attr & (ZEND_ACC_PPP_MASK | ZEND_ACC_READONLY);
if (!flags) {
bool is_promoted = flags || hooks_ast;
if (!is_promoted) {
continue;
}

Expand Down Expand Up @@ -8543,6 +8546,10 @@ static void zend_compile_property_hooks(
/* Will be removed again, in case of Iterator or IteratorAggregate. */
ce->get_iterator = zend_hooked_object_get_iterator;
}

if (!prop_info->ce->parent_name) {
zend_verify_hooked_property(ce, prop_info, prop_name);
}
}

static void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t flags, zend_ast *attr_ast) /* {{{ */
Expand Down Expand Up @@ -8679,10 +8686,6 @@ static void zend_compile_prop_decl(zend_ast *ast, zend_ast *type_ast, uint32_t f

if (hooks_ast) {
zend_compile_property_hooks(info, name, type_ast, zend_ast_get_list(hooks_ast));

if (!ce->parent_name) {
zend_verify_hooked_property(ce, info, name);
}
}

if (attr_ast) {
Expand Down
Loading