Skip to content

Add support for property initialization during cloning #6538

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

Closed
wants to merge 1 commit into from
Closed
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
16 changes: 16 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_error1.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
--TEST--
Test that the property initializer list is required when "with" is given
--FILE--
<?php

class Foo
{
public function withProperties()
{
return clone $this with;
}
}

?>
--EXPECTF--
Parse error: syntax error, unexpected token ";", expecting "{" in %s on line %d
16 changes: 16 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_error2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
--TEST--
Test that the property initializer list cannot contain integer keys
--FILE--
<?php

class Foo
{
public function withProperties()
{
return clone $this with {1: "value"};
}
}

?>
--EXPECTF--
Parse error: syntax error, unexpected integer "1" in %s on line %d
18 changes: 18 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_error3.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
--TEST--
Test that the property initializer list can only contain literals
--FILE--
<?php

class Foo
{
public function withProperties()
{
$property = "string";

return clone $this with {$property: "value"};
}
}

?>
--EXPECTF--
Parse error: syntax error, unexpected variable "$property" in %s on line %d
21 changes: 21 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_error4.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
Test that the clone property initializer respects visibility
--FILE--
<?php

class Foo
{
private $bar;
}

$foo = new Foo();

try {
$foo = clone $foo with {bar: 1};
} catch (Error $exception) {
echo $exception->getMessage() . "\n";
}

?>
--EXPECT--
Cannot access private property Foo::$bar
19 changes: 19 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_error5.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
--TEST--
Test that the "clone with" doesn't support expressions as property names
--FILE--
<?php

class Foo
{
public $bar;
}

$property = "bar";

$foo = new Foo();
$foo = clone $foo with {$property: 1};
var_dump($foo);

?>
--EXPECTF--
Parse error: syntax error, unexpected variable "$property" in %s on line %d
38 changes: 38 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_success1.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
--TEST--
Test that declared properties can be initialized during cloning
--FILE--
<?php

class Foo
{
public int $property1;
public string $property2;

public function withProperties()
{
return clone $this with {
property1: 1,
property2: "foo",
};
}
}

$foo = new Foo();
var_dump($foo);
$bar = $foo->withProperties();
var_dump($bar);

?>
--EXPECT--
object(Foo)#1 (0) {
["property1"]=>
uninitialized(int)
["property2"]=>
uninitialized(string)
}
object(Foo)#2 (2) {
["property1"]=>
int(1)
["property2"]=>
string(3) "foo"
}
31 changes: 31 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_success2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
--TEST--
Test that dynamic properties can be initialized during cloning
--FILE--
<?php

class Foo
{
public function withProperties()
{
return clone $this with {
property1: 1,
property2: "foo",
Copy link
Contributor

@TysonAndre TysonAndre Dec 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could add a test of this with function calls, e.g.

  • If a previous property expression throws, subsequent ones are not evaluated?
  • If an expression throws, type checks are not performed (e.g. clone $this with { strTypedProperty: throw new Exception("test") })
  • temporary values such as strtolower("ABC") are freed if there's a type error assigning to a typed property

EDIT: Oh. So far, this is only implemented for constant operands in Zend/zend_vm_def.h

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the ideas, I'll definitely add such tests!

};
}
}

$foo = new Foo();
var_dump($foo);
$bar = $foo->withProperties();
var_dump($bar);

?>
--EXPECT--
object(Foo)#1 (0) {
}
object(Foo)#2 (2) {
["property1"]=>
int(1)
["property2"]=>
string(3) "foo"
}
24 changes: 24 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_success3.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
--TEST--
Test that the property initializer list can be empty
--FILE--
<?php

class Foo
{
public function withProperties()
{
return clone $this with {};
}
}

$foo = new Foo();
var_dump($foo);
$bar = $foo->withProperties();
var_dump($bar);

?>
--EXPECT--
object(Foo)#1 (0) {
}
object(Foo)#2 (0) {
}
27 changes: 27 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_success4.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
Test that the "clone with" is still chainable
--FILE--
<?php

class Foo
{
public $bar = 1;

public function withProperties()
{
return (clone $this with {})->bar;
}
}

$foo = new Foo();
var_dump($foo);
$bar = $foo->withProperties();
var_dump($bar);

?>
--EXPECT--
object(Foo)#1 (1) {
["bar"]=>
int(1)
}
int(1)
21 changes: 21 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_success5.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
--TEST--
Test that the "clone with" works with dynamic properties
--FILE--
<?php

class Foo
{
}

$foo = new Foo();
$foo = clone $foo with {bar: 1, baz: ""};
var_dump($foo);

?>
--EXPECT--
object(Foo)#2 (2) {
["bar"]=>
int(1)
["baz"]=>
string(0) ""
}
24 changes: 24 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_success6.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
--TEST--
Test that the "clone with" works with expressions as property values
--FILE--
<?php

class Foo
{
public $bar;
public $baz;
}

$foo = new Foo();
$foo = clone $foo with {bar: new stdClass(), baz: strpos("abc", "b")};
var_dump($foo);

?>
--EXPECT--
object(Foo)#2 (2) {
["bar"]=>
object(stdClass)#3 (0) {
}
["baz"]=>
int(1)
}
20 changes: 20 additions & 0 deletions Zend/tests/clone_initializer/clone_initializer_success7.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
Test that the "clone with" works with expressions as clone operand
--FILE--
<?php

