From f6c2450cdb7e24b5dbd118494560cc8452dd3688 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Mon, 11 Jan 2021 01:16:20 -0800 Subject: [PATCH] fix #16555, fixes #16405: len, high honors '\0' for cstring in vm (#16610) --- compiler/vm.nim | 4 ++++ compiler/vmdef.nim | 1 + compiler/vmgen.nim | 12 +++++++----- lib/system.nim | 20 +++++++++++++------- tests/stdlib/thashes.nim | 18 ++++++++++++++++-- tests/stdlib/tstring.nim | 34 +++++++++++++++++++++++++++++++++- 6 files changed, 74 insertions(+), 15 deletions(-) diff --git a/compiler/vm.nim b/compiler/vm.nim index 0f75faee3..b8df456e0 100644 --- a/compiler/vm.nim +++ b/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 diff --git a/compiler/vmdef.nim b/compiler/vmdef.nim index 6d6c55250..eab18f417 100644 --- a/compiler/vmdef.nim +++ b/compiler/vmdef.nim @@ -86,6 +86,7 @@ type opcSubImmInt, opcLenSeq, opcLenStr, + opcLenCstring, opcIncl, opcInclRange, opcExcl, opcCard, opcMulInt, opcDivInt, opcModInt, opcAddFloat, opcSubFloat, opcMulFloat, opcDivFloat, diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim index 46931eb54..6790276a9 100644 --- a/compiler/vmgen.nim +++ b/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) diff --git a/lib/system.nim b/lib/system.nim index e220ba7a3..25184eb15 100644 --- a/lib/system.nim +++ b/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. diff --git a/tests/stdlib/thashes.nim b/tests/stdlib/thashes.nim index 17640387a..ce7bb7d8c 100644 --- a/tests/stdlib/thashes.nim +++ b/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() diff --git a/tests/stdlib/tstring.nim b/tests/stdlib/tstring.nim index 4d5a15940..fcbacc533 100644 --- a/tests/stdlib/tstring.nim +++ b/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()