fix #16555, fixes #16405: len, high honors '\0' for cstring in vm (#16610)

compiler/vm.nim

@@ -872,6 +872,10 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       decodeBImm(rkInt)
       assert regs[rb].kind == rkNode
       regs[ra].intVal = regs[rb].node.strVal.len - imm
+    of opcLenCstring:
+      decodeBImm(rkInt)
+      assert regs[rb].kind == rkNode
+      regs[ra].intVal = regs[rb].node.strVal.cstring.len - imm
     of opcIncl:
       decodeB(rkNode)
       let b = regs[rb].regToNode

compiler/vmdef.nim

@@ -86,6 +86,7 @@ type
     opcSubImmInt,
     opcLenSeq,
     opcLenStr,
+    opcLenCstring,
 
     opcIncl, opcInclRange, opcExcl, opcCard, opcMulInt, opcDivInt, opcModInt,
     opcAddFloat, opcSubFloat, opcMulFloat, opcDivFloat,

compiler/vmgen.nim

@@ -1034,7 +1034,10 @@ proc genMagic(c: PCtx; n: PNode; dest: var TDest; m: TMagic) =
   of mLengthOpenArray, mLengthArray, mLengthSeq:
     genUnaryABI(c, n, dest, opcLenSeq)
   of mLengthStr:
-    genUnaryABI(c, n, dest, opcLenStr)
+    case n[1].typ.kind
+    of tyString: genUnaryABI(c, n, dest, opcLenStr)
+    of tyCString: genUnaryABI(c, n, dest, opcLenCstring)
+    else: doAssert false, $n[1].typ.kind
   of mIncl, mExcl:
     unused(c, n, dest)
     var d = c.genx(n[1])
@@ -1178,10 +1181,9 @@ proc genMagic(c: PCtx; n: PNode; dest: var TDest; m: TMagic) =
     if dest < 0: dest = c.getTemp(n.typ)
     let tmp = c.genx(n[1])
     case n[1].typ.skipTypes(abstractVar-{tyTypeDesc}).kind:
-    of tyString, tyCString:
-      c.gABI(n, opcLenStr, dest, tmp, 1)
-    else:
-      c.gABI(n, opcLenSeq, dest, tmp, 1)
+    of tyString: c.gABI(n, opcLenStr, dest, tmp, 1)
+    of tyCString: c.gABI(n, opcLenCstring, dest, tmp, 1)
+    else: c.gABI(n, opcLenSeq, dest, tmp, 1)
     c.freeTemp(tmp)
   of mEcho:
     unused(c, n, dest)

lib/system.nim

@@ -701,18 +701,24 @@ proc len*(x: string): int {.magic: "LengthStr", noSideEffect.}
   ##   var str = "Hello world!"
   ##   echo len(str) # => 12
 
-proc len*(x: cstring): int {.magic: "LengthStr", noSideEffect.}
-  ## Returns the length of a compatible string. This is sometimes
-  ## an O(n) operation.
+proc len*(x: cstring): int {.magic: "LengthStr", noSideEffect.} =
+  ## Returns the length of a compatible string. This is an O(n) operation except
+  ## in js at runtime.
   ##
   ## **Note:** On the JS backend this currently counts UTF-16 code points
   ## instead of bytes at runtime (not at compile time). For now, if you
   ## need the byte length of the UTF-8 encoding, convert to string with
   ## `$` first then call `len`.
-  ##
-  ## .. code-block:: Nim
-  ##   var str: cstring = "Hello world!"
-  ##   len(str) # => 12
+  runnableExamples:
+    doAssert len(cstring"abc") == 3
+    doAssert len(cstring r"ab\0c") == 5 # \0 is escaped
+    doAssert len(cstring"ab\0c") == 5 # ditto
+    var a: cstring = "ab\0c"
+    when defined(js): doAssert a.len == 4 # len ignores \0 for js
+    else: doAssert a.len == 2 # \0 is a null terminator
+    static:
+      var a2: cstring = "ab\0c"
+      doAssert a2.len == 2 # \0 is a null terminator, even in js vm
 
 proc len*(x: (type array)|array): int {.magic: "LengthArray", noSideEffect.}
   ## Returns the length of an array or an array type.

tests/stdlib/thashes.nim

@@ -86,8 +86,6 @@ block largeSize: # longer than 4 characters
   doAssert hash(xx, 0, 3) == hash(ssl, 0, 3)
 
 proc main() =
-
-
   doAssert hash(0.0) == hash(0)
   doAssert hash(cstring"abracadabra") == 97309975
   doAssert hash(cstring"abracadabra") == hash("abracadabra")
@@ -115,6 +113,22 @@ proc main() =
     doAssert hash(-9999.283456) != 0
     doAssert hash(84375674.0) != 0
 
+  block: # bug #16555
+    proc fn(): auto =
+      # avoids hardcoding values
+      var a = "abc\0def"
+      var b = a.cstring
+      result = (hash(a), hash(b))
+      doAssert result[0] != result[1]
+    when not defined(js):
+      doAssert fn() == static(fn())
+    else:
+      # xxx this is a tricky case; consistency of hashes for cstring's containing
+      # '\0\' matters for c backend but less for js backend since such strings
+      # are much less common in js backend; we make vm for js backend consistent
+      # with c backend instead of js backend because FFI code (or other) could
+      # run at CT, expecting c semantics.
+      discard
 
 static: main()
 main()

tests/stdlib/tstring.nim

@@ -96,11 +96,43 @@ func reverse*(a: string): string =
 
 
 proc main() =
+  # xxx put all tests here to test in VM and RT
   test_string_slice()
   test_string_cmp()
 
   tester(1)
-  doAssert reverse("hello") == "olleh"
+
+  block: # reverse
+    doAssert reverse("hello") == "olleh"
+
+  block: # len, high
+    var a = "ab\0cd"
+    var b = a.cstring
+    doAssert a.len == 5
+    block: # bug #16405
+      when defined(js):
+        when nimvm: doAssert b.len == 2
+        else: doAssert b.len == 5
+      else: doAssert b.len == 2
+
+    doAssert a.high == a.len - 1
+    doAssert b.high == b.len - 1
+
+    doAssert "".len == 0
+    doAssert "".high == -1
+    doAssert "".cstring.len == 0
+    doAssert "".cstring.high == -1
+
+    var c: cstring = nil
+    template impl() =
+      doAssert c.len == 0
+      doAssert c.high == -1
+    when defined js:
+      when nimvm: impl()
+      else:
+        # xxx pending bug #16674
+        discard
+    else: impl()
 
 static: main()
 main()