Fix subclassing dict

The get, set and del item methods didn't correctly lookup the value
from the parent native instance because the functions took the type
from the instance.

Fixes #8758

Author: tannewt

py/objdict.c

@@ -49,6 +49,22 @@ const mp_obj_dict_t mp_const_empty_dict_obj = {
     }
 };
 
+// CIRCUITPY-CHANGE: Native methods are passed the subclass instance so they can
+// refer to subclass members. Dict only cares about the native struct so this
+// function gets it.
+STATIC mp_obj_dict_t *native_dict(mp_obj_t self_in) {
+    // Check for OrderedDict first because it is marked as a subclass of dict. However, it doesn't
+    // store its state in subobj like python types to native types do.
+    mp_obj_t native_instance = MP_OBJ_NULL;
+    #if MICROPY_PY_COLLECTIONS_ORDEREDDICT
+    native_instance = mp_obj_cast_to_native_base(self_in, MP_OBJ_FROM_PTR(&mp_type_ordereddict));
+    #endif
+    if (native_instance == MP_OBJ_NULL) {
+        native_instance = mp_obj_cast_to_native_base(self_in, MP_OBJ_FROM_PTR(&mp_type_dict));
+    }
+    return MP_OBJ_TO_PTR(native_instance);
+}
+
 STATIC mp_obj_t dict_update(size_t n_args, const mp_obj_t *args, mp_map_t *kwargs);
 
 // This is a helper function to iterate through a dictionary.  The state of
@@ -71,7 +87,7 @@ STATIC mp_map_elem_t *dict_iter_next(mp_obj_dict_t *dict, size_t *cur) {
 }
 
 STATIC void dict_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) {
-    mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in);
+    mp_obj_dict_t *self = native_dict(self_in);
     bool first = true;
     const char *item_separator = ", ";
     const char *key_separator = ": ";
