Skip to content

[RFC] Short list syntax #1849

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 1 commit into from
Closed

[RFC] Short list syntax #1849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 1 deletion Zend/tests/assert/expect_015.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ Warning: assert(): assert(0 && ($a = function () {
$x = $a ? $b : $c;
$x = $a ?: $c;
$x = $a ?? $b;
list($a, $b, $c) = [1, 2 => 'x', 'z' => 'c'];
[$a, $b, $c] = [1, 2 => 'x', 'z' => 'c'];
@foo();
$y = clone $x;
yield 1 => 2;
Expand Down
6 changes: 6 additions & 0 deletions Zend/tests/list_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,16 @@

list($a, list($b)) = array(new stdclass, array(new stdclass));
var_dump($a, $b);
[$a, [$b]] = array(new stdclass, array(new stdclass));
var_dump($a, $b);
Copy link
Member

Choose a reason for hiding this comment

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

Missing test for error on [42] = ... and similar.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've not added many [] tests, it's using the exactly same syntax (array_pair_list) in parser and the differences are abstracted away inside AST.
If there would be a difference, I'd add tests, but here they would be just outright redundant.

Copy link
Member

Choose a reason for hiding this comment

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

You've added eight or so new error conditions and only one of them is currently covered.

I'm asking about [42] = ... specifically because it's something the list() implementation would not necessarily check for (as it already had parser level checks for that).

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, added list_008.phpt

Copy link
Member

Choose a reason for hiding this comment

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

Another case of interest may be [&$a] = ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, yeah, list() still doesn't support assignment by-reference. I'd wanted to fix that.


?>
--EXPECT--
object(stdClass)#1 (0) {
}
object(stdClass)#2 (0) {
}
object(stdClass)#3 (0) {
}
object(stdClass)#4 (0) {
}
10 changes: 10 additions & 0 deletions Zend/tests/list_008.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
--TEST--
Assignment to invalid list() value
--FILE--
<?php

[42] = [1];

?>
--EXPECTF--
Fatal error: Assignments can only happen to writable values in %s on line %d
14 changes: 14 additions & 0 deletions Zend/tests/list_009.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
list with by-reference assignment should fail
--FILE--
<?php

$a = [1];
[&$foo] = $a;
$foo = 2;

var_dump($a);

?>
--EXPECTF--
Fatal error: [] and list() assignments cannot be by reference in %s on line %d
2 changes: 1 addition & 1 deletion Zend/tests/list_mixed_keyed_unkeyed.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ list($zero, 1 => $one, "foo" => $foo) = $contrivedKeyedAndUnkeyedArrayExample;