class Foo
{
public static function create()
{
return new Foo();
}
}

$foo = clone Foo::create() with {};
var_dump($foo);

?>
--EXPECT--
object(Foo)#2 (0) {
}
6 changes: 6 additions & 0 deletions Zend/zend_ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -1602,6 +1602,11 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
case ZEND_AST_MATCH_ARM_LIST:
zend_ast_export_list(str, (zend_ast_list*)ast, 0, 0, indent);
break;
case ZEND_AST_PROPERTY_INITIALIZER_LIST:
smart_str_appends(str, " {");
zend_ast_export_list(str, (zend_ast_list*)ast, 0, 0, indent);
smart_str_appends(str, " }");
break;
case ZEND_AST_CLOSURE_USES:
smart_str_appends(str, " use(");
zend_ast_export_var_list(str, (zend_ast_list*)ast, indent);
Expand Down Expand Up @@ -2074,6 +2079,7 @@ static ZEND_COLD void zend_ast_export_ex(smart_str *str, zend_ast *ast, int prio
}
break;
case ZEND_AST_NAMED_ARG:
case ZEND_AST_INITIALIZER_EXPR:
smart_str_append(str, zend_ast_get_str(ast->child[0]));
smart_str_appends(str, ": ");
ast = ast->child[1];
Expand Down
4 changes: 3 additions & 1 deletion Zend/zend_ast.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ enum _zend_ast_kind {
ZEND_AST_ATTRIBUTE_LIST,
ZEND_AST_ATTRIBUTE_GROUP,
ZEND_AST_MATCH_ARM_LIST,
ZEND_AST_PROPERTY_INITIALIZER_LIST,

/* 0 child nodes */
ZEND_AST_MAGIC_CONST = 0 << ZEND_AST_NUM_CHILDREN_SHIFT,
Expand All @@ -82,7 +83,6 @@ enum _zend_ast_kind {
ZEND_AST_ISSET,
ZEND_AST_SILENCE,
ZEND_AST_SHELL_EXEC,
ZEND_AST_CLONE,
ZEND_AST_EXIT,
ZEND_AST_PRINT,
ZEND_AST_INCLUDE_OR_EVAL,
Expand Down Expand Up @@ -113,6 +113,7 @@ enum _zend_ast_kind {
ZEND_AST_STATIC_PROP,
ZEND_AST_CALL,
ZEND_AST_CLASS_CONST,
ZEND_AST_CLONE,
ZEND_AST_ASSIGN,
ZEND_AST_ASSIGN_REF,
ZEND_AST_ASSIGN_OP,
Expand Down Expand Up @@ -147,6 +148,7 @@ enum _zend_ast_kind {
ZEND_AST_MATCH,
ZEND_AST_MATCH_ARM,
ZEND_AST_NAMED_ARG,
ZEND_AST_INITIALIZER_EXPR,

/* 3 child nodes */
ZEND_AST_METHOD_CALL = 3 << ZEND_AST_NUM_CHILDREN_SHIFT,
Expand Down
32 changes: 31 additions & 1 deletion Zend/zend_compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,7 @@ void zend_do_free(znode *op1) /* {{{ */
case ZEND_ASSIGN_DIM_OP:
case ZEND_ASSIGN_OBJ_OP:
case ZEND_ASSIGN_STATIC_PROP_OP:
case ZEND_INIT_OBJ:
case ZEND_PRE_INC_STATIC_PROP:
case ZEND_PRE_DEC_STATIC_PROP:
case ZEND_PRE_INC_OBJ:
Expand Down Expand Up @@ -4538,11 +4539,40 @@ void zend_compile_new(znode *result, zend_ast *ast) /* {{{ */
void zend_compile_clone(znode *result, zend_ast *ast) /* {{{ */
{
zend_ast *obj_ast = ast->child[0];
zend_ast *initializer_ast = ast->child[1];

znode obj_node;
zend_compile_expr(&obj_node, obj_ast);
zend_op *opline = zend_emit_op(result, ZEND_CLONE, &obj_node, NULL);

zend_emit_op_tmp(result, ZEND_CLONE, &obj_node, NULL);
if (initializer_ast) {
zend_ast_list *list = zend_ast_get_list(initializer_ast);
uint32_t i;

for (i = 0; i < list->children; i++) {
zend_ast *property_init_expr_ast = list->child[i];
ZEND_ASSERT(property_init_expr_ast->kind == ZEND_AST_INITIALIZER_EXPR);
zend_string *property_name = zval_make_interned_string(zend_ast_get_zval(property_init_expr_ast->child[0]));

zend_ast *property_value_ast = property_init_expr_ast->child[1];
znode property_value_node;
zend_compile_expr(&property_value_node, property_value_ast);

znode property_object_node;
property_object_node.op_type = IS_VAR;
GET_NODE(&property_object_node, opline->result);

znode property_name_node;
property_name_node.op_type = IS_CONST;
ZVAL_STR_COPY(&property_name_node.u.constant, property_name);
opline = zend_emit_op(result, ZEND_INIT_OBJ, &property_object_node, &property_name_node);
opline->result.var = get_temporary_variable();
opline->extended_value = zend_alloc_cache_slots(3);

zend_emit_op_data(&property_value_node);
GET_NODE(result, opline->result);
}
}
}
/* }}} */

Expand Down
Loading