[gc_heap] Respect deletions in Dict::{keys,values}()

oilshell · Nov 29, 2020 · 2e17d3e · 2e17d3e
1 parent 5780a4f
commit 2e17d3e
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 15 deletions.
diff --git a/mycpp/gc_heap.h b/mycpp/gc_heap.h
@@ -715,13 +715,6 @@ class List : public gc_heap::Obj {
     }
   }
 
-  // Internal constructor for keys() and values()
-  List(Slab<T>* from_slab, int n) : List() {
-    reserve(n);
-    len_ = n;
-    memcpy(slab_->items_, from_slab->items_, n * sizeof(T));
-  }
-
   // Implements L[i]
   T index(int i) {
     if (i < len_) {
@@ -893,6 +886,25 @@ const int kDeletedEntry = -1;
 // index_.
 const int kEmptyEntry = -2;
 
+// Helper for keys() and values()
+template <typename T>
+List<T>* ListFromDictSlab(Slab<int>* index, Slab<T>* slab, int n) {
+  // TODO: Reserve the right amount of space
+  List<T>* result = Alloc<List<T>>();
+
+  for (int i = 0; i < n; ++i) {
+    int special = index->items_[i];
+    if (special == kDeletedEntry) {
+      continue;
+    }
+    if (special == kEmptyEntry) {
+      break;
+    }
+    result->append(slab->items_[i]);
+  }
+  return result;
+}
+
 inline bool keys_equal(int left, int right) {
   return left == right;
 }
@@ -1034,15 +1046,13 @@ class Dict : public gc_heap::Obj {
     }
   }
 
-  // TODO: Remove deleted items!
   List<K>* keys() {
-    // Make a copy of the Slab
-    return Alloc<List<K>>(keys_, len_);
+    return ListFromDictSlab<K>(index_, keys_, capacity_);
   }
 
   // For AssocArray transformations
   List<V>* values() {
-    return Alloc<List<V>>(values_, len_);
+    return ListFromDictSlab<V>(index_, values_, capacity_);
   }
 
   void clear() {

diff --git a/mycpp/my_runtime_test.cc b/mycpp/my_runtime_test.cc
@@ -542,11 +542,18 @@ TEST dict_methods_test() {
   ASSERT(str_equals0("key2", keys->index(1)));
   ASSERT(str_equals0("key3", keys->index(2)));
 
+  mylib::dict_remove(d2, NewStr("key"));
+  ASSERT_EQ_FMT(2, len(d2), "%d");
+
+  auto keys2 = d2->keys();
+  ASSERT_EQ_FMT(2, len(keys2), "%d");
+  ASSERT(str_equals0("key2", keys2->index(0)));
+  ASSERT(str_equals0("key3", keys2->index(1)));
+
   auto values = d2->values();
-  ASSERT_EQ_FMT(3, len(values), "%d");
-  ASSERT_EQ(42, values->index(0));
-  ASSERT_EQ(2, values->index(1));
-  ASSERT_EQ(3, values->index(2));
+  ASSERT_EQ_FMT(2, len(values), "%d");
+  ASSERT_EQ(2, values->index(0));
+  ASSERT_EQ(3, values->index(1));
 
   d2->clear();
   ASSERT_EQ(0, len(d2));