py/objdict: Fix popitem for ordered dicts.

The popitem method wasn't implemented for ordered dicts and would result in
an invalid state.

Fixes issue #5956.
This commit is contained in:
Jim Mussared 2020-04-23 01:10:30 +10:00 committed by Damien George
parent 347c8917dc
commit 57fce3bdb2
4 changed files with 36 additions and 4 deletions

View File

@ -177,6 +177,7 @@ mp_map_elem_t *mp_map_lookup(mp_map_t *map, mp_obj_t index, mp_map_lookup_kind_t
--map->used; --map->used;
memmove(elem, elem + 1, (top - elem - 1) * sizeof(*elem)); memmove(elem, elem + 1, (top - elem - 1) * sizeof(*elem));
// put the found element after the end so the caller can access it if needed // put the found element after the end so the caller can access it if needed
// note: caller must NULL the value so the GC can clean up (e.g. see dict_get_helper).
elem = &map->table[map->used]; elem = &map->table[map->used];
elem->key = MP_OBJ_NULL; elem->key = MP_OBJ_NULL;
elem->value = value; elem->value = value;

View File

@ -26,6 +26,8 @@
#ifndef MICROPY_INCLUDED_PY_OBJ_H #ifndef MICROPY_INCLUDED_PY_OBJ_H
#define MICROPY_INCLUDED_PY_OBJ_H #define MICROPY_INCLUDED_PY_OBJ_H
#include <assert.h>
#include "py/mpconfig.h" #include "py/mpconfig.h"
#include "py/misc.h" #include "py/misc.h"
#include "py/qstr.h" #include "py/qstr.h"
@ -423,6 +425,7 @@ typedef enum _mp_map_lookup_kind_t {
extern const mp_map_t mp_const_empty_map; extern const mp_map_t mp_const_empty_map;
static inline bool mp_map_slot_is_filled(const mp_map_t *map, size_t pos) { static inline bool mp_map_slot_is_filled(const mp_map_t *map, size_t pos) {
assert(pos < map->alloc);
return (map)->table[pos].key != MP_OBJ_NULL && (map)->table[pos].key != MP_OBJ_SENTINEL; return (map)->table[pos].key != MP_OBJ_NULL && (map)->table[pos].key != MP_OBJ_SENTINEL;
} }

View File

@ -44,13 +44,15 @@ STATIC mp_map_elem_t *dict_iter_next(mp_obj_dict_t *dict, size_t *cur) {
size_t max = dict->map.alloc; size_t max = dict->map.alloc;
mp_map_t *map = &dict->map; mp_map_t *map = &dict->map;
for (size_t i = *cur; i < max; i++) { size_t i = *cur;
for (; i < max; i++) {
if (mp_map_slot_is_filled(map, i)) { if (mp_map_slot_is_filled(map, i)) {
*cur = i + 1; *cur = i + 1;
return &(map->table[i]); return &(map->table[i]);
} }
} }
assert(map->used == 0 || i == max);
return NULL; return NULL;
} }
@ -321,11 +323,17 @@ STATIC mp_obj_t dict_popitem(mp_obj_t self_in) {
mp_check_self(mp_obj_is_dict_type(self_in)); mp_check_self(mp_obj_is_dict_type(self_in));
mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in); mp_obj_dict_t *self = MP_OBJ_TO_PTR(self_in);
mp_ensure_not_fixed(self); mp_ensure_not_fixed(self);
size_t cur = 0; if (self->map.used == 0) {
mp_map_elem_t *next = dict_iter_next(self, &cur);
if (next == NULL) {
mp_raise_msg(&mp_type_KeyError, MP_ERROR_TEXT("popitem(): dictionary is empty")); mp_raise_msg(&mp_type_KeyError, MP_ERROR_TEXT("popitem(): dictionary is empty"));
} }
size_t cur = 0;
#if MICROPY_PY_COLLECTIONS_ORDEREDDICT
if (self->map.is_ordered) {
cur = self->map.used - 1;
}
#endif
mp_map_elem_t *next = dict_iter_next(self, &cur);
assert(next);
self->map.used--; self->map.used--;
mp_obj_t items[] = {next->key, next->value}; mp_obj_t items[] = {next->key, next->value};
next->key = MP_OBJ_SENTINEL; // must mark key as sentinel to indicate that it was deleted next->key = MP_OBJ_SENTINEL; // must mark key as sentinel to indicate that it was deleted

View File

@ -24,3 +24,23 @@ d["abc"] = 123
print(len(d)) print(len(d))
print(list(d.keys())) print(list(d.keys()))
print(list(d.values())) print(list(d.values()))
# pop an element
print(d.popitem())
print(len(d))
print(list(d.keys()))
print(list(d.values()))
# add an element after popping
d["xyz"] = 321
print(len(d))
print(list(d.keys()))
print(list(d.values()))
# pop until empty
print(d.popitem())
print(d.popitem())
try:
d.popitem()
except:
print('empty')