Skip to content

Commit 29b67b0

Browse files
iluuu1994iluuu1994
committed
Implement code review feedback
1 parent a7dbfd3 commit 29b67b0

11 files changed

+84
-32
lines changed

‎Zend/Optimizer/dce.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,8 @@ static inline bool may_have_side_effects(
246246
if ((opline->extended_value & (ZEND_BIND_IMPLICIT|ZEND_BIND_EXPLICIT))) {
247247
return 1;
248248
}
249-
/* Modifies local variables and static variables which are observable through reflection */
250-
if (opline->extended_value & ZEND_BIND_REF) {
249+
/* Modifies static variables which are observable through reflection */
250+
if ((opline->extended_value & ZEND_BIND_REF) && opline->op2_type != IS_UNUSED) {
251251
return 1;
252252
}
253253
}

‎Zend/Optimizer/zend_inference.c

+1-4
Original file line numberDiff line numberDiff line change
@@ -4742,11 +4742,8 @@ ZEND_API bool zend_may_throw_ex(const zend_op *opline, const zend_ssa_op *ssa_op
47424742
if (t1 & (MAY_BE_OBJECT|MAY_BE_RESOURCE|MAY_BE_ARRAY_OF_OBJECT|MAY_BE_ARRAY_OF_RESOURCE|MAY_BE_ARRAY_OF_ARRAY)) {
47434743
/* Destructor may throw. */
47444744
return 1;
4745-
} else {
4746-
zval *value = (zval*)((char*)op_array->static_variables->arData + (opline->extended_value & ~(ZEND_BIND_REF|ZEND_BIND_IMPLICIT|ZEND_BIND_EXPLICIT)));
4747-
/* Destructor of overwritten local variable may throw. */
4748-
return Z_TYPE_P(value) == IS_CONSTANT_AST;
47494745
}
4746+
return 0;
47504747
case ZEND_ASSIGN_DIM:
47514748
if ((opline+1)->op1_type == IS_CV) {
47524749
if (_ssa_op1_info(op_array, ssa, opline+1, ssa_op+1) & MAY_BE_UNDEF) {

‎Zend/tests/static_variables_closure_bind.phpt

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
Static variable can't override nd closure variables
2+
Static variable can't override bound closure variables
33
--FILE--
44
<?php
55

‎Zend/tests/static_variables_global_2.phpt

+4
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,15 @@ function foo() {
1212
}
1313

1414
foo();
15+
var_dump($a);
1516
foo();
17+
var_dump($a);
1618

1719
?>
1820
--EXPECT--
1921
int(42)
2022
int(41)
2123
int(42)
24+
int(42)
2225
int(41)
26+
int(42)

‎Zend/tests/static_variables_recursive.phpt

+14-14
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Static variable with recursive initializer
44
<?php
55

66
function foo($i) {
7-
static $a = $i <= 10 ? foo($i + 1) : 'Done';
7+
static $a = $i <= 10 ? foo($i + 1) : "Done $i";
88
var_dump($a);
99
return $i;
1010
}
@@ -14,16 +14,16 @@ foo(5);
1414

1515
?>
1616
--EXPECT--
17-
string(4) "Done"
18-
string(4) "Done"
19-
string(4) "Done"
20-
string(4) "Done"
21-
string(4) "Done"
22-
string(4) "Done"
23-
string(4) "Done"
24-
string(4) "Done"
25-
string(4) "Done"
26-
string(4) "Done"
27-
string(4) "Done"
28-
string(4) "Done"
29-
string(4) "Done"
17+
string(7) "Done 11"
18+
string(7) "Done 11"
19+
string(7) "Done 11"
20+
string(7) "Done 11"
21+
string(7) "Done 11"
22+
string(7) "Done 11"
23+
string(7) "Done 11"
24+
string(7) "Done 11"
25+
string(7) "Done 11"
26+
string(7) "Done 11"
27+
string(7) "Done 11"
28+
string(7) "Done 11"
29+
string(7) "Done 11"

‎Zend/zend_compile.c

+9-4
Original file line numberDiff line numberDiff line change
@@ -4869,7 +4869,6 @@ static void zend_compile_static_var_common(zend_string *var_name, zval *value, u
48694869
static void zend_compile_static_var(zend_ast *ast) /* {{{ */
48704870
{
48714871
zend_ast *var_ast = ast->child[0];
4872-
zend_ast *value_ast = ast->child[1];
48734872
zend_string *var_name = zend_ast_get_str(var_ast);
48744873

48754874
if (zend_string_equals_literal(var_name, "this")) {
@@ -4887,13 +4886,19 @@ static void zend_compile_static_var(zend_ast *ast) /* {{{ */
48874886
zend_error_noreturn(E_COMPILE_ERROR, "Duplicate declaration of static variable $%s", ZSTR_VAL(var_name));
48884887
}
48894888

4890-
if (!value_ast) {
4891-
zend_compile_static_var_common(var_name, &EG(uninitialized_zval), ZEND_BIND_REF);
4889+
zend_eval_const_expr(&ast->child[1]);
4890+
zend_ast *value_ast = ast->child[1];
4891+
4892+
if (!value_ast || value_ast->kind == ZEND_AST_ZVAL) {
4893+
zval *value_zv = value_ast
4894+
? zend_ast_get_zval(value_ast)
4895+
: &EG(uninitialized_zval);
4896+
zend_compile_static_var_common(var_name, value_zv, ZEND_BIND_REF);
48924897
} else {
48934898
zend_op *opline;
48944899

48954900
zval *placeholder_ptr = zend_hash_update(CG(active_op_array)->static_variables, var_name, &EG(uninitialized_zval));
4896-
Z_TYPE_EXTRA_P(placeholder_ptr) |= IS_TYPE_UNINITIALIZED;
4901+
Z_TYPE_EXTRA_P(placeholder_ptr) |= IS_STATIC_VAR_UNINITIALIZED;
48974902
uint32_t placeholder_offset = (uint32_t)((char*)placeholder_ptr - (char*)CG(active_op_array)->static_variables->arData);
48984903

48994904
uint32_t static_def_jmp_opnum = get_next_op_number();

‎Zend/zend_types.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -697,10 +697,10 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
697697
#define IS_TYPE_REFCOUNTED (1<<0)
698698
#define IS_TYPE_COLLECTABLE (1<<1)
699699
/* Used for static variables to check if they have been initialized. We can't use IS_UNDEF because
700-
* we can't store IS_UNDEF zvals in static_variables HashTable. This needs to live in type_info so
701-
* that the ZEND_ASSIGN overrides it but lives in extra to avoid breaking the Z_REFCOUNTED()
700+
* we can't store IS_UNDEF zvals in the static_variables HashTable. This needs to live in type_info
701+
* so that the ZEND_ASSIGN overrides it but is moved to extra to avoid breaking the Z_REFCOUNTED()
702702
* optimization that only checks for Z_TYPE_FLAGS() without `& (IS_TYPE_COLLECTABLE|IS_TYPE_REFCOUNTED)`. */
703-
#define IS_TYPE_UNINITIALIZED (1<<0)
703+
#define IS_STATIC_VAR_UNINITIALIZED (1<<0)
704704

705705
#if 1
706706
/* This optimized version assumes that we have a single "type_flag" */

‎Zend/zend_vm_def.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -8871,7 +8871,7 @@ ZEND_VM_HANDLER(183, ZEND_BIND_STATIC, CV, ANY, REF)
88718871
if (OP2_TYPE == IS_UNUSED) {
88728872
ZVAL_COPY_VALUE(&ref->val, value);
88738873
} else {
8874-
i_zval_ptr_dtor(value);
8874+
ZEND_ASSERT(!Z_REFCOUNTED_P(value));
88758875
ZVAL_COPY(&ref->val, GET_OP2_ZVAL_PTR_DEREF(BP_VAR_R));
88768876
FREE_OP2();
88778877
}
@@ -8907,7 +8907,7 @@ ZEND_VM_HANDLER(203, ZEND_JMP_STATIC_DEF, ANY, JMP_ADDR)
89078907
ZEND_ASSERT(GC_REFCOUNT(ht) == 1);
89088908

89098909
value = (zval*)((char*)ht->arData + (opline->extended_value));
8910-
if (Z_TYPE_EXTRA_P(value) & IS_TYPE_UNINITIALIZED) {
8910+
if (Z_TYPE_EXTRA_P(value) & IS_STATIC_VAR_UNINITIALIZED) {
89118911
ZEND_VM_NEXT_OPCODE();
89128912
} else {
89138913
ZEND_VM_JMP_EX(OP_JMP_ADDR(opline, opline->op2), 0);

‎Zend/zend_vm_execute.h

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/opcache/tests/optimize_static_002.phpt

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
Keep BIND_STATIC when static variable has an initializer
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.optimization_level=-1
7+
--EXTENSIONS--
8+
opcache
9+
--FILE--
10+
<?php
11+
function foo() {
12+
// Eliminating this does have a small side effect of not showing & for $a when var_dumping static variables
13+
static $a = 42;
14+
var_dump((new ReflectionFunction(__FUNCTION__))->getStaticVariables());
15+
}
16+
foo();
17+
?>
18+
--EXPECT--
19+
array(1) {
20+
["a"]=>
21+
int(42)
22+
}

‎ext/opcache/tests/optimize_static_003.phpt

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
Keep BIND_STATIC when static variable has an initializer
3+
--INI--
4+
opcache.enable=1
5+
opcache.enable_cli=1
6+
opcache.optimization_level=-1
7+
--EXTENSIONS--
8+
opcache
9+
--FILE--
10+
<?php
11+
function bar() {
12+
return 42;
13+
}
14+
function foo() {
15+
static $a = bar();
16+
var_dump((new ReflectionFunction(__FUNCTION__))->getStaticVariables());
17+
}
18+
foo();
19+
?>
20+
--EXPECT--
21+
array(1) {
22+
["a"]=>
23+
&int(42)
24+
}

0 commit comments

Comments
 (0)