Add MapEntry to core to let maps be directly iterated.

Fix #372.

src/vm/wren_core.c

@@ -466,7 +466,7 @@ DEF_PRIMITIVE(map_keyIteratorValue)
   MapEntry* entry = &map->entries[index];
   if (IS_UNDEFINED(entry->key))
   {
-    RETURN_ERROR("Invalid map iterator value.");
+    RETURN_ERROR("Invalid map iterator.");
   }
 
   RETURN_VAL(entry->key);
@@ -481,7 +481,7 @@ DEF_PRIMITIVE(map_valueIteratorValue)
   MapEntry* entry = &map->entries[index];
   if (IS_UNDEFINED(entry->key))
   {
-    RETURN_ERROR("Invalid map iterator value.");
+    RETURN_ERROR("Invalid map iterator.");
   }
 
   RETURN_VAL(entry->value);
@@ -1280,7 +1280,7 @@ void wrenInitializeCore(WrenVM* vm)
   PRIMITIVE(vm->mapClass, "containsKey(_)", map_containsKey);
   PRIMITIVE(vm->mapClass, "count", map_count);
   PRIMITIVE(vm->mapClass, "remove(_)", map_remove);
-  PRIMITIVE(vm->mapClass, "iterate_(_)", map_iterate);
+  PRIMITIVE(vm->mapClass, "iterate(_)", map_iterate);
   PRIMITIVE(vm->mapClass, "keyIteratorValue_(_)", map_keyIteratorValue);
   PRIMITIVE(vm->mapClass, "valueIteratorValue_(_)", map_valueIteratorValue);
 

src/vm/wren_core.wren

@@ -200,7 +200,7 @@ class List is Sequence {
   }
 }
 
-class Map {
+class Map is Sequence {
   keys { MapKeySequence.new(this) }
   values { MapValueSequence.new(this) }
 
@@ -216,6 +216,24 @@ class Map {
 
     return result + "}"
   }
+
+  iteratorValue(iterator) {
+    return MapEntry.new(
+        keyIteratorValue_(iterator),
+        valueIteratorValue_(iterator))
+  }
+}
+
+class MapEntry {
+  construct new(key, value) {
+    _key = key
+    _value = value
+  }
+
+  key { _key }
+  value { _value }
+
+  toString { "%(_key):%(_value)" }
 }
 
 class MapKeySequence is Sequence {
@@ -223,7 +241,7 @@ class MapKeySequence is Sequence {
     _map = map
   }
 
-  iterate(n) { _map.iterate_(n) }
+  iterate(n) { _map.iterate(n) }
   iteratorValue(iterator) { _map.keyIteratorValue_(iterator) }
 }
 
@@ -232,7 +250,7 @@ class MapValueSequence is Sequence {
     _map = map
   }
 
-  iterate(n) { _map.iterate_(n) }
+  iterate(n) { _map.iterate(n) }
   iteratorValue(iterator) { _map.valueIteratorValue_(iterator) }
 }
 

src/vm/wren_core.wren.inc

@@ -202,7 +202,7 @@ static const char* coreModuleSource =
 "  }\n"
 "}\n"
 "\n"
-"class Map {\n"
+"class Map is Sequence {\n"
 "  keys { MapKeySequence.new(this) }\n"
 "  values { MapValueSequence.new(this) }\n"
 "\n"
@@ -218,6 +218,24 @@ static const char* coreModuleSource =
 "\n"
 "    return result + \"}\"\n"
 "  }\n"
+"\n"
+"  iteratorValue(iterator) {\n"
+"    return MapEntry.new(\n"
+"        keyIteratorValue_(iterator),\n"
+"        valueIteratorValue_(iterator))\n"
+"  }\n"
+"}\n"
+"\n"
+"class MapEntry {\n"
+"  construct new(key, value) {\n"
+"    _key = key\n"
+"    _value = value\n"
+"  }\n"
+"\n"
+"  key { _key }\n"
+"  value { _value }\n"
+"\n"
+"  toString { \"%(_key):%(_value)\" }\n"
 "}\n"
 "\n"
 "class MapKeySequence is Sequence {\n"
@@ -225,7 +243,7 @@ static const char* coreModuleSource =
 "    _map = map\n"
 "  }\n"
 "\n"
-"  iterate(n) { _map.iterate_(n) }\n"
+"  iterate(n) { _map.iterate(n) }\n"
 "  iteratorValue(iterator) { _map.keyIteratorValue_(iterator) }\n"
 "}\n"
 "\n"
@@ -234,7 +252,7 @@ static const char* coreModuleSource =
 "    _map = map\n"
 "  }\n"
 "\n"
-"  iterate(n) { _map.iterate_(n) }\n"
+"  iterate(n) { _map.iterate(n) }\n"
 "  iteratorValue(iterator) { _map.valueIteratorValue_(iterator) }\n"
 "}\n"
 "\n"

test/core/map/is_empty.wren

@@ -0,0 +1,2 @@
+System.print({}.isEmpty) // expect: true
+System.print({1: 1}.isEmpty) // expect: false

test/core/map/iterate.wren

@@ -0,0 +1,45 @@
+var a = {"one": 1, "two": 2, "three": 3, "four": 4}
+
+// The precise numeric values aren't defined since they are indexes into the
+// entry table and the hashing process isn't specified. So we just validate
+// what we can assume about them.
+
+System.print(a.iterate(null) is Num) // expect: true
+System.print(a.iterate(null) >= 0) // expect: true
+
+System.print(a.iterate(0) is Num) // expect: true
+System.print(a.iterate(0) > 0) // expect: true
+System.print(a.iterate(1) is Num) // expect: true
+System.print(a.iterate(1) > 0) // expect: true
+System.print(a.iterate(2) is Num) // expect: true
+System.print(a.iterate(2) > 0) // expect: true
+System.print(a.iterate(3) is Num) // expect: true
+System.print(a.iterate(3) > 0) // expect: true
+
+var previous = -1
+var iterator = a.iterate(null)
+while (iterator) {
+  System.print(iterator > previous)
+  System.print(iterator is Num)
+  previous = iterator
+  iterator = a.iterate(iterator)
+}
+// First entry:
+// expect: true
+// expect: true
+// Second entry:
+// expect: true
+// expect: true
+// Third entry:
+// expect: true
+// expect: true
+// Fourth entry:
+// expect: true
+// expect: true
+
+// Out of bounds.
+System.print(a.iterate(16)) // expect: false
+System.print(a.iterate(-1)) // expect: false
+
+// Nothing to iterate in an empty map.
+System.print({}.iterate(null)) // expect: false

test/core/map/iterate_iterator_not_int.wren

@@ -0,0 +1,2 @@
+var a = {1: 2, 3: 4}
+a.iterate(1.5) // expect runtime error: Iterator must be an integer.

test/core/map/iterate_iterator_not_num.wren

@@ -0,0 +1,2 @@
+var a = {1: 2, 3: 4}
+a.iterate("2") // expect runtime error: Iterator must be a number.

test/core/map/iterator_value.wren

@@ -0,0 +1,13 @@
+var a = {1: "one"}
+
+// The actual iterator values are implementation specific, so ask the map.
+var iterator = a.iterate(null)
+var value = a.iteratorValue(iterator)
+
+System.print(value is MapEntry) // expect: true
+System.print(value.key)         // expect: 1
+System.print(value.value)       // expect: one
+
+// The entry does not track the underlying map.
+a[1] = "updated"
+System.print(value.value)       // expect: one

test/core/map/iterator_value_iterator_not_int.wren

@@ -0,0 +1,2 @@
+var a = {1: "one"}
+a.iteratorValue(1.5) // expect runtime error: Iterator must be an integer.

test/core/map/iterator_value_iterator_not_num.wren

@@ -0,0 +1,2 @@
+var a = {1: "one"}
+a.iteratorValue("2") // expect runtime error: Iterator must be a number.

test/core/map/iterator_value_iterator_too_large.wren

@@ -0,0 +1,6 @@
+var a = {1: "one"}
+
+// The maximum value is based on the map's capacity, not its count, so use a
+// sufficiently large enough value for the test to make not affected by growth
+// strategy.
+a.iteratorValue(9999) // expect runtime error: Iterator out of bounds.

test/core/map/iterator_value_iterator_too_small.wren

@@ -0,0 +1,6 @@
+var a = {1: "one"}
+
+// The maximum value is based on the map's capacity, not its count, so use a
+// sufficiently large enough value for the test to make not affected by growth
+// strategy.
+a.iteratorValue(-9999) // expect runtime error: Iterator out of bounds.

test/core/map_entry/new.wren

@@ -0,0 +1,4 @@
+var entry = MapEntry.new("key", "value")
+
+System.print(entry.key)   // expect: key
+System.print(entry.value) // expect: value