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.

5 files changed, 67 insertions, 2 deletions

diff --git a/UPGRADING b/UPGRADING
index 46d92f8115..3e4879c6ef 100644
--- 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
index 0000000000..d56b4f0663
--- /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 a8c4a89862..df6516a300 100644
--- 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 d56fbd1381..33c40296e0 100644
--- 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 a3dd2b2272..120d92beb5 100644
--- 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  |     |     */