?>
--EXPECTF--
Parse error: syntax error, unexpected %s in %s on line %d
Fatal error: Cannot mix keyed and unkeyed array entries in assignments in %s on line %d
5 changes: 0 additions & 5 deletions Zend/zend_ast.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1099,11 +1099,6 @@ static void zend_ast_export_ex(smart_str *str, zend_ast *ast, int priority, int
simple_list:
zend_ast_export_list(str, (zend_ast_list*)ast, 1, 20, indent);
break;
case ZEND_AST_LIST:
smart_str_appends(str, "list(");
zend_ast_export_list(str, (zend_ast_list*)ast, 1, 20, indent);
smart_str_appendc(str, ')');
break;
case ZEND_AST_ARRAY:
smart_str_appendc(str, '[');
zend_ast_export_list(str, (zend_ast_list*)ast, 1, 20, indent);
Expand Down
1 change: 0 additions & 1 deletion Zend/zend_ast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ enum _zend_ast_kind {

/* list nodes */
ZEND_AST_ARG_LIST = 1 << ZEND_AST_IS_LIST_SHIFT,
ZEND_AST_LIST,
ZEND_AST_ARRAY,
ZEND_AST_ENCAPS_LIST,
ZEND_AST_EXPR_LIST,
Expand Down
99 changes: 77 additions & 22 deletions Zend/zend_compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2577,13 +2577,13 @@ static void zend_separate_if_call_and_write(znode *node, zend_ast *ast, uint32_t

void zend_delayed_compile_var(znode *result, zend_ast *ast, uint32_t type);
void zend_compile_assign(znode *result, zend_ast *ast);
static void zend_compile_list_assign(znode *result, zend_ast *ast, znode *expr_node);
static void zend_compile_list_assign(znode *result, zend_ast *ast, znode *expr_node, zend_bool old_style);

static inline void zend_emit_assign_znode(zend_ast *var_ast, znode *value_node) /* {{{ */
{
znode dummy_node;
if (var_ast->kind == ZEND_AST_LIST) {
zend_compile_list_assign(&dummy_node, var_ast, value_node);
if (var_ast->kind == ZEND_AST_ARRAY) {
zend_compile_list_assign(&dummy_node, var_ast, value_node, var_ast->attr);
} else {
zend_ast *assign_ast = zend_ast_create(ZEND_AST_ASSIGN, var_ast,
zend_ast_create_znode(value_node));
Expand Down Expand Up @@ -2727,18 +2727,35 @@ void zend_compile_static_prop(znode *result, zend_ast *ast, uint32_t type, int d
}
/* }}} */

static void zend_compile_unkeyed_list_assign(zend_ast_list *list, znode *expr_node) /* {{{ */
static void zend_verify_list_assign_target(zend_ast *var_ast, zend_bool old_style) /* {{{ */ {
if (var_ast->kind == ZEND_AST_ARRAY) {
if (old_style != var_ast->attr) {
zend_error(E_COMPILE_ERROR, "Cannot mix [] and list()");
}
} else if (!zend_can_write_to_variable(var_ast)) {
zend_error(E_COMPILE_ERROR, "Assignments can only happen to writable values");
}
}
/* }}} */

static void zend_compile_unkeyed_list_assign(zend_ast_list *list, znode *expr_node, zend_bool old_style) /* {{{ */
{
uint32_t i;
zend_bool has_elems = 0;

for (i = 0; i < list->children; ++i) {
zend_ast *var_ast = list->child[i];
zend_ast *elem_ast = list->child[i];
zend_ast *var_ast;
znode fetch_result, dim_node;

if (var_ast == NULL) {
if (elem_ast == NULL) {
continue;
}
if (elem_ast->attr) {
zend_error(E_COMPILE_ERROR, "[] and list() assignments cannot be by reference");
}

var_ast = elem_ast->child[0];
has_elems = 1;

dim_node.op_type = IS_CONST;
Expand All @@ -2748,6 +2765,12 @@ static void zend_compile_unkeyed_list_assign(zend_ast_list *list, znode *expr_no
Z_TRY_ADDREF(expr_node->u.constant);
}

if (elem_ast->child[1] != NULL) {
zend_error(E_COMPILE_ERROR, "Cannot mix keyed and unkeyed array entries in assignments");
}

zend_verify_list_assign_target(var_ast, old_style);

zend_emit_op(&fetch_result, ZEND_FETCH_LIST, expr_node, &dim_node);
zend_emit_assign_znode(var_ast, &fetch_result);
}
Expand All @@ -2758,7 +2781,7 @@ static void zend_compile_unkeyed_list_assign(zend_ast_list *list, znode *expr_no
}
/* }}} */

static void zend_compile_keyed_list_assign(zend_ast_list *list, znode *expr_node) /* {{{ */
static void zend_compile_keyed_list_assign(zend_ast_list *list, znode *expr_node, zend_bool old_style) /* {{{ */
{
uint32_t i;

Expand All @@ -2768,26 +2791,40 @@ static void zend_compile_keyed_list_assign(zend_ast_list *list, znode *expr_node
zend_ast *key_ast = pair_ast->child[1];
znode fetch_result, dim_node;

if (pair_ast->attr) {
zend_error(E_COMPILE_ERROR, "[] and list() assignments cannot be by reference");
}

zend_compile_expr(&dim_node, key_ast);

if (expr_node->op_type == IS_CONST) {
Z_TRY_ADDREF(expr_node->u.constant);
}

if (var_ast == NULL) {
zend_error(E_COMPILE_ERROR, "Cannot use empty array entries in keyed array");
}

if (key_ast == NULL) {
zend_error(E_COMPILE_ERROR, "Cannot mix keyed and unkeyed array entries in assignments");
}

zend_verify_list_assign_target(var_ast, old_style);

zend_emit_op(&fetch_result, ZEND_FETCH_LIST, expr_node, &dim_node);
zend_emit_assign_znode(var_ast, &fetch_result);
}
}
/* }}} */

static void zend_compile_list_assign(znode *result, zend_ast *ast, znode *expr_node) /* {{{ */
static void zend_compile_list_assign(znode *result, zend_ast *ast, znode *expr_node, zend_bool old_style) /* {{{ */
{
zend_ast_list *list = zend_ast_get_list(ast);

if (list->children > 0 && list->child[0] != NULL && list->child[0]->kind == ZEND_AST_ARRAY_ELEM) {
zend_compile_keyed_list_assign(list, expr_node);
if (list->children > 0 && list->child[0] != NULL && list->child[0]->child[1] != NULL /* has key */) {
zend_compile_keyed_list_assign(list, expr_node, old_style);
} else {
zend_compile_unkeyed_list_assign(list, expr_node);
zend_compile_unkeyed_list_assign(list, expr_node, old_style);
}

*result = *expr_node;
Expand Down Expand Up @@ -2837,13 +2874,16 @@ zend_bool zend_list_has_assign_to(zend_ast *list_ast, zend_string *name) /* {{{
zend_ast_list *list = zend_ast_get_list(list_ast);
uint32_t i;
for (i = 0; i < list->children; i++) {
zend_ast *var_ast = list->child[i];
if (!var_ast) {
zend_ast *elem_ast = list->child[i];
zend_ast *var_ast;

if (!elem_ast) {
continue;
}
var_ast = elem_ast->child[0];

/* Recursively check nested list()s */
if (var_ast->kind == ZEND_AST_LIST && zend_list_has_assign_to(var_ast, name)) {
if (var_ast->kind == ZEND_AST_ARRAY && zend_list_has_assign_to(var_ast, name)) {
return 1;
}

Expand Down Expand Up @@ -2925,15 +2965,15 @@ void zend_compile_assign(znode *result, zend_ast *ast) /* {{{ */

zend_emit_op_data(&expr_node);
return;
case ZEND_AST_LIST:
case ZEND_AST_ARRAY:
if (zend_list_has_assign_to_self(var_ast, expr_ast)) {
/* list($a, $b) = $a should evaluate the right $a first */
zend_compile_simple_var_no_cv(&expr_node, expr_ast, BP_VAR_R, 0);
} else {
zend_compile_expr(&expr_node, expr_ast);
}

zend_compile_list_assign(result, var_ast, &expr_node);
zend_compile_list_assign(result, var_ast, &expr_node, var_ast->attr);
return;
EMPTY_SWITCH_DEFAULT_CASE();
}
Expand Down Expand Up @@ -4340,7 +4380,7 @@ void zend_compile_foreach(zend_ast *ast) /* {{{ */
if (key_ast->kind == ZEND_AST_REF) {
zend_error_noreturn(E_COMPILE_ERROR, "Key element cannot be a reference");
}
if (key_ast->kind == ZEND_AST_LIST) {
if (key_ast->kind == ZEND_AST_ARRAY) {
zend_error_noreturn(E_COMPILE_ERROR, "Cannot use list as key element");
}
}
Expand Down Expand Up @@ -6357,14 +6397,22 @@ static zend_bool zend_try_ct_eval_array(zval *result, zend_ast *ast) /* {{{ */
uint32_t i;
zend_bool is_constant = 1;

if (ast->attr) {
zend_error(E_COMPILE_ERROR, "Cannot use list() as standalone expression");
}

/* First ensure that *all* child nodes are constant and by-val */
for (i = 0; i < list->children; ++i) {
zend_ast *elem_ast = list->child[i];
zend_bool by_ref = elem_ast->attr;

if (elem_ast == NULL) {
zend_error(E_COMPILE_ERROR, "Cannot use empty array elements in arrays");
}

zend_eval_const_expr(&elem_ast->child[0]);
zend_eval_const_expr(&elem_ast->child[1]);

if (by_ref || elem_ast->child[0]->kind != ZEND_AST_ZVAL
if (elem_ast->attr /* by_ref */ || elem_ast->child[0]->kind != ZEND_AST_ZVAL
|| (elem_ast->child[1] && elem_ast->child[1]->kind != ZEND_AST_ZVAL)
) {
is_constant = 0;
Expand Down Expand Up @@ -6982,9 +7030,16 @@ void zend_compile_array(znode *result, zend_ast *ast) /* {{{ */

for (i = 0; i < list->children; ++i) {
zend_ast *elem_ast = list->child[i];
zend_ast *value_ast = elem_ast->child[0];
zend_ast *key_ast = elem_ast->child[1];
zend_bool by_ref = elem_ast->attr;
zend_ast *value_ast, *key_ast;
zend_bool by_ref;

if (elem_ast == NULL) {
zend_error(E_COMPILE_ERROR, "Cannot use empty array elements in arrays");
}

value_ast = elem_ast->child[0];
key_ast = elem_ast->child[1];
by_ref = elem_ast->attr;

znode value_node, key_node, *key_node_ptr = NULL;

Expand Down
Loading