py: Remove DELETE_SUBSCR opcode, combine with STORE_SUBSCR.

dpgeorge · dpgeorge · commit f4c9b33abf0a · 2014-04-08T21:32:29.000+01:00
This makes the runtime and object APIs more consistent.  mp_store_subscr
functionality now moved into objects (ie list and dict store_item).
diff --git a/py/bc0.h b/py/bc0.h
@@ -40,7 +40,6 @@
 
 #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)
diff --git a/py/emitbc.c b/py/emitbc.c
@@ -37,6 +37,7 @@ struct _emit_t {
 };
 
 STATIC void emit_bc_rot_two(emit_t *emit);
+STATIC void emit_bc_rot_three(emit_t *emit);
 
 emit_t *emit_bc_new(uint max_num_labels) {
     emit_t *emit = m_new0(emit_t, 1);
@@ -515,8 +516,9 @@ STATIC void emit_bc_delete_attr(emit_t *emit, qstr qstr) {
 }
 
 STATIC void emit_bc_delete_subscr(emit_t *emit) {
-    emit_bc_pre(emit, -2);
-    emit_write_byte_code_byte(emit, MP_BC_DELETE_SUBSCR);
+    emit_bc_load_null(emit);
+    emit_bc_rot_three(emit);
+    emit_bc_store_subscr(emit);
 }
 
 STATIC void emit_bc_dup_top(emit_t *emit) {
diff --git a/py/obj.h b/py/obj.h
@@ -163,8 +163,8 @@ typedef mp_obj_t (*mp_call_fun_t)(mp_obj_t fun, uint n_args, uint n_kw, const mp
 typedef mp_obj_t (*mp_unary_op_fun_t)(int op, mp_obj_t);
 typedef mp_obj_t (*mp_binary_op_fun_t)(int op, mp_obj_t, mp_obj_t);
 typedef void (*mp_load_attr_fun_t)(mp_obj_t self_in, qstr attr, mp_obj_t *dest); // for fail, do nothing; for attr, dest[0] = value; for method, dest[0] = method, dest[1] = self
-typedef bool (*mp_store_attr_fun_t)(mp_obj_t self_in, qstr attr, mp_obj_t value); // return true if store succeeded
-typedef bool (*mp_store_item_fun_t)(mp_obj_t self_in, mp_obj_t index, mp_obj_t value); // return true if store succeeded
+typedef bool (*mp_store_attr_fun_t)(mp_obj_t self_in, qstr attr, mp_obj_t value); // return true if store succeeded; if value==MP_OBJ_NULL then delete
+typedef bool (*mp_store_item_fun_t)(mp_obj_t self_in, mp_obj_t index, mp_obj_t value); // return true if store succeeded; if value==MP_OBJ_NULL then delete
 
 typedef struct _mp_method_t {
     qstr name;
@@ -218,8 +218,9 @@ struct _mp_obj_type_t {
 
     mp_load_attr_fun_t load_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)
+
+    // Implements container[index] = val.  If val == MP_OBJ_NULL, then it's a delete.
+    // Note that load_item is implemented by binary_op(RT_BINARY_OP_SUBSCR)
     mp_store_item_fun_t store_item;
 
     mp_fun_1_t getiter;
diff --git a/py/objarray.c b/py/objarray.c
@@ -143,6 +143,10 @@ STATIC mp_obj_t array_append(mp_obj_t self_in, mp_obj_t arg) {
 STATIC MP_DEFINE_CONST_FUN_OBJ_2(array_append_obj, array_append);
 
 STATIC bool array_store_item(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value) {
+    if (value == MP_OBJ_NULL) {
+        // delete item; does this need to be implemented?
+        return false;
+    }
     mp_obj_array_t *o = self_in;
     uint index = mp_get_index(o->base.type, o->len, index_in, false);
     mp_binary_set_val(o->typecode, o->items, index, value);
diff --git a/py/objdict.c b/py/objdict.c
@@ -114,6 +114,14 @@ STATIC mp_obj_t dict_binary_op(int op, mp_obj_t lhs_in, mp_obj_t rhs_in) {
     }
 }
 
+STATIC bool dict_store_item(mp_obj_t self_in, mp_obj_t index, mp_obj_t value) {
+    if (value == MP_OBJ_NULL) {
+        mp_obj_dict_delete(self_in, index);
+    } else {
+        mp_obj_dict_store(self_in, index, value);
+    }
+    return true;
+}
 
 /******************************************************************************/
 /* dict iterator                                                              */
@@ -484,6 +492,7 @@ const mp_obj_type_t mp_type_dict = {
     .make_new = dict_make_new,
     .unary_op = dict_unary_op,
     .binary_op = dict_binary_op,
+    .store_item = dict_store_item,
     .getiter = dict_getiter,
     .locals_dict = (mp_obj_t)&dict_locals_dict,
 };
diff --git a/py/objlist.c b/py/objlist.c
@@ -19,6 +19,7 @@ typedef struct _mp_obj_list_t {
 STATIC mp_obj_t mp_obj_new_list_iterator(mp_obj_list_t *list, int cur);
 STATIC mp_obj_list_t *list_new(uint n);
 STATIC mp_obj_t list_extend(mp_obj_t self_in, mp_obj_t arg_in);
+STATIC mp_obj_t list_pop(uint n_args, const mp_obj_t *args);
 
 // TODO: Move to mpconfig.h
 #define LIST_MIN_ALLOC 4
@@ -146,6 +147,16 @@ STATIC mp_obj_t list_binary_op(int op, mp_obj_t lhs, mp_obj_t rhs) {
     }
 }
 
+STATIC bool list_store_item(mp_obj_t self_in, mp_obj_t index, mp_obj_t value) {
+    if (value == MP_OBJ_NULL) {
+        mp_obj_t args[2] = {self_in, index};
+        list_pop(2, args);
+    } else {
+        mp_obj_list_store(self_in, index, value);
+    }
+    return true;
+}
+
 STATIC mp_obj_t list_getiter(mp_obj_t o_in) {
     return mp_obj_new_list_iterator(o_in, 0);
 }
@@ -349,6 +360,7 @@ const mp_obj_type_t mp_type_list = {
     .make_new = list_make_new,
     .unary_op = list_unary_op,
     .binary_op = list_binary_op,
+    .store_item = list_store_item,
     .getiter = list_getiter,
     .locals_dict = (mp_obj_t)&list_locals_dict,
 };
diff --git a/py/objtype.c b/py/objtype.c
@@ -269,6 +269,11 @@ STATIC bool class_store_attr(mp_obj_t self_in, qstr attr, mp_obj_t value) {
 }
 
 bool class_store_item(mp_obj_t self_in, mp_obj_t index, mp_obj_t value) {
+    if (value == MP_OBJ_NULL) {
+        // delete item
+        // TODO implement me!
+        return false;
+    }
     mp_obj_class_t *self = self_in;
     mp_obj_t member = mp_obj_class_lookup(self->base.type, MP_QSTR___setitem__);
     if (member != MP_OBJ_NULL) {
diff --git a/py/runtime.c b/py/runtime.c
@@ -841,37 +841,18 @@ void mp_store_attr(mp_obj_t base, qstr attr, mp_obj_t value) {
 
 void mp_store_subscr(mp_obj_t base, mp_obj_t index, mp_obj_t value) {
     DEBUG_OP_printf("store subscr %p[%p] <- %p\n", base, index, value);
-    if (MP_OBJ_IS_TYPE(base, &mp_type_list)) {
-        // list store
-        mp_obj_list_store(base, index, value);
-    } else if (MP_OBJ_IS_TYPE(base, &mp_type_dict)) {
-        // dict store
-        mp_obj_dict_store(base, index, value);
-    } else {
-        mp_obj_type_t *type = mp_obj_get_type(base);
-        if (type->store_item != NULL) {
-            bool r = type->store_item(base, index, value);
-            if (r) {
-                return;
-            }
-            // TODO: call base classes here?
+    mp_obj_type_t *type = mp_obj_get_type(base);
+    if (type->store_item != NULL) {
+        bool r = type->store_item(base, index, value);
+        if (r) {
+            return;
         }
-        nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_TypeError, "'%s' object does not support item assignment", mp_obj_get_type_str(base)));
+        // TODO: call base classes here?
     }
-}
-
-void mp_delete_subscr(mp_obj_t base, mp_obj_t index) {
-    DEBUG_OP_printf("delete subscr %p[%p]\n", base, index);
-    /* list delete not implemented
-    if (MP_OBJ_IS_TYPE(base, &mp_type_list)) {
-        // list delete
-        mp_obj_list_delete(base, index);
-    } else */
-    if (MP_OBJ_IS_TYPE(base, &mp_type_dict)) {
-        // dict delete
-        mp_obj_dict_delete(base, index);
-    } else {
+    if (value == MP_OBJ_NULL) {
         nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_TypeError, "'%s' object does not support item deletion", mp_obj_get_type_str(base)));
+    } else {
+        nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_TypeError, "'%s' object does not support item assignment", mp_obj_get_type_str(base)));
     }
 }
 
diff --git a/py/vm.c b/py/vm.c
@@ -324,11 +324,6 @@ mp_vm_return_kind_t mp_execute_byte_code_2(const byte *code_info, const byte **i
                         mp_delete_global(qst);
                         break;
 
-                    case MP_BC_DELETE_SUBSCR:
-                        mp_delete_subscr(sp[-1], sp[0]);
-                        sp -= 2;
-                        break;
-
                     case MP_BC_DUP_TOP:
                         obj1 = TOP();
                         PUSH(obj1);
diff --git a/tests/basics/del-attr.py b/tests/basics/del-attr.py
@@ -0,0 +1,28 @@
+# del a class attribute
+
+del C.f
+try:
+    print(C.x)
+except AttributeError:
+    print("AttributeError")
+try:
+    del C.f
+except AttributeError:
+    print("AttributeError")
+
+# del an instance attribute
+
+c = C()
+
+c.x = 1
+print(c.x)
+
+del c.x
+try:
+    print(c.x)
+except AttributeError:
+    print("AttributeError")
+try:
+    del c.x
+except AttributeError:
+    print("AttributeError")
diff --git a/tests/basics/del-name.py b/tests/basics/del-name.py
@@ -0,0 +1,18 @@
+# del global
+
+x = 1
+print(x)
+del x
+try:
+    print(x)
+except NameError:
+    print("NameError")
+try:
+    del x
+except: # NameError:
+    # FIXME uPy returns KeyError for this
+    print("NameError")
+
+class C:
+    def f():
+        pass
diff --git a/tests/basics/del-subscr.py b/tests/basics/del-subscr.py
@@ -0,0 +1,13 @@
+l = [1, 2, 3]
+print(l)
+del l[0]
+print(l)
+del l[-1]
+print(l)
+
+d = {1:2, 3:4, 5:6}
+del d[1]
+del d[3]
+print(d)
+del d[5]
+print(d)
