diff --git a/builtin/core.wren b/builtin/core.wren index 3867bda9..e9d0aa92 100644 --- a/builtin/core.wren +++ b/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 + "}" } } diff --git a/src/vm/wren_core.c b/src/vm/wren_core.c index 05da56dc..1824ed7b 100644 --- a/src/vm/wren_core.c +++ b/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" diff --git a/test/core/list/to_string.wren b/test/core/list/to_string.wren index f20d720b..54b9aac1 100644 --- a/test/core/list/to_string.wren +++ b/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] diff --git a/test/core/map/to_string.wren b/test/core/map/to_string.wren index 46b6b5b9..0ff72822 100644 --- a/test/core/map/to_string.wren +++ b/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. \ No newline at end of file +// 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]}