From 1d24ea5207ba4b62b20dbba22ab2800496418463 Mon Sep 17 00:00:00 2001
From: Damien George <damien.p.george@gmail.com>
Date: Tue, 8 Apr 2014 21:11:49 +0100
Subject: [PATCH] py: Finish implementation of all del opcodes.

At this point, all opcodes are now implemented!

Some del opcodes have been combined with store opcodes, with the value
to store being MP_OBJ_NULL.
---
 py/bc0.h       | 14 +++++++-------
 py/emitbc.c    | 16 +++++++++-------
 py/obj.h       |  2 +-
 py/objmodule.c | 10 ++++++++--
 py/objtype.c   | 29 ++++++++++++++++++++++-------
 py/runtime.c   | 10 ++++++++--
 py/runtime.h   |  1 +
 py/showbc.c    |  5 -----
 py/vm.c        | 10 +++++-----
 9 files changed, 61 insertions(+), 36 deletions(-)

diff --git a/py/bc0.h b/py/bc0.h
index 80a8248de..0fe33d48a 100644
--- a/py/bc0.h
+++ b/py/bc0.h
@@ -1,6 +1,10 @@
 // Micro Python byte-codes.
 // The comment at the end of the line (if it exists) tells the arguments to the byte-code.
 
+// TODO Add MP_BC_LOAD_FAST_CHECKED and MP_BC_STORE_FAST_CHECKED for acting on those
+// locals which have del called on them anywhere in the function.
+// UnboundLocalError: local variable '%s' referenced before assignment
+
 #define MP_BC_LOAD_CONST_FALSE   (0x10)
 #define MP_BC_LOAD_CONST_NONE    (0x11)
 #define MP_BC_LOAD_CONST_TRUE    (0x12)
@@ -34,12 +38,9 @@
 #define MP_BC_STORE_ATTR         (0x37) // qstr
 #define MP_BC_STORE_SUBSCR       (0x38)
 
-#define MP_BC_DELETE_FAST_N      (0x39) // uint
-#define MP_BC_DELETE_DEREF       (0x3a) // uint
-#define MP_BC_DELETE_NAME        (0x3b) // qstr
-#define MP_BC_DELETE_GLOBAL      (0x3c) // qstr
-#define MP_BC_DELETE_ATTR        (0x3d) // qstr
-#define MP_BC_DELETE_SUBSCR      (0x3e)
+#define MP_BC_DELETE_NAME        (0x39) // qstr
+#define MP_BC_DELETE_GLOBAL      (0x3a) // qstr
+#define MP_BC_DELETE_SUBSCR      (0x3b)
 
 #define MP_BC_DUP_TOP            (0x40)
 #define MP_BC_DUP_TOP_TWO        (0x41)
@@ -97,4 +98,3 @@
 #define MP_BC_IMPORT_NAME        (0xe0) // qstr
 #define MP_BC_IMPORT_FROM        (0xe1) // qstr
 #define MP_BC_IMPORT_STAR        (0xe2)
-
diff --git a/py/emitbc.c b/py/emitbc.c
index 11519fdd8..2a390a056 100644
--- a/py/emitbc.c
+++ b/py/emitbc.c
@@ -36,6 +36,8 @@ struct _emit_t {
     byte dummy_data[8];
 };
 
