Skip to content

Commit d1157cb

Browse files
committed
Relax closure $this unbinding deprecation
Only deprecate unbinding of $this from a closure if $this is syntactically used within the closure. This is desired to support Laravel's macro system, see laravel/framework#29482. This should still allow us to implement the performance improvements we're interested in for PHP 8, without breaking existing use-cases.
1 parent 8807889 commit d1157cb

File tree

5 files changed

+67
-2
lines changed

5 files changed

+67
-2
lines changed

UPGRADING

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ PHP 7.4 UPGRADE NOTES
365365
ReflectionMethod::getClosure() and closure rebinding is deprecated. Doing
366366
so is equivalent to calling a non-static method statically, which has been
367367
deprecated since PHP 7.0.
368-
. Unbinding $this of a non-static closure is deprecated.
368+
. Unbinding $this of a non-static closure that uses $this is deprecated.
369369
. Using "parent" inside a class without a parent is deprecated, and will throw
370370
a compile-time error in the future. Currently an error will only be
371371
generated if/when the parent is accessed at run-time.

Zend/tests/closure_062.phpt

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
--TEST--
2+
Closure $this unbinding deprecation
3+
--FILE--
4+
<?php
5+
6+
class Test {
7+
public function method() {
8+
echo "instance scoped, non-static, \$this used\n";
9+
$fn = function() {
10+
var_dump($this);
11+
};
12+
$fn->bindTo(null);
13+
echo "instance scoped, static, \$this used\n";
14+
$fn = static function() {
15+
var_dump($this);
16+
};
17+
$fn->bindTo(null);
18+
echo "instance scoped, non-static, \$this not used\n";
19+
$fn = function() {
20+
var_dump($notThis);
21+
};
22+
$fn->bindTo(null);
23+
}
24+
25+
public static function staticMethod() {
26+
echo "static scoped, non-static, \$this used\n";
27+
$fn = function() {
28+
var_dump($this);
29+
};
30+
$fn->bindTo(null);
31+
echo "static scoped, static, \$this used\n";
32+
$fn = static function() {
33+
var_dump($this);
34+
};
35+
$fn->bindTo(null);
36+
echo "static scoped, static, \$this not used\n";
37+
$fn = function() {
38+
var_dump($notThis);
39+
};
40+
$fn->bindTo(null);
41+
}
42+
}
43+
44+
(new Test)->method();
45+
Test::staticMethod();
46+
47+
?>
48+
--EXPECTF--
49+
instance scoped, non-static, $this used
50+
51+
Deprecated: Unbinding $this of closure is deprecated in %s on line %d
52+
instance scoped, static, $this used
53+
instance scoped, non-static, $this not used
54+
static scoped, non-static, $this used
55+
static scoped, static, $this used
56+
static scoped, static, $this not used

Zend/zend_closures.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ static zend_bool zend_valid_closure_binding(
9090
} else {
9191
zend_error(E_DEPRECATED, "Unbinding $this of a method is deprecated");
9292
}
93-
} else if (!is_fake_closure && !Z_ISUNDEF(closure->this_ptr)) {
93+
} else if (!is_fake_closure && !Z_ISUNDEF(closure->this_ptr)
94+
&& (func->common.fn_flags & ZEND_ACC_USES_THIS)) {
9495
// TODO: Only deprecate if it had $this *originally*?
9596
zend_error(E_DEPRECATED, "Unbinding $this of closure is deprecated");
9697
}

Zend/zend_compile.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2380,6 +2380,7 @@ static zend_op *zend_compile_simple_var(znode *result, zend_ast *ast, uint32_t t
23802380
opline->result_type = IS_TMP_VAR;
23812381
result->op_type = IS_TMP_VAR;
23822382
}
2383+
CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
23832384
return opline;
23842385
} else if (zend_try_compile_cv(result, ast) == FAILURE) {
23852386
return zend_compile_simple_var_no_cv(result, ast, type, delayed);
@@ -2474,6 +2475,7 @@ static zend_op *zend_delayed_compile_prop(znode *result, zend_ast *ast, uint32_t
24742475

24752476
if (is_this_fetch(obj_ast)) {
24762477
obj_node.op_type = IS_UNUSED;
2478+
CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
24772479
} else {
24782480
opline = zend_delayed_compile_var(&obj_node, obj_ast, type, 0);
24792481
if (opline && type == BP_VAR_W && (opline->opcode == ZEND_FETCH_STATIC_PROP_W || opline->opcode == ZEND_FETCH_OBJ_W)) {
@@ -3010,6 +3012,7 @@ uint32_t zend_compile_args(zend_ast *ast, zend_function *fbc) /* {{{ */
30103012
if (is_this_fetch(arg)) {
30113013
zend_emit_op(&arg_node, ZEND_FETCH_THIS, NULL, NULL);
30123014
opcode = ZEND_SEND_VAR_EX;
3015+
CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
30133016
break;
30143017
} else if (zend_try_compile_cv(&arg_node, arg) == SUCCESS) {
30153018
opcode = ZEND_SEND_VAR_EX;
@@ -3878,6 +3881,7 @@ void zend_compile_method_call(znode *result, zend_ast *ast, uint32_t type) /* {{
38783881

38793882
if (is_this_fetch(obj_ast)) {
38803883
obj_node.op_type = IS_UNUSED;
3884+
CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
38813885
} else {
38823886
zend_compile_expr(&obj_node, obj_ast);
38833887
}
@@ -7785,6 +7789,7 @@ void zend_compile_isset_or_empty(znode *result, zend_ast *ast) /* {{{ */
77857789
case ZEND_AST_VAR:
77867790
if (is_this_fetch(var_ast)) {
77877791
opline = zend_emit_op(result, ZEND_ISSET_ISEMPTY_THIS, NULL, NULL);
7792+
CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
77887793
} else if (zend_try_compile_cv(&var_node, var_ast) == SUCCESS) {
77897794
opline = zend_emit_op(result, ZEND_ISSET_ISEMPTY_CV, &var_node, NULL);
77907795
} else {

Zend/zend_compile.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,9 @@ typedef struct _zend_oparray_context {
337337
/* function is a destructor | | | */
338338
#define ZEND_ACC_DTOR (1 << 29) /* | X | | */
339339
/* | | | */
340+
/* closure uses $this | | | */
341+
#define ZEND_ACC_USES_THIS (1 << 30) /* | X | | */
342+
/* | | | */
340343
/* op_array uses strict mode types | | | */
341344
#define ZEND_ACC_STRICT_TYPES (1U << 31) /* | X | | */
342345

0 commit comments

Comments
 (0)