fix #17159 items(cstring) works in VM (#17160)

* fix #17159 items(cstring) works in VM

* improve test coverage tests/stdlib/tcstring.nim; add helpers: whenRuntimeJs, whenVMorJs

* document items(cstring)

* address comments

4 files changed, 156 insertions, 44 deletions

diff --git a/lib/system/iterators.nim b/lib/system/iterators.nim
index 7f0076a44..7dac4ec77 100644
--- a/lib/system/iterators.nim
+++ b/lib/system/iterators.nim
@@ -56,31 +56,62 @@ iterator items*[T](a: set[T]): T {.inline.} =
 
 iterator items*(a: cstring): char {.inline.} =
   ## Iterates over each item of `a`.
-  when defined(js):
+  runnableExamples:
+    from std/sequtils import toSeq
+    assert toSeq("abc\0def".cstring) == @['a', 'b', 'c']
+    assert toSeq("abc".cstring) == @['a', 'b', 'c']
+  #[
+  assert toSeq(nil.cstring) == @[] # xxx fails with SIGSEGV
+  this fails with SIGSEGV; unclear whether we want to instead yield nothing
+  or pay a small price to check for `nil`, a benchmark is needed. Note that
+  other procs support `nil`.
+  ]#
+  template impl() =
     var i = 0
-    var L = len(a)
-    while i < L:
+    let n = len(a)
+    while i < n:
       yield a[i]
       inc(i)
+  when defined(js): impl()
   else:
-    var i = 0
-    while a[i] != '\0':
-      yield a[i]
-      inc(i)
+    when nimvm:
+      # xxx `cstring` should behave like c backend instead.
+      impl()
+    else:
+      var i = 0
+      while a[i] != '\0':
+        yield a[i]
+        inc(i)
 
 iterator mitems*(a: var cstring): var char {.inline.} =
   ## Iterates over each item of `a` so that you can modify the yielded value.
-  when defined(js):
+  # xxx this should give CT error in js RT.
+  runnableExamples:
+    from std/sugar import collect
+    var a = "abc\0def"
+    var b = a.cstring
+    let s = collect:
+      for bi in mitems(b):
+        if bi == 'b': bi = 'B'
+        bi
+    assert s == @['a', 'B', 'c']
+    assert b == "aBc"
+    assert a == "aBc\0def"
+
+  template impl() =
     var i = 0
-    var L = len(a)
-    while i < L:
+    let n = len(a)
+    while i < n:
       yield a[i]
       inc(i)
+  when defined(js): impl()
   else:
-    var i = 0
-    while a[i] != '\0':
-      yield a[i]
-      inc(i)
+    when nimvm: impl()
+    else:
+      var i = 0
+      while a[i] != '\0':
+        yield a[i]
+        inc(i)
 
 iterator items*[T: enum and Ordinal](E: typedesc[T]): T =
   ## Iterates over the values of `E`.
diff --git a/testament/lib/stdtest/testutils.nim b/testament/lib/stdtest/testutils.nim
index 50dda22e2..36f951272 100644
--- a/testament/lib/stdtest/testutils.nim
+++ b/testament/lib/stdtest/testutils.nim
@@ -42,3 +42,27 @@ template enableRemoteNetworking*: bool =
   ## process calls, e.g. `testament all` calls itself, which in turns invokes
   ## a `nim` invocation (possibly via additional intermediate processes).
   getEnv("NIM_TESTAMENT_REMOTE_NETWORKING") == "1"
+
+template whenRuntimeJs*(bodyIf, bodyElse) =
+  ##[
+  Behaves as `when defined(js) and not nimvm` (which isn't legal yet).
+  pending improvements to `nimvm`, this sugar helps; use as follows:
+
+  whenRuntimeJs:
+    doAssert defined(js)
+    when nimvm: doAssert false
+    else: discard
+  do:
+    discard
+  ]##
+  when nimvm: bodyElse
+  else:
+    when defined(js): bodyIf
+    else: bodyElse
+
+template whenVMorJs*(bodyIf, bodyElse) =
+  ## Behaves as: `when defined(js) or nimvm`
+  when nimvm: bodyIf
+  else:
+    when defined(js): bodyIf
+    else: bodyElse
diff --git a/tests/arc/tcstring.nim b/tests/arc/tcstring.nim
deleted file mode 100644
index 79c8a7fcf..000000000
--- a/tests/arc/tcstring.nim
+++ /dev/null
@@ -1,16 +0,0 @@
-discard """
-  cmd: "nim c --gc:arc -r $file"
-  nimout: '''hello
-h
-o
-'''
-"""
-
-# Issue #13321: [codegen] --gc:arc does not properly emit cstring, results in SIGSEGV
-
-let a = "hello".cstring
-echo a
-echo a[0]
-echo a[4]
-doAssert a[a.len] == '\0'
-
diff --git a/tests/stdlib/tcstring.nim b/tests/stdlib/tcstring.nim
index 299925119..98da5d5c4 100644
--- a/tests/stdlib/tcstring.nim
+++ b/tests/stdlib/tcstring.nim
@@ -1,19 +1,92 @@
 discard """
   targets: "c cpp js"
+  matrix: "; --gc:arc"
 """
 
