Skip to content

Commit 1057cce

Browse files
committed
Always memoize calls in lhs of coalesce assignment
We don't want to invoke calls twice, even if they are considered "variables", i.e. might be writable if returning a reference. Function calls behave the same in all BP contexts so they don't need to be invoked twice. The singular exception to this is nullsafe coalesce in isset/empty, because it needs to return false/true respectively when short-circuited. However, since nullsafe calls are not allwed in write context we may ignore this problem. Closes GH-11592
1 parent c0ce3e7 commit 1057cce

File tree

2 files changed

+77
-16
lines changed

2 files changed

+77
-16
lines changed

Zend/tests/assign_coalesce_008.phpt

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
--TEST--
2+
Assign coalesce: All calls should be memoized
3+
--FILE--
4+
<?php
5+
class Foo {
6+
public $prop;
7+
8+
public function foo() {
9+
echo __METHOD__, "\n";
10+
return $this;
11+
}
12+
13+
public function bar() {
14+
echo __METHOD__, "\n";
15+
return 'prop';
16+
}
17+
18+
public function __isset($name) {
19+
echo __METHOD__, "\n";
20+
return false;
21+
}
22+
23+
public function __set($name, $value) {
24+
echo __METHOD__, "\n";
25+
var_dump($value);
26+
}
27+
}
28+
29+
function &foo() {
30+
global $foo;
31+
echo __FUNCTION__, "\n";
32+
return $foo;
33+
}
34+
function bar() {
35+
echo __FUNCTION__, "\n";
36+
}
37+
38+
foo(bar())['bar'] ??= 42;
39+
var_dump($foo);
40+
41+
$foo = new Foo();
42+
$foo->foo()->foo()->{$foo->bar()} ??= 42;
43+
var_dump($foo);
44+
$foo->foo()->baz ??= 42;
45+
46+
?>
47+
--EXPECT--
48+
bar
49+
foo
50+
array(1) {
51+
["bar"]=>
52+
int(42)
53+
}
54+
Foo::foo
55+
Foo::foo
56+
Foo::bar
57+
object(Foo)#1 (1) {
58+
["prop"]=>
59+
int(42)
60+
}
61+
Foo::foo
62+
Foo::__isset
63+
Foo::__set
64+
int(42)

Zend/zend_compile.c

+13-16
Original file line numberDiff line numberDiff line change
@@ -4589,14 +4589,7 @@ static void zend_compile_call(znode *result, zend_ast *ast, uint32_t type) /* {{
45894589
if (runtime_resolution) {
45904590
if (zend_string_equals_literal_ci(zend_ast_get_str(name_ast), "assert")
45914591
&& !is_callable_convert) {
4592-
if (CG(memoize_mode) == ZEND_MEMOIZE_NONE) {
4593-
zend_compile_assert(result, zend_ast_get_list(args_ast), Z_STR(name_node.u.constant), NULL, ast->lineno);
4594-
} else {
4595-
/* We want to always memoize assert calls, even if they are positioned in
4596-
* write-context. This prevents memoizing their arguments that might not be
4597-
* evaluated if assertions are disabled, using a TMPVAR that wasn't initialized. */
4598-
zend_compile_memoized_expr(result, ast);
4599-
}
4592+
zend_compile_assert(result, zend_ast_get_list(args_ast), Z_STR(name_node.u.constant), NULL, ast->lineno);
46004593
} else {
46014594
zend_compile_ns_call(result, &name_node, args_ast, ast->lineno);
46024595
}
@@ -4615,14 +4608,7 @@ static void zend_compile_call(znode *result, zend_ast *ast, uint32_t type) /* {{
46154608

46164609
/* Special assert() handling should apply independently of compiler flags. */
46174610
if (fbc && zend_string_equals_literal(lcname, "assert") && !is_callable_convert) {
4618-
if (CG(memoize_mode) == ZEND_MEMOIZE_NONE) {
4619-
zend_compile_assert(result, zend_ast_get_list(args_ast), lcname, fbc, ast->lineno);
4620-
} else {
4621-
/* We want to always memoize assert calls, even if they are positioned in
4622-
* write-context. This prevents memoizing their arguments that might not be
4623-
* evaluated if assertions are disabled, using a TMPVAR that wasn't initialized. */
4624-
zend_compile_memoized_expr(result, ast);
4625-
}
4611+
zend_compile_assert(result, zend_ast_get_list(args_ast), lcname, fbc, ast->lineno);
46264612
zend_string_release(lcname);
46274613
zval_ptr_dtor(&name_node.u.constant);
46284614
return;
@@ -10591,6 +10577,17 @@ static zend_op *zend_compile_var_inner(znode *result, zend_ast *ast, uint32_t ty
1059110577
{
1059210578
CG(zend_lineno) = zend_ast_get_lineno(ast);
1059310579

10580+
if (CG(memoize_mode) != ZEND_MEMOIZE_NONE) {
10581+
switch (ast->kind) {
10582+
case ZEND_AST_CALL:
10583+
case ZEND_AST_METHOD_CALL:
10584+
case ZEND_AST_NULLSAFE_METHOD_CALL:
10585+
case ZEND_AST_STATIC_CALL:
10586+
zend_compile_memoized_expr(result, ast);
10587+
return &CG(active_op_array)->opcodes[CG(active_op_array)->last - 1];
10588+
}
10589+
}
10590+
1059410591
switch (ast->kind) {
1059510592
case ZEND_AST_VAR:
1059610593
return zend_compile_simple_var(result, ast, type, 0);

0 commit comments

Comments
 (0)