@@ -144,7 +160,7 @@ mp_obj_t mp_obj_dict_make_new(const mp_obj_type_t *type, size_t n_args, size_t n
 }
 
 STATIC mp_obj_t dict_unary_op(mp_unary_op_t op, mp_obj_t self_in) {
-    mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in);
+    mp_obj_dict_t *self = native_dict(self_in);
     switch (op) {
         case MP_UNARY_OP_BOOL:
             return mp_obj_new_bool(self->map.used != 0);
@@ -162,7 +178,7 @@ STATIC mp_obj_t dict_unary_op(mp_unary_op_t op, mp_obj_t self_in) {
 }
 
 STATIC mp_obj_t dict_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs_in) {
-    mp_obj_dict_t *o = MP_OBJ_TO_PTR(lhs_in);
+    mp_obj_dict_t *o = native_dict(lhs_in);
     switch (op) {
         case MP_BINARY_OP_CONTAINS: {
             mp_map_elem_t *elem = mp_map_lookup(&o->map, rhs_in, MP_MAP_LOOKUP);
@@ -223,7 +239,7 @@ STATIC mp_obj_t dict_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs_
 
 // Note: Make sure this is inlined in load part of dict_subscr() below.
 mp_obj_t mp_obj_dict_get(mp_obj_t self_in, mp_obj_t index) {
-    mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in);
+    mp_obj_dict_t *self = native_dict(self_in);
     mp_map_elem_t *elem = mp_map_lookup(&self->map, index, MP_MAP_LOOKUP);
     if (elem == NULL) {
         mp_raise_type_arg(&mp_type_KeyError, index);
@@ -239,7 +255,7 @@ STATIC mp_obj_t dict_subscr(mp_obj_t self_in, mp_obj_t index, mp_obj_t value) {
         return mp_const_none;
     } else if (value == MP_OBJ_SENTINEL) {
         // load
-        mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in);
+        mp_obj_dict_t *self = native_dict(self_in);
         mp_map_elem_t *elem = mp_map_lookup(&self->map, index, MP_MAP_LOOKUP);
         if (elem == NULL) {
             mp_raise_type_arg(&mp_type_KeyError, index);
@@ -264,7 +280,7 @@ STATIC void PLACE_IN_ITCM(mp_ensure_not_fixed)(const mp_obj_dict_t * dict) {
 
 STATIC mp_obj_t dict_clear(mp_obj_t self_in) {
     mp_check_self(mp_obj_is_dict_or_ordereddict(self_in));
-    mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in);
+    mp_obj_dict_t *self = native_dict(self_in);
     mp_ensure_not_fixed(self);
 
     mp_map_clear(&self->map);
@@ -275,9 +291,9 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(dict_clear_obj, dict_clear);
 
 mp_obj_t mp_obj_dict_copy(mp_obj_t self_in) {
     mp_check_self(mp_obj_is_dict_or_ordereddict(self_in));
-    mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in);
+    mp_obj_dict_t *self = native_dict(self_in);
     mp_obj_t other_out = mp_obj_new_dict(self->map.alloc);
-    mp_obj_dict_t *other = MP_OBJ_TO_PTR(other_out);
+    mp_obj_dict_t *other = native_dict(other_out);
     other->base.type = self->base.type;
     other->map.used = self->map.used;
     other->map.all_keys_are_qstrs = self->map.all_keys_are_qstrs;
@@ -324,7 +340,7 @@ STATIC MP_DEFINE_CONST_CLASSMETHOD_OBJ(dict_fromkeys_obj, MP_ROM_PTR(&dict_fromk
 
 STATIC mp_obj_t dict_get_helper(size_t n_args, const mp_obj_t *args, mp_map_lookup_kind_t lookup_kind) {
     mp_check_self(mp_obj_is_dict_or_ordereddict(args[0]));
-    mp_obj_dict_t *self = MP_OBJ_TO_PTR(args[0]);
+    mp_obj_dict_t *self = native_dict(args[0]);
     if (lookup_kind != MP_MAP_LOOKUP) {
         mp_ensure_not_fixed(self);
     }
@@ -369,7 +385,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(dict_setdefault_obj, 2, 3, dict_setde
 
 STATIC mp_obj_t dict_popitem(mp_obj_t self_in) {
     mp_check_self(mp_obj_is_dict_or_ordereddict(self_in));
-    mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in);
+    mp_obj_dict_t *self = native_dict(self_in);
     mp_ensure_not_fixed(self);
     if (self->map.used == 0) {
         mp_raise_msg_varg(&mp_type_KeyError, MP_ERROR_TEXT("pop from empty %q"), MP_QSTR_dict);
@@ -394,7 +410,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(dict_popitem_obj, dict_popitem);
 
 STATIC mp_obj_t dict_update(size_t n_args, const mp_obj_t *args, mp_map_t *kwargs) {
     mp_check_self(mp_obj_is_dict_or_ordereddict(args[0]));
-    mp_obj_dict_t *self = MP_OBJ_TO_PTR(args[0]);
+    mp_obj_dict_t *self = native_dict(args[0]);
     mp_ensure_not_fixed(self);
 
     mp_arg_check_num(n_args, kwargs->used, 1, 2, true);
@@ -726,7 +742,7 @@ size_t mp_obj_dict_len(mp_obj_t self_in) {
 
 mp_obj_t mp_obj_dict_store(mp_obj_t self_in, mp_obj_t key, mp_obj_t value) {
     mp_check_self(mp_obj_is_dict_or_ordereddict(self_in));
-    mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in);
+    mp_obj_dict_t *self = native_dict(self_in);
     mp_ensure_not_fixed(self);
     mp_map_lookup(&self->map, key, MP_MAP_LOOKUP_ADD_IF_NOT_FOUND)->value = value;
     return self_in;

py/opmethods.c

@@ -27,22 +27,26 @@
 #include "py/obj.h"
 #include "py/builtin.h"
 
+// CIRCUITPY-CHANGE: These three functions are used by dict only. In CP, we hard
+// code the type to dict so that subclassed types still use the native dict
+// subscr. MP doesn't have this problem because it passes the native instance
+// in. CP passes the subclass instance.
 STATIC mp_obj_t op_getitem(mp_obj_t self_in, mp_obj_t key_in) {
-    const mp_obj_type_t *type = mp_obj_get_type(self_in);
+    const mp_obj_type_t *type = &mp_type_dict;
     // Note: assumes type must have subscr (only used by dict).
     return MP_OBJ_TYPE_GET_SLOT(type, subscr)(self_in, key_in, MP_OBJ_SENTINEL);
 }
 MP_DEFINE_CONST_FUN_OBJ_2(mp_op_getitem_obj, op_getitem);
 
 STATIC mp_obj_t op_setitem(mp_obj_t self_in, mp_obj_t key_in, mp_obj_t value_in) {
-    const mp_obj_type_t *type = mp_obj_get_type(self_in);
+    const mp_obj_type_t *type = &mp_type_dict;
     // Note: assumes type must have subscr (only used by dict).
     return MP_OBJ_TYPE_GET_SLOT(type, subscr)(self_in, key_in, value_in);
 }
 MP_DEFINE_CONST_FUN_OBJ_3(mp_op_setitem_obj, op_setitem);
 
 STATIC mp_obj_t op_delitem(mp_obj_t self_in, mp_obj_t key_in) {
-    const mp_obj_type_t *type = mp_obj_get_type(self_in);
+    const mp_obj_type_t *type = &mp_type_dict;
     // Note: assumes type must have subscr (only used by dict).
     return MP_OBJ_TYPE_GET_SLOT(type, subscr)(self_in, key_in, MP_OBJ_NULL);
 }

tests/basics/subclass_native_dict.py

@@ -0,0 +1,28 @@
+class a:
+    def __init__(self):
+        self.d = {}
+
+    def __setitem__(self, k, v):
+        print("a", k, v)
+        self.d[k] = v
+
+    def __getitem__(self, k):
+        return self.d[k]
+
+class b(a):
+    def __setitem__(self, k, v):
+        print("b", k, v)
+        super().__setitem__(k, v)
+
+b1 = b()
+b1[1] = 2
+print(b1[1])
+
+class mydict(dict):
+    def __setitem__(self, k, v):
+        print(k, v)
+        super().__setitem__(k, v)
+
+d = mydict()
+d[3] = 4
+print(d[3])