+from std/sugar import collect
+from stdtest/testutils import whenRuntimeJs, whenVMorJs
 
-block: # bug #13859
-  let str = "abc".cstring
-  doAssert len(str).int8 == 3
-  doAssert len(str).int16 == 3
-  doAssert len(str).int32 == 3
-  var str2 = "cde".cstring
-  doAssert len(str2).int8 == 3
-  doAssert len(str2).int16 == 3
-  doAssert len(str2).int32 == 3
-
-  const str3 = "abc".cstring
-  doAssert len(str3).int32 == 3
-  doAssert len("abc".cstring).int16 == 3
-  doAssert len("abc".cstring).float32 == 3.0
+template testMitems() =
+  block:
+    var a = "abc"
+    var b = a.cstring
+    let s = collect:
+      for bi in mitems(b):
+        if bi == 'b': bi = 'B'
+        bi
+    whenRuntimeJs:
+      discard # xxx mitems should give CT error instead of @['\x00', '\x00', '\x00']
+    do:
+      doAssert s == @['a', 'B', 'c']
+
+  block:
+    var a = "abc\0def"
+    var b = a.cstring
+    let s = collect:
+      for bi in mitems(b):
+        if bi == 'b': bi = 'B'
+        bi
+    whenRuntimeJs:
+      discard # ditto
+    do:
+      doAssert s == @['a', 'B', 'c']
+
+proc mainProc() =
+  testMitems()
+
+template main() =
+  block: # bug #13859
+    let str = "abc".cstring
+    doAssert len(str).int8 == 3
+    doAssert len(str).int16 == 3
+    doAssert len(str).int32 == 3
+    var str2 = "cde".cstring
+    doAssert len(str2).int8 == 3
+    doAssert len(str2).int16 == 3
+    doAssert len(str2).int32 == 3
+
+    const str3 = "abc".cstring
+    doAssert len(str3).int32 == 3
+    doAssert len("abc".cstring).int16 == 3
+    doAssert len("abc".cstring).float32 == 3.0
+
+  block: # bug #17159
+    block:
+      var a = "abc"
+      var b = a.cstring
+      doAssert $(b, ) == """("abc",)"""
+      let s = collect:
+        for bi in b: bi
+      doAssert s == @['a', 'b', 'c']
+
+    block:
+      var a = "abc\0def"
+      var b = a.cstring
+      let s = collect:
+        for bi in b: bi
+      whenRuntimeJs:
+        doAssert $(b, ) == """("abc\x00def",)"""
+        doAssert s == @['a', 'b', 'c', '\x00', 'd', 'e', 'f']
+      do:
+        doAssert $(b, ) == """("abc",)"""
+        doAssert s == @['a', 'b', 'c']
+
+  block:
+    when defined(gcArc): # xxx SIGBUS
+      discard
+    else:
+      mainProc()
+    when false: # xxx bug vm: Error: unhandled exception: 'node' is not accessible using discriminant 'kind' of type 'TFullReg' [FieldDefect]
+      testMitems()
+
+  block: # bug #13321: [codegen] --gc:arc does not properly emit cstring, results in SIGSEGV
+    let a = "hello".cstring
+    doAssert $a == "hello"
+    doAssert $a[0] == "h"
+    doAssert $a[4] == "o"
+    whenVMorJs: discard # xxx this should work in vm, refs https://github.com/timotheecour/Nim/issues/619
+    do:
+      doAssert a[a.len] == '\0'
+
+static: main()
+main()