Skip to content

Commit dbe6d01

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.
1 parent b24b351 commit dbe6d01

File tree

2 files changed

+77
-16
lines changed

2 files changed

+77
-16
lines changed

Zend/tests/assign_coalesce_008.phpt

Lines changed: 64 additions & 0 deletions
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

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4587,14 +4587,7 @@ static void zend_compile_call(znode *result, zend_ast *ast, uint32_t type) /* {{
45874587
if (runtime_resolution) {
45884588
if (zend_string_equals_literal_ci(zend_ast_get_str(name_ast), "assert")
45894589
&& !is_callable_convert) {
4590-
if (CG(memoize_mode) == ZEND_MEMOIZE_NONE) {
4591-
zend_compile_assert(result, zend_ast_get_list(args_ast), Z_STR(name_node.u.constant), NULL, ast->lineno);
4592-
} else {
4593-
/* We want to always memoize assert calls, even if they are positioned in
4594-
* write-context. This prevents memoizing their arguments that might not be
4595-
* evaluated if assertions are disabled, using a TMPVAR that wasn't initialized. */
4596-
zend_compile_memoized_expr(result, ast);
4597-
}
4590+
zend_compile_assert(result, zend_ast_get_list(args_ast), Z_STR(name_node.u.constant), NULL, ast->lineno);
45984591
} else {
45994592
zend_compile_ns_call(result, &name_node, args_ast, ast->lineno);
46004593
}
@@ -4613,14 +4606,7 @@ static void zend_compile_call(znode *result, zend_ast *ast, uint32_t type) /* {{
46134606

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

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

0 commit comments

Comments
 (0)