From 590de399f01cd08aa4825b26b91785e07abcf68c Mon Sep 17 00:00:00 2001
From: Damien George <damien@micropython.org>
Date: Tue, 3 May 2022 10:38:15 +1000
Subject: [PATCH] py/emitcommon: Don't implicitly close class vars that are
 assigned to.

When in a class body or at the module level don't implicitly close over
variables that have been assigned to.

Fixes issue #8603.

Signed-off-by: Damien George <damien@micropython.org>
---
 py/emitcommon.c             | 13 +++++--
 py/scope.h                  |  1 +
 tests/basics/scope_class.py | 77 +++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 4 deletions(-)
 create mode 100644 tests/basics/scope_class.py

diff --git a/py/emitcommon.c b/py/emitcommon.c
index c1780d2db9..647430232a 100644
--- a/py/emitcommon.c
+++ b/py/emitcommon.c
@@ -44,9 +44,14 @@ qstr_short_t mp_emit_common_use_qstr(mp_emit_common_t *emit, qstr qst) {
 void mp_emit_common_get_id_for_modification(scope_t *scope, qstr qst) {
     // name adding/lookup
     id_info_t *id = scope_find_or_add_id(scope, qst, ID_INFO_KIND_GLOBAL_IMPLICIT);
-    if (SCOPE_IS_FUNC_LIKE(scope->kind) && id->kind == ID_INFO_KIND_GLOBAL_IMPLICIT) {
-        // rebind as a local variable
-        id->kind = ID_INFO_KIND_LOCAL;
+    if (id->kind == ID_INFO_KIND_GLOBAL_IMPLICIT) {
+        if (SCOPE_IS_FUNC_LIKE(scope->kind)) {
+            // rebind as a local variable
+            id->kind = ID_INFO_KIND_LOCAL;
+        } else {
+            // mark this as assigned, to prevent it from being closed over
+            id->kind = ID_INFO_KIND_GLOBAL_IMPLICIT_ASSIGNED;
+        }
     }
 }
 
@@ -57,7 +62,7 @@ void mp_emit_common_id_op(emit_t *emit, const mp_emit_method_table_id_ops_t *emi
     assert(id != NULL);
 
     // call the emit backend with the correct code
-    if (id->kind == ID_INFO_KIND_GLOBAL_IMPLICIT) {
+    if (id->kind == ID_INFO_KIND_GLOBAL_IMPLICIT || id->kind == ID_INFO_KIND_GLOBAL_IMPLICIT_ASSIGNED) {
         emit_method_table->global(emit, qst, MP_EMIT_IDOP_GLOBAL_NAME);
     } else if (id->kind == ID_INFO_KIND_GLOBAL_EXPLICIT) {
         emit_method_table->global(emit, qst, MP_EMIT_IDOP_GLOBAL_GLOBAL);
diff --git a/py/scope.h b/py/scope.h
index 5006deadea..b781dde427 100644
--- a/py/scope.h
+++ b/py/scope.h
@@ -32,6 +32,7 @@
 typedef enum {
     ID_INFO_KIND_UNDECIDED,
     ID_INFO_KIND_GLOBAL_IMPLICIT,
+    ID_INFO_KIND_GLOBAL_IMPLICIT_ASSIGNED,
     ID_INFO_KIND_GLOBAL_EXPLICIT,
     ID_INFO_KIND_LOCAL, // in a function f, written and only referenced by f
     ID_INFO_KIND_CELL,  // in a function f, read/written by children of f
diff --git a/tests/basics/scope_class.py b/tests/basics/scope_class.py
new file mode 100644
index 0000000000..9c519695dc
--- /dev/null
+++ b/tests/basics/scope_class.py
@@ -0,0 +1,77 @@
+# test scoping rules that involve a class
+
+# the inner A.method should be independent to the local function called method
+def test1():
+    def method():
+        pass
+
+    class A:
+        def method():
+            pass
+
+    print(hasattr(A, "method"))
+    print(hasattr(A(), "method"))
+
+
+test1()
+
+
+# the inner A.method is a closure and overrides the local function called method
+def test2():
+    def method():
+        return "outer"
+
+    class A:
+        nonlocal method
+
+        def method():
+            return "inner"
+
+    print(hasattr(A, "method"))
+    print(hasattr(A(), "method"))
+    return method()  # this is actually A.method
+
+
+print(test2())
+
+
+# a class body will capture external variables by value (not by reference)
+def test3(x):
+    class A:
+        local = x
+
+    x += 1
+    return x, A.local
+
+
+print(test3(42))
+
+
+# assigning to a variable in a class will implicitly prevent it from closing over a variable
+def test4(global_):
+    class A:
+        local = global_  # fetches outer global_
+        global_ = "global2"  # creates class attribute
+
+    global_ += 1  # updates local variable
+    return global_, A.local, A.global_
+
+
+global_ = "global"
+print(test4(42), global_)
+
+
+# methods within a class can close over variables outside the class
+def test5(x):
+    def closure():
+        return x
+
+    class A:
+        def method():
+            return x, closure()
+
+    closure = lambda: x + 1  # change it after A has been created
+    return A
+
+
+print(test5(42).method())