+STATIC void emit_bc_rot_two(emit_t *emit);
+
 emit_t *emit_bc_new(uint max_num_labels) {
     emit_t *emit = m_new0(emit_t, 1);
     emit->max_num_labels = max_num_labels;
@@ -487,14 +489,13 @@ STATIC void emit_bc_store_subscr(emit_t *emit) {
 }
 
 STATIC void emit_bc_delete_fast(emit_t *emit, qstr qstr, int local_num) {
-    assert(local_num >= 0);
-    emit_bc_pre(emit, 0);
-    emit_write_byte_code_byte_uint(emit, MP_BC_DELETE_FAST_N, local_num);
+    emit_bc_load_null(emit);
+    emit_bc_store_fast(emit, qstr, local_num);
 }
 
 STATIC void emit_bc_delete_deref(emit_t *emit, qstr qstr, int local_num) {
-    emit_bc_pre(emit, 0);
-    emit_write_byte_code_byte_qstr(emit, MP_BC_DELETE_DEREF, local_num);
+    emit_bc_load_null(emit);
+    emit_bc_store_deref(emit, qstr, local_num);
 }
 
 STATIC void emit_bc_delete_name(emit_t *emit, qstr qstr) {
@@ -508,8 +509,9 @@ STATIC void emit_bc_delete_global(emit_t *emit, qstr qstr) {
 }
 
 STATIC void emit_bc_delete_attr(emit_t *emit, qstr qstr) {
-    emit_bc_pre(emit, -1);
-    emit_write_byte_code_byte_qstr(emit, MP_BC_DELETE_ATTR, qstr);
+    emit_bc_load_null(emit);
+    emit_bc_rot_two(emit);
+    emit_bc_store_attr(emit, qstr);
 }
 
 STATIC void emit_bc_delete_subscr(emit_t *emit) {
diff --git a/py/obj.h b/py/obj.h
index 77cf7838e..ee37c8905 100644
--- a/py/obj.h
+++ b/py/obj.h
@@ -217,7 +217,7 @@ struct _mp_obj_type_t {
     mp_binary_op_fun_t binary_op;   // can return NULL if op not supported
 
     mp_load_attr_fun_t load_attr;
-    mp_store_attr_fun_t store_attr;
+    mp_store_attr_fun_t store_attr; // if value is MP_OBJ_NULL, then delete that attribute
     // Implements container[index] = val; note that load_item is implemented
     // by binary_op(RT_BINARY_OP_SUBSCR)
     mp_store_item_fun_t store_item;
diff --git a/py/objmodule.c b/py/objmodule.c
index df7c991b8..15ccd68ac 100644
--- a/py/objmodule.c
+++ b/py/objmodule.c
@@ -27,8 +27,14 @@ STATIC void module_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) {
 
 STATIC bool module_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) {
     mp_obj_module_t *self = self_in;
-    // TODO CPython allows STORE_ATTR to a module, but is this the correct implementation?
-    mp_obj_dict_store(self->globals, MP_OBJ_NEW_QSTR(attr), value);
+    if (value == MP_OBJ_NULL) {
+        // delete attribute
+        mp_obj_dict_delete(self->globals, MP_OBJ_NEW_QSTR(attr));
+    } else {
+        // store attribute
+        // TODO CPython allows STORE_ATTR to a module, but is this the correct implementation?
+        mp_obj_dict_store(self->globals, MP_OBJ_NEW_QSTR(attr), value);
+    }
     return true;
 }
 
diff --git a/py/objtype.c b/py/objtype.c
index 92f1f5703..a2094f671 100644
--- a/py/objtype.c
+++ b/py/objtype.c
@@ -257,8 +257,15 @@ STATIC void class_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) {
 
 STATIC bool class_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) {
     mp_obj_class_t *self = self_in;
-    mp_map_lookup(&self->members, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_ADD_IF_NOT_FOUND)->value = value;
-    return true;
+    if (value == MP_OBJ_NULL) {
+        // delete attribute
+        mp_map_elem_t *el = mp_map_lookup(&self->members, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_REMOVE_IF_FOUND);
+        return el != NULL;
+    } else {
+        // store attribute
+        mp_map_lookup(&self->members, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_ADD_IF_NOT_FOUND)->value = value;
+        return true;
+    }
 }
 
 bool class_store_item(mp_obj_t self_in, mp_obj_t index, mp_obj_t value) {
@@ -356,11 +363,19 @@ STATIC bool type_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) {
     if (self->locals_dict != NULL) {
         assert(MP_OBJ_IS_TYPE(self->locals_dict, &mp_type_dict)); // Micro Python restriction, for now
         mp_map_t *locals_map = mp_obj_dict_get_map(self->locals_dict);
-        mp_map_elem_t *elem = mp_map_lookup(locals_map, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_ADD_IF_NOT_FOUND);
-        // note that locals_map may be in ROM, so add will fail in that case
-        if (elem != NULL) {
-            elem->value = value;
-            return true;
+        if (value == MP_OBJ_NULL) {
+            // delete attribute
+            mp_map_elem_t *elem = mp_map_lookup(locals_map, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_REMOVE_IF_FOUND);
+            // note that locals_map may be in ROM, so remove will fail in that case
+            return elem != NULL;
+        } else {
+            // store attribute
+            mp_map_elem_t *elem = mp_map_lookup(locals_map, MP_OBJ_NEW_QSTR(attr), MP_MAP_LOOKUP_ADD_IF_NOT_FOUND);
+            // note that locals_map may be in ROM, so add will fail in that case
+            if (elem != NULL) {
+                elem->value = value;
+                return true;
+            }
         }
     }
 
diff --git a/py/runtime.c b/py/runtime.c
index 3d1ae72c2..3400d8dc8 100644
--- a/py/runtime.c
+++ b/py/runtime.c
@@ -132,8 +132,8 @@ void mp_store_name(qstr qstr, mp_obj_t obj) {
 
 void mp_delete_name(qstr qstr) {
     DEBUG_OP_printf("delete name %s\n", qstr_str(qstr));
-    // TODO raise NameError if qstr not found
-    mp_map_lookup(&dict_locals->map, MP_OBJ_NEW_QSTR(qstr), MP_MAP_LOOKUP_REMOVE_IF_FOUND);
+    // TODO convert KeyError to NameError if qstr not found
+    mp_obj_dict_delete(dict_locals, MP_OBJ_NEW_QSTR(qstr));
 }
 
 void mp_store_global(qstr qstr, mp_obj_t obj) {
@@ -141,6 +141,12 @@ void mp_store_global(qstr qstr, mp_obj_t obj) {
     mp_obj_dict_store(dict_globals, MP_OBJ_NEW_QSTR(qstr), obj);
 }
 
+void mp_delete_global(qstr qstr) {
+    DEBUG_OP_printf("delete global %s\n", qstr_str(qstr));
+    // TODO convert KeyError to NameError if qstr not found
+    mp_obj_dict_delete(dict_globals, MP_OBJ_NEW_QSTR(qstr));
+}
+
 mp_obj_t mp_unary_op(int op, mp_obj_t arg) {
     DEBUG_OP_printf("unary %d %p\n", op, arg);
 
diff --git a/py/runtime.h b/py/runtime.h
index ab34be2da..4fc381852 100644
--- a/py/runtime.h
+++ b/py/runtime.h
@@ -20,6 +20,7 @@ mp_obj_t mp_load_build_class(void);
 void mp_store_name(qstr qstr, mp_obj_t obj);
 void mp_store_global(qstr qstr, mp_obj_t obj);
 void mp_delete_name(qstr qstr);
+void mp_delete_global(qstr qstr);
 
 mp_obj_t mp_unary_op(int op, mp_obj_t arg);
 mp_obj_t mp_binary_op(int op, mp_obj_t lhs, mp_obj_t rhs);
diff --git a/py/showbc.c b/py/showbc.c
index 823769c0e..c56620435 100644
--- a/py/showbc.c
+++ b/py/showbc.c
@@ -193,11 +193,6 @@ void mp_byte_code_print(const byte *ip, int len) {
                 printf("STORE_SUBSCR");
                 break;
 
-            case MP_BC_DELETE_FAST_N:
-                DECODE_UINT;
-                printf("DELETE_FAST_N " UINT_FMT, unum);
-                break;
-
             case MP_BC_DELETE_NAME:
                 DECODE_QSTR;
                 printf("DELETE_NAME %s", qstr_str(qstr));
diff --git a/py/vm.c b/py/vm.c
index 869a9381a..474a1aeaf 100644
--- a/py/vm.c
+++ b/py/vm.c
@@ -314,16 +314,16 @@ dispatch_loop:
                         sp -= 3;
                         break;
 
-                    case MP_BC_DELETE_FAST_N:
-                        DECODE_UINT;
-                        fastn[-unum] = MP_OBJ_NULL;
-                        break;
-
                     case MP_BC_DELETE_NAME:
                         DECODE_QSTR;
                         mp_delete_name(qst);
                         break;
 
+                    case MP_BC_DELETE_GLOBAL:
+                        DECODE_QSTR;
+                        mp_delete_global(qst);
+                        break;
+
                     case MP_BC_DELETE_SUBSCR:
                         mp_delete_subscr(sp[-1], sp[0]);
                         sp -= 2;
-- 
GitLab