py: Making closures now passes pointer to stack, not a tuple for vars.

Closed over variables are now passed on the stack, instead of creating a
tuple and passing that.  This way memory for the closed over variables
can be allocated within the closure object itself.  See issue #510 for
background.

11 files changed, 71 insertions, 56 deletions

diff --git a/py/compile.c b/py/compile.c
index b0f1f9e00d..57f6f1546f 100644
--- a/py/compile.c
+++ b/py/compile.c
@@ -889,8 +889,7 @@ void close_over_variables_etc(compiler_t *comp, scope_t *this_scope, int n_pos_d
     if (nfree == 0) {
         EMIT_ARG(make_function, this_scope, n_pos_defaults, n_kw_defaults);
     } else {
-        EMIT_ARG(build_tuple, nfree);
-        EMIT_ARG(make_closure, this_scope, n_pos_defaults, n_kw_defaults);
+        EMIT_ARG(make_closure, this_scope, nfree, n_pos_defaults, n_kw_defaults);
     }
 }
 
@@ -957,6 +956,8 @@ void compile_funcdef_param(compiler_t *comp, mp_parse_node_t pn) {
                     // we need to do this here before we start building the map for the default keywords
                     if (comp->num_default_params > 0) {
                         EMIT_ARG(build_tuple, comp->num_default_params);
+                    } else {
+                        EMIT(load_null); // sentinel indicating empty default positional args
                     }
                     // first default dict param, so make the map
                     EMIT_ARG(build_map, 0);
@@ -1009,6 +1010,7 @@ qstr compile_funcdef_helper(compiler_t *comp, mp_parse_node_struct_t *pns, uint
     // the default keywords args may have already made the tuple; if not, do it now
     if (comp->num_default_params > 0 && comp->num_dict_params == 0) {
         EMIT_ARG(build_tuple, comp->num_default_params);
+        EMIT(load_null); // sentinel indicating empty default keyword args
     }
 #endif
 
diff --git a/py/emit.h b/py/emit.h
index 74cc5baa4a..64c2c85480 100644
--- a/py/emit.h
+++ b/py/emit.h
@@ -42,6 +42,7 @@ typedef struct _emit_method_table_t {
     void (*load_const_id)(emit_t *emit, qstr qstr);
     void (*load_const_str)(emit_t *emit, qstr qstr, bool bytes);
     void (*load_const_verbatim_str)(emit_t *emit, const char *str); // only needed for emitcpy
+    void (*load_null)(emit_t *emit);
     void (*load_fast)(emit_t *emit, qstr qstr, uint id_flags, int local_num);
     void (*load_deref)(emit_t *emit, qstr qstr, int local_num);
     void (*load_closure)(emit_t *emit, qstr qstr, int local_num); // only needed for emitcpy
@@ -100,7 +101,7 @@ typedef struct _emit_method_table_t {
     void (*unpack_sequence)(emit_t *emit, int n_args);
     void (*unpack_ex)(emit_t *emit, int n_left, int n_right);
     void (*make_function)(emit_t *emit, scope_t *scope, uint n_pos_defaults, uint n_kw_defaults);
-    void (*make_closure)(emit_t *emit, scope_t *scope, uint n_pos_defaults, uint n_kw_defaults);
+    void (*make_closure)(emit_t *emit, scope_t *scope, uint n_closed_over, uint n_pos_defaults, uint n_kw_defaults);
     void (*call_function)(emit_t *emit, int n_positional, int n_keyword, uint star_flags);
     void (*call_method)(emit_t *emit, int n_positional, int n_keyword, uint star_flags);
     void (*return_value)(emit_t *emit);
diff --git a/py/emitbc.c b/py/emitbc.c
index d7b309a37f..c445d9748d 100644
--- a/py/emitbc.c
+++ b/py/emitbc.c
@@ -761,35 +761,21 @@ STATIC void emit_bc_make_function(emit_t *emit, scope_t *scope, uint n_pos_defau
         emit_bc_pre(emit, 1);
         emit_write_byte_code_byte_ptr(emit, MP_BC_MAKE_FUNCTION, scope->raw_code);
     } else {
-        if (n_pos_defaults == 0) {
-            // load dummy entry for non-existent positional default tuple
-            emit_bc_load_null(emit);
-            emit_bc_rot_two(emit);
-        } else if (n_kw_defaults == 0) {
-            // load dummy entry for non-existent keyword default dict
-            emit_bc_load_null(emit);
-        }
         emit_bc_pre(emit, -1);
         emit_write_byte_code_byte_ptr(emit, MP_BC_MAKE_FUNCTION_DEFARGS, scope->raw_code);
     }
 }
 
-STATIC void emit_bc_make_closure(emit_t *emit, scope_t *scope, uint n_pos_defaults, uint n_kw_defaults) {
+STATIC void emit_bc_make_closure(emit_t *emit, scope_t *scope, uint n_closed_over, uint n_pos_defaults, uint n_kw_defaults) {
     if (n_pos_defaults == 0 && n_kw_defaults == 0) {
-        emit_bc_pre(emit, 0);
+        emit_bc_pre(emit, -n_closed_over + 1);
         emit_write_byte_code_byte_ptr(emit, MP_BC_MAKE_CLOSURE, scope->raw_code);
+        emit_write_byte_code_byte(emit, n_closed_over);
     } else {
-        if (n_pos_defaults == 0) {
-            // load dummy entry for non-existent positional default tuple
-            emit_bc_load_null(emit);
-            emit_bc_rot_three(emit);
-        } else if (n_kw_defaults == 0) {
-            // load dummy entry for non-existent keyword default dict
-            emit_bc_load_null(emit);
-            emit_bc_rot_two(emit);
-        }
-        emit_bc_pre(emit, -2);
+        assert(n_closed_over <= 255);
+        emit_bc_pre(emit, -2 - n_closed_over + 1);
         emit_write_byte_code_byte_ptr(emit, MP_BC_MAKE_CLOSURE_DEFARGS, scope->raw_code);
+        emit_write_byte_code_byte(emit, n_closed_over);
     }
 }
 
@@ -870,6 +856,7 @@ const emit_method_table_t emit_bc_method_table = {
     emit_bc_load_const_id,
     emit_bc_load_const_str,
     emit_bc_load_const_verbatim_str,
+    emit_bc_load_null,
     emit_bc_load_fast,
     emit_bc_load_deref,
     emit_bc_load_closure,
diff --git a/py/emitcpy.c b/py/emitcpy.c
index 119cf818cf..e90b5c428a 100644
--- a/py/emitcpy.c
+++ b/py/emitcpy.c
@@ -228,6 +228,11 @@ STATIC void emit_cpy_load_const_verbatim_str(emit_t *emit, const char *str) {
     }
 }
 
+STATIC void emit_cpy_load_null(emit_t *emit) {
+    // unused for cpy
+    assert(0);
+}
+
 STATIC void emit_cpy_load_fast(emit_t *emit, qstr qstr, uint id_flags, int local_num) {
     emit_pre(emit, 1, 3);
     if (emit->pass == PASS_3) {
@@ -764,7 +769,8 @@ STATIC void emit_cpy_make_function(emit_t *emit, scope_t *scope, uint n_pos_defa
     }
 }
 
-STATIC void emit_cpy_make_closure(emit_t *emit, scope_t *scope, uint n_pos_defaults, uint n_kw_defaults) {
+STATIC void emit_cpy_make_closure(emit_t *emit, scope_t *scope, uint n_closed_over, uint n_pos_defaults, uint n_kw_defaults) {
+    emit_cpy_build_tuple(emit, n_closed_over);
     load_cpy_const_code_and_name(emit, scope->simple_name);
     emit_pre(emit, -2 - n_pos_defaults - 2 * n_kw_defaults, 3);
     if (emit->pass == PASS_3) {
@@ -815,6 +821,7 @@ const emit_method_table_t emit_cpython_method_table = {
     emit_cpy_load_const_id,
     emit_cpy_load_const_str,
     emit_cpy_load_const_verbatim_str,
+    emit_cpy_load_null,
     emit_cpy_load_fast,
     emit_cpy_load_deref,
     emit_cpy_load_closure,
diff --git a/py/emitglue.c b/py/emitglue.c
index ce296f4321..8b1fd97021 100644
--- a/py/emitglue.c
+++ b/py/emitglue.c
@@ -158,10 +158,17 @@ mp_obj_t mp_make_function_from_raw_code(mp_raw_code_t *rc, mp_obj_t def_args, mp
     return fun;
 }
 
-mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, mp_obj_t closure_tuple, mp_obj_t def_args, mp_obj_t def_kw_args) {
-    DEBUG_OP_printf("make_closure_from_raw_code %p\n", rc);
+mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, uint n_closed_over, const mp_obj_t *args) {
+    DEBUG_OP_printf("make_closure_from_raw_code %p %u %p\n", rc, n_closed_over, argrs);
     // make function object
-    mp_obj_t ffun = mp_make_function_from_raw_code(rc, def_args, def_kw_args);
+    mp_obj_t ffun;
+    if (n_closed_over & 0x100) {
+        // default positional and keyword args given
+        ffun = mp_make_function_from_raw_code(rc, args[0], args[1]);
+    } else {
+        // default positional and keyword args not given
+        ffun = mp_make_function_from_raw_code(rc, MP_OBJ_NULL, MP_OBJ_NULL);
+    }
     // wrap function in closure object
-    return mp_obj_new_closure(ffun, closure_tuple);
+    return mp_obj_new_closure(ffun, n_closed_over & 0xff, args + ((n_closed_over >> 7) & 2));
 }
diff --git a/py/emitglue.h b/py/emitglue.h
index 2f9960bfa1..9aea2e4d45 100644
--- a/py/emitglue.h
+++ b/py/emitglue.h
@@ -37,4 +37,4 @@ void mp_emit_glue_assign_native_code(mp_raw_code_t *rc, void *f, uint len, int n
 void mp_emit_glue_assign_inline_asm_code(mp_raw_code_t *rc, void *f, uint len, int n_args);
 
 mp_obj_t mp_make_function_from_raw_code(mp_raw_code_t *rc, mp_obj_t def_args, mp_obj_t def_kw_args);
-mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, mp_obj_t closure_tuple, mp_obj_t def_args, mp_obj_t def_kw_args);
+mp_obj_t mp_make_closure_from_raw_code(mp_raw_code_t *rc, uint n_closed_over, const mp_obj_t *args);
diff --git a/py/emitnative.c b/py/emitnative.c
index d86456244a..dc6e37c53f 100644
--- a/py/emitnative.c
+++ b/py/emitnative.c
@@ -700,6 +700,11 @@ STATIC void emit_native_load_const_verbatim_str(emit_t *emit, const char *str) {
     assert(0);
 }
 
+STATIC void emit_native_load_null(emit_t *emit) {
+    emit_native_pre(emit);
+    emit_post_push_imm(emit, VTYPE_PYOBJ, 0);
+}
+
 STATIC void emit_native_load_fast(emit_t *emit, qstr qstr, uint id_flags, int local_num) {
     vtype_kind_t vtype = emit->local_vtype[local_num];
     if (vtype == VTYPE_UNBOUND) {
@@ -1209,7 +1214,7 @@ STATIC void emit_native_make_function(emit_t *emit, scope_t *scope, uint n_pos_d
     emit_post_push_reg(emit, VTYPE_PYOBJ, REG_RET);
 }
 
-STATIC void emit_native_make_closure(emit_t *emit, scope_t *scope, uint n_pos_defaults, uint n_kw_defaults) {
+STATIC void emit_native_make_closure(emit_t *emit, scope_t *scope, uint n_closed_over, uint n_pos_defaults, uint n_kw_defaults) {
     assert(0);
 }
 
@@ -1335,6 +1340,7 @@ const emit_method_table_t EXPORT_FUN(method_table) = {
     emit_native_load_const_id,
     emit_native_load_const_str,
     emit_native_load_const_verbatim_str,
+    emit_native_load_null,
     emit_native_load_fast,
     emit_native_load_deref,
     emit_native_load_closure,
diff --git a/py/emitpass1.c b/py/emitpass1.c
index ca34f220eb..bccdb4cc87 100644
--- a/py/emitpass1.c
+++ b/py/emitpass1.c
@@ -191,4 +191,5 @@ const emit_method_table_t emit_pass1_method_table = {
     (void*)emit_pass1_dummy,
     (void*)emit_pass1_dummy,
     (void*)emit_pass1_dummy,
+    (void*)emit_pass1_dummy,
 };
diff --git a/py/obj.h b/py/obj.h
index 44d9bf6a6d..d62d376ab4 100644
--- a/py/obj.h
+++ b/py/obj.h
@@ -357,7 +357,7 @@ mp_obj_t mp_obj_new_exception_msg_varg(const mp_obj_type_t *exc_type, const char
 mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_args, mp_obj_t def_args, const byte *code);
 mp_obj_t mp_obj_new_fun_asm(uint n_args, void *fun);
 mp_obj_t mp_obj_new_gen_wrap(mp_obj_t fun);
-mp_obj_t mp_obj_new_closure(mp_obj_t fun, mp_obj_t closure_tuple);
+mp_obj_t mp_obj_new_closure(mp_obj_t fun, uint n_closed, const mp_obj_t *closed);
 mp_obj_t mp_obj_new_tuple(uint n, const mp_obj_t *items);
 mp_obj_t mp_obj_new_list(uint n, mp_obj_t *items);
 mp_obj_t mp_obj_new_dict(int n_args);
diff --git a/py/objclosure.c b/py/objclosure.c
index de7554385a..09371b0348 100644
--- a/py/objclosure.c
+++ b/py/objclosure.c
@@ -11,28 +11,28 @@
 typedef struct _mp_obj_closure_t {
     mp_obj_base_t base;
     mp_obj_t fun;
-    mp_obj_tuple_t *closure_tuple;
+    machine_uint_t n_closed;
+    mp_obj_t closed[];
 } mp_obj_closure_t;
 
 mp_obj_t closure_call(mp_obj_t self_in, uint n_args, uint n_kw, const mp_obj_t *args) {
     mp_obj_closure_t *self = self_in;
-    mp_obj_tuple_t *t = self->closure_tuple;
 
     // need to concatenate closed-over-vars and args
 
-    int n_total = t->len + n_args + 2 * n_kw;
+    int n_total = self->n_closed + n_args + 2 * n_kw;
     if (n_total <= 5) {
         // use stack to allocate temporary args array
         mp_obj_t args2[5];
-        memcpy(args2, t->items, t->len * sizeof(mp_obj_t));
-        memcpy(args2 + t->len, args, (n_args + 2 * n_kw) * sizeof(mp_obj_t));
-        return mp_call_function_n_kw(self->fun, t->len + n_args, n_kw, args2);
+        memcpy(args2, self->closed, self->n_closed * sizeof(mp_obj_t));
+        memcpy(args2 + self->n_closed, args, (n_args + 2 * n_kw) * sizeof(mp_obj_t));
+        return mp_call_function_n_kw(self->fun, self->n_closed + n_args, n_kw, args2);
     } else {
         // use heap to allocate temporary args array
         mp_obj_t *args2 = m_new(mp_obj_t, n_total);
-        memcpy(args2, t->items, t->len * sizeof(mp_obj_t));
-        memcpy(args2 + t->len, args, (n_args + 2 * n_kw) * sizeof(mp_obj_t));
-        mp_obj_t res = mp_call_function_n_kw(self->fun, t->len + n_args, n_kw, args2);
+        memcpy(args2, self->closed, self->n_closed * sizeof(mp_obj_t));
+        memcpy(args2 + self->n_closed, args, (n_args + 2 * n_kw) * sizeof(mp_obj_t));
+        mp_obj_t res = mp_call_function_n_kw(self->fun, self->n_closed + n_args, n_kw, args2);
         m_del(mp_obj_t, args2, n_total);
         return res;
     }
@@ -41,13 +41,12 @@ mp_obj_t closure_call(mp_obj_t self_in, uint n_args, uint n_kw, const mp_obj_t *
 #if 0
 STATIC void closure_print(void (*print)(void *env, const char *fmt, ...), void *env, mp_obj_t o_in, mp_print_kind_t kind) {
     mp_obj_closure_t *o = o_in;
-    print(env, "<closure %p (closed: %p) ", o, o->closure_tuple);
-    mp_obj_tuple_t *t = o->closure_tuple;
-    for (int i = 0; i < t->len; i++) {
-        if (t->items[i] == MP_OBJ_NULL) {
+    print(env, "<closure %p, n_closed=%u ", o, o->n_closed);
+    for (int i = 0; i < o->n_closed; i++) {
+        if (o->closed[i] == MP_OBJ_NULL) {
             print(env, "(nil)");
         } else {
-            mp_obj_print_helper(print, env, t->items[i], PRINT_REPR);
+            mp_obj_print_helper(print, env, o->closed[i], PRINT_REPR);
         }
         print(env, " ");
     }
@@ -62,10 +61,11 @@ const mp_obj_type_t closure_type = {
     .call = closure_call,
 };
 
-mp_obj_t mp_obj_new_closure(mp_obj_t fun, mp_obj_t closure_tuple) {
-    mp_obj_closure_t *o = m_new_obj(mp_obj_closure_t);
+mp_obj_t mp_obj_new_closure(mp_obj_t fun, uint n_closed_over, const mp_obj_t *closed) {
+    mp_obj_closure_t *o = m_new_obj_var(mp_obj_closure_t, mp_obj_t, n_closed_over);
     o->base.type = &closure_type;
     o->fun = fun;
-    o->closure_tuple = closure_tuple;
+    o->n_closed = n_closed_over;
+    memcpy(o->closed, closed, n_closed_over * sizeof(mp_obj_t));
     return o;
 }
diff --git a/py/vm.c b/py/vm.c
index c8e4ba9f48..78a96a48cc 100644
--- a/py/vm.c
+++ b/py/vm.c
@@ -759,19 +759,23 @@ unwind_jump:
                     SET_TOP(mp_make_function_from_raw_code((mp_raw_code_t*)unum, TOP(), obj1));
                     DISPATCH();
 
-                ENTRY(MP_BC_MAKE_CLOSURE):
+                ENTRY(MP_BC_MAKE_CLOSURE): {
                     DECODE_PTR;
-                    // Stack layout: closure_tuple <- TOS
-                    SET_TOP(mp_make_closure_from_raw_code((mp_raw_code_t*)unum, TOP(), MP_OBJ_NULL, MP_OBJ_NULL));
+                    machine_uint_t n_closed_over = *ip++;
+                    // Stack layout: closed_overs <- TOS
+                    sp -= n_closed_over - 1;
+                    SET_TOP(mp_make_closure_from_raw_code((mp_raw_code_t*)unum, n_closed_over, sp));
                     DISPATCH();
+                }
 
-                ENTRY(MP_BC_MAKE_CLOSURE_DEFARGS):
+                ENTRY(MP_BC_MAKE_CLOSURE_DEFARGS): {
                     DECODE_PTR;
-                    // Stack layout: def_tuple def_dict closure_tuple <- TOS
-                    obj1 = POP();
-                    obj2 = POP();
-                    SET_TOP(mp_make_closure_from_raw_code((mp_raw_code_t*)unum, obj1, TOP(), obj2));
+                    machine_uint_t n_closed_over = *ip++;
+                    // Stack layout: def_tuple def_dict closed_overs <- TOS
+                    sp -= 2 + n_closed_over - 1;
+                    SET_TOP(mp_make_closure_from_raw_code((mp_raw_code_t*)unum, 0x100 | n_closed_over, sp));
                     DISPATCH();
+                }
 
                 ENTRY(MP_BC_CALL_FUNCTION):
                     DECODE_UINT;