Don't stackoverflow on recursive lists and maps. Fix #3.

builtin/core.wren

@@ -127,6 +127,34 @@ class WhereSequence is Sequence {
 }
 
 class String is Sequence {
+  // Avoids recursively calling [toString] on [object] and overflowing the
+  // stack.
+  //
+  // If we are already within a call to [safeToString_] on [object] in this
+  // fiber, then this returns "...". Otherwise, it returns the result of
+  // calling [ifUnseen].
+  static safeToString_(object, ifUnseen) {
+    if (__seenByFiber == null) __seenByFiber = new Map
+    var seen = __seenByFiber[Fiber.current]
+    if (seen == null) {
+      __seenByFiber[Fiber.current] = seen = new List
+    }
+
+    // See if we are recursing on it.
+    for (outer in seen) {
+      if (Object.same(outer, object)) return "..."
+    }
+
+    seen.add(object)
+
+    var result = ifUnseen.call()
+
+    seen.removeAt(-1)
+    if (seen.count == 0) __seenByFiber.remove(Fiber.current)
+
+    return result
+  }
+
   bytes { new StringByteSequence(this) }
 }
 
@@ -148,7 +176,7 @@ class List is Sequence {
     return other
   }
 
-  toString { "[" + join(", ") + "]" }
+  toString { String.safeToString_(this) { "[" + join(", ") + "]" } }
 
   +(other) {
     var result = this[0..-1]
@@ -164,16 +192,18 @@ class Map {
   values { new MapValueSequence(this) }
 
   toString {
-    var first = true
-    var result = "{"
+    return String.safeToString_(this) {
+      var first = true
+      var result = "{"
 
-    for (key in keys) {
-      if (!first) result = result + ", "
-      first = false
-      result = result + key.toString + ": " + this[key].toString
+      for (key in keys) {
+        if (!first) result = result + ", "
+        first = false
+        result = result + key.toString + ": " + this[key].toString
+      }
+
+      return result + "}"
     }
-
-    return result + "}"
   }
 }
 

src/vm/wren_core.c

@@ -141,6 +141,34 @@ static const char* coreLibSource =
 "}\n"
 "\n"
 "class String is Sequence {\n"
+"  // Avoids recursively calling [toString] on [object] and overflowing the\n"
+"  // stack.\n"
+"  //\n"
+"  // If we are already within a call to [safeToString_] on [object] in this\n"
+"  // fiber, then this returns \"...\". Otherwise, it returns the result of\n"
+"  // calling [ifUnseen].\n"
+"  static safeToString_(object, ifUnseen) {\n"
+"    if (__seenByFiber == null) __seenByFiber = new Map\n"
+"    var seen = __seenByFiber[Fiber.current]\n"
+"    if (seen == null) {\n"
+"      __seenByFiber[Fiber.current] = seen = new List\n"
+"    }\n"
+"\n"
+"    // See if we are recursing on it.\n"
+"    for (outer in seen) {\n"
+"      if (Object.same(outer, object)) return \"...\"\n"
+"    }\n"
+"\n"
+"    seen.add(object)\n"
+"\n"
+"    var result = ifUnseen.call()\n"
+"\n"
+"    seen.removeAt(-1)\n"
+"    if (seen.count == 0) __seenByFiber.remove(Fiber.current)\n"
+"\n"
+"    return result\n"
+"  }\n"
+"\n"
 "  bytes { new StringByteSequence(this) }\n"
 "}\n"
 "\n"
@@ -162,7 +190,7 @@ static const char* coreLibSource =
 "    return other\n"
 "  }\n"
 "\n"
-"  toString { \"[\" + join(\", \") + \"]\" }\n"
+"  toString { String.safeToString_(this) { \"[\" + join(\", \") + \"]\" } }\n"
 "\n"
 "  +(other) {\n"
 "    var result = this[0..-1]\n"
@@ -178,16 +206,18 @@ static const char* coreLibSource =
 "  values { new MapValueSequence(this) }\n"
 "\n"
 "  toString {\n"
-"    var first = true\n"
-"    var result = \"{\"\n"
+"    return String.safeToString_(this) {\n"
+"      var first = true\n"
+"      var result = \"{\"\n"
 "\n"
-"    for (key in keys) {\n"
-"      if (!first) result = result + \", \"\n"
-"      first = false\n"
-"      result = result + key.toString + \": \" + this[key].toString\n"
+"      for (key in keys) {\n"
+"        if (!first) result = result + \", \"\n"
+"        first = false\n"
+"        result = result + key.toString + \": \" + this[key].toString\n"
+"      }\n"
+"\n"
+"      return result + \"}\"\n"
 "    }\n"
-"\n"
-"    return result + \"}\"\n"
 "  }\n"
 "}\n"
 "\n"

test/core/list/to_string.wren

@@ -14,4 +14,37 @@ class Foo {
 
 IO.print([1, new Foo, 2]) // expect: [1, Foo.toString, 2]
 
-// TODO: Handle lists that contain themselves.
+// Lists that directly contain themselves.
+var list = []
+list.add(list)
+IO.print(list) // expect: [...]
+
+list = [1, 2]
+list[0] = list
+IO.print(list) // expect: [..., 2]
+
+list = [1, 2]
+list[1] = list
+IO.print(list) // expect: [1, ...]
+
+// Lists that indirectly contain themselves.
+list = [null, [2, [3, null, 4], null, 5], 6]
+list[0] = list
+list[1][1][1] = list
+list[1][2] = list
+IO.print(list) // expect: [..., [2, [3, ..., 4], ..., 5], 6]
+
+// List containing an object that calls toString on a recursive list.
+class Box {
+  new(field) { _field = field }
+  toString { "box " + _field.toString }
+}
+
+list = [1, 2]
+list.add(new Box(list))
+IO.print(list) // expect: [1, 2, box ...]
+
+// List containing a map containing the list.
+list = [1, null, 2]
+list[1] = {"list": list}
+IO.print(list) // expect: [1, {list: ...}, 2]

test/core/map/to_string.wren

@@ -24,4 +24,28 @@ IO.print(s == "{1: 2, 3: 4, 5: 6}" ||
          s == "{5: 6, 1: 2, 3: 4}" ||
          s == "{5: 6, 3: 4, 1: 2}") // expect: true
 
-// TODO: Handle maps that contain themselves.
+// Map that directly contains itself.
+var map = {}
+map["key"] = map
+IO.print(map) // expect: {key: ...}
+
+// Map that indirectly contains itself.
+map = {}
+map["a"] = {"b": {"c": map}}
+
+IO.print(map) // expect: {a: {b: {c: ...}}}
+
+// Map containing an object that calls toString on a recursive map.
+class Box {
+  new(field) { _field = field }
+  toString { "box " + _field.toString }
+}
+
+map = {}
+map["box"] = new Box(map)
+IO.print(map) // expect: {box: box ...}
+
+// Map containing a list containing the map.
+map = {}
+map["list"] = [1, map, 2]
+IO.print(map) // expect: {list: [1, ..., 2]}