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.

diff --git a/UPGRADING b/UPGRADING
index 46d92f8115a103572b9e9b2db5738e7fbd678c06..3e4879c6efac7d3bb865acc2a8964559ed36bfb8 100644 (file)
--- a/UPGRADING
+++ b/UPGRADING
@@ -365,7 +365,7 @@ PHP 7.4 UPGRADE NOTES
     ReflectionMethod::getClosure() and closure rebinding is deprecated. Doing
     so is equivalent to calling a non-static method statically, which has been
     deprecated since PHP 7.0.
-  . Unbinding $this of a non-static closure is deprecated.
+  . Unbinding $this of a non-static closure that uses $this is deprecated.
   . Using "parent" inside a class without a parent is deprecated, and will throw
     a compile-time error in the future. Currently an error will only be
     generated if/when the parent is accessed at run-time.

diff --git a/Zend/tests/closure_062.phpt b/Zend/tests/closure_062.phpt
new file mode 100644 (file)
index 0000000..d56b4f0
--- /dev/null
+++ b/Zend/tests/closure_062.phpt
@@ -0,0 +1,56 @@
+--TEST--
+Closure $this unbinding deprecation
+--FILE--
+<?php
+
+class Test {
+    public function method() {
+        echo "instance scoped, non-static, \$this used\n";
+        $fn = function() {
+            var_dump($this);
+        };
+        $fn->bindTo(null);
+        echo "instance scoped, static, \$this used\n";
+        $fn = static function() {
+            var_dump($this);
+        };
+        $fn->bindTo(null);
+        echo "instance scoped, non-static, \$this not used\n";
+        $fn = function() {
+            var_dump($notThis);
+        };
+        $fn->bindTo(null);
+    }
+
+    public static function staticMethod() {
+        echo "static scoped, non-static, \$this used\n";
+        $fn = function() {
+            var_dump($this);
+        };
+        $fn->bindTo(null);
+        echo "static scoped, static, \$this used\n";
+        $fn = static function() {
+            var_dump($this);
+        };
+        $fn->bindTo(null);
+        echo "static scoped, static, \$this not used\n";
+        $fn = function() {
+            var_dump($notThis);
+        };
+        $fn->bindTo(null);
+    }
+}
+
+(new Test)->method();
+Test::staticMethod();
+
+?>
+--EXPECTF--
+instance scoped, non-static, $this used
+
+Deprecated: Unbinding $this of closure is deprecated in %s on line %d
+instance scoped, static, $this used
+instance scoped, non-static, $this not used
+static scoped, non-static, $this used
+static scoped, static, $this used
+static scoped, static, $this not used

diff --git a/Zend/zend_closures.c b/Zend/zend_closures.c
index a8c4a898627fcaa28baac135c0aca69f2b7a1833..df6516a300405436afc5d91bd95d2e95dad94ad3 100644 (file)
--- a/Zend/zend_closures.c
+++ b/Zend/zend_closures.c
@@ -90,7 +90,8 @@ static zend_bool zend_valid_closure_binding(
                } else {
                        zend_error(E_DEPRECATED, "Unbinding $this of a method is deprecated");
                }
-       } else if (!is_fake_closure && !Z_ISUNDEF(closure->this_ptr)) {
+       } else if (!is_fake_closure && !Z_ISUNDEF(closure->this_ptr)
+                       && (func->common.fn_flags & ZEND_ACC_USES_THIS)) {
                // TODO: Only deprecate if it had $this *originally*?
                zend_error(E_DEPRECATED, "Unbinding $this of closure is deprecated");
        }

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index d56fbd1381a0828be16b07cc6196fc9039a6943b..33c40296e06dcfe744b45819b6c1f4069c7c970c 100644 (file)
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -2380,6 +2380,7 @@ static zend_op *zend_compile_simple_var(znode *result, zend_ast *ast, uint32_t t
                        opline->result_type = IS_TMP_VAR;
                        result->op_type = IS_TMP_VAR;
                }
+               CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
                return opline;
        } else if (zend_try_compile_cv(result, ast) == FAILURE) {
                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
 
        if (is_this_fetch(obj_ast)) {
                obj_node.op_type = IS_UNUSED;
+               CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
        } else {
                opline = zend_delayed_compile_var(&obj_node, obj_ast, type, 0);
                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) /* {{{ */
                                                if (is_this_fetch(arg)) {
                                                        zend_emit_op(&arg_node, ZEND_FETCH_THIS, NULL, NULL);
                                                        opcode = ZEND_SEND_VAR_EX;
+                                                       CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
                                                        break;
                                                } else if (zend_try_compile_cv(&arg_node, arg) == SUCCESS) {
                                                        opcode = ZEND_SEND_VAR_EX;
@@ -3878,6 +3881,7 @@ void zend_compile_method_call(znode *result, zend_ast *ast, uint32_t type) /* {{
 
        if (is_this_fetch(obj_ast)) {
                obj_node.op_type = IS_UNUSED;
+               CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
        } else {
                zend_compile_expr(&obj_node, obj_ast);
        }
@@ -7785,6 +7789,7 @@ void zend_compile_isset_or_empty(znode *result, zend_ast *ast) /* {{{ */
                case ZEND_AST_VAR:
                        if (is_this_fetch(var_ast)) {
                                opline = zend_emit_op(result, ZEND_ISSET_ISEMPTY_THIS, NULL, NULL);
+                               CG(active_op_array)->fn_flags |= ZEND_ACC_USES_THIS;
                        } else if (zend_try_compile_cv(&var_node, var_ast) == SUCCESS) {
                                opline = zend_emit_op(result, ZEND_ISSET_ISEMPTY_CV, &var_node, NULL);
                        } else {

diff --git a/Zend/zend_compile.h b/Zend/zend_compile.h
index a3dd2b2272629de7b89bad8a678e0a83fe56ef36..120d92beb5ff1e12330d9d8fe99dc92db05da7fb 100644 (file)
--- a/Zend/zend_compile.h
+++ b/Zend/zend_compile.h
@@ -337,6 +337,9 @@ typedef struct _zend_oparray_context {
 /* function is a destructor                               |     |     |     */
 #define ZEND_ACC_DTOR                    (1 << 29) /*     |  X  |     |     */
 /*                                                        |     |     |     */
+/* closure uses $this                                     |     |     |     */
+#define ZEND_ACC_USES_THIS               (1 << 30) /*     |  X  |     |     */
+/*                                                        |     |     |     */
 /* op_array uses strict mode types                        |     |     |     */
 #define ZEND_ACC_STRICT_TYPES            (1U << 31) /*    |  X  |     |     */