improvements to addInt and $ for integer types (#18592)

* improvements to $(SomeInteger) and addInt
* remove mIntToStr, mInt64ToStr
* improvements
* fix tests/pragmas/tinjectstmt.nim; the diff is harmless, cgen code is identical with -d:danger or debug mode
* rm tests/system/tstrmantle.nim
* revert compiler/jsgen.nim for -d:nimVersion140

compiler/ast.nim

@@ -652,8 +652,8 @@ type
     mUnaryMinusI, mUnaryMinusI64, mAbsI, mNot,
     mUnaryPlusI, mBitnotI,
     mUnaryPlusF64, mUnaryMinusF64,
-    mCharToStr, mBoolToStr, mIntToStr, mInt64ToStr,
-    mFloatToStr, # for -d:nimVersion140
+    mCharToStr, mBoolToStr,
+    mIntToStr, mInt64ToStr, mFloatToStr, # for -d:nimVersion140
     mCStrToStr,
     mStrToStr, mEnumToStr,
     mAnd, mOr,
@@ -722,8 +722,8 @@ const
     mEqRef, mEqProc, mLePtr, mLtPtr, mEqCString, mXor,
     mUnaryMinusI, mUnaryMinusI64, mAbsI, mNot, mUnaryPlusI, mBitnotI,
     mUnaryPlusF64, mUnaryMinusF64,
-    mCharToStr, mBoolToStr, mIntToStr, mInt64ToStr,
-    mFloatToStr,
+    mCharToStr, mBoolToStr,
+    mIntToStr, mInt64ToStr, mFloatToStr,
     mCStrToStr,
     mStrToStr, mEnumToStr,
     mAnd, mOr,

lib/std/private/digitsutils.nim

@@ -29,18 +29,34 @@ const
 #   doAssert res == digits100
 
 proc utoa2Digits*(buf: var openArray[char]; pos: int; digits: uint32) {.inline.} =
-  assert(digits <= 99)
   buf[pos] = digits100[2 * digits]
   buf[pos+1] = digits100[2 * digits + 1]
   #copyMem(buf, unsafeAddr(digits100[2 * digits]), 2 * sizeof((char)))
 
 proc trailingZeros2Digits*(digits: uint32): int32 {.inline.} =
-  assert(digits <= 99)
   return trailingZeros100[digits]
 
-func addIntImpl*(result: var string, origin: uint64) =
+when defined(js):
+  proc numToString(a: SomeInteger): cstring {.importjs: "((#) + \"\")".}
+
+func addChars[T](result: var string, x: T, start: int, n: int) {.inline.} =
+  let old = result.len
+  result.setLen old + n
+  template impl =
+    for i in 0..<n: result[old + i] = x[start + i]
+  when nimvm: impl
+  else:
+    when defined(js) or defined(nimscript): impl
+    else:
+      {.noSideEffect.}:
+        copyMem result[old].addr, x[start].unsafeAddr, n
+
+func addChars[T](result: var string, x: T) {.inline.} =
+  addChars(result, x, 0, x.len)
+
+func addIntImpl(result: var string, x: uint64) {.inline.} =
   var tmp {.noinit.}: array[24, char]
-  var num = origin
+  var num = x
   var next = tmp.len - 1
   const nbatch = 100
 
@@ -60,17 +76,39 @@ func addIntImpl*(result: var string, origin: uint64) =
     tmp[next] = digits100[index + 1]
     tmp[next - 1] = digits100[index]
     dec next
-  let n = result.len
-  let length = tmp.len - next
-  result.setLen n + length
-  when nimvm:
-    for i in 0..<length:
-      result[n+i] = tmp[next+i]
-  else:
-    when defined(js) or defined(nimscript):
-      for i in 0..<length:
-        result[n+i] = tmp[next+i]
-    else:
-      {.noSideEffect.}:
-        copyMem result[n].addr, tmp[next].addr, length
+  addChars(result, tmp, next, tmp.len - next)
 
+func addInt*(result: var string, x: uint64) =
+  when nimvm: addIntImpl(result, x)
+  else:
+    when not defined(js): addIntImpl(result, x)
+    else:
+      addChars(result, numToString(x))
+
+proc addInt*(result: var string; x: int64) =
+  ## Converts integer to its string representation and appends it to `result`.
+  runnableExamples:
+    var s = "foo"
+    s.addInt(45)
+    assert s == "foo45"
+  template impl =
+    var num: uint64
+    if x < 0:
+      if x == low(int64):
+        num = uint64(x)
+      else:
+        num = uint64(-x)
+      let base = result.len
+      setLen(result, base + 1)
+      result[base] = '-'
+    else:
+      num = uint64(x)
+    addInt(result, num)
+  when nimvm: impl()
+  else:
+    when defined(js):
+      addChars(result, numToString(x))
+    else: impl()
+
+proc addInt*(result: var string; x: int) {.inline.} =
+  addInt(result, int64(x))

lib/std/private/miscdollars.nim

@@ -1,3 +1,5 @@
+from std/private/digitsutils import addInt
+
 template toLocation*(result: var string, file: string | cstring, line: int, col: int) =
   ## avoids spurious allocations
   # Hopefully this can be re-used everywhere so that if a user needs to customize,
@@ -5,11 +7,8 @@ template toLocation*(result: var string, file: string | cstring, line: int, col:
   result.add file
   if line > 0:
     result.add "("
-    # simplify this after moving moving `include strmantle` above import assertions`
-    when declared(addInt): result.addInt line
-    else: result.add $line
+    addInt(result, line)
     if col > 0:
       result.add ", "
-      when declared(addInt): result.addInt col
-      else: result.add $col
+      addInt(result, col)
     result.add ")"

lib/system.nim

@@ -2463,14 +2463,13 @@ when notJSnotNims:
     else:
       {.error: "Only closure iterator is allowed!".}
 
+from std/private/digitsutils import addInt
+export addInt
+
 when defined(js):
   include "system/jssys"
   include "system/reprjs"
 
-when defined(js) or defined(nimscript):
-  proc addInt*(result: var string; x: int64) =
-    result.add $x
-
 proc quit*(errormsg: string, errorcode = QuitFailure) {.noreturn.} =
   ## A shorthand for `echo(errormsg); quit(errorcode)`.
   when defined(nimscript) or defined(js) or (hostOS == "standalone"):
@@ -2918,7 +2917,7 @@ proc addQuoted*[T](s: var string, x: T) =
     s.addEscapedChar(x)
     s.add("'")
   # prevent temporary string allocation
-  elif T is SomeSignedInt:
+  elif T is SomeInteger:
     s.addInt(x)
   elif T is SomeFloat:
     s.addFloat(x)

lib/system/assertions.nim

@@ -12,7 +12,6 @@ import std/private/miscdollars
 
 type InstantiationInfo = tuple[filename: string, line: int, column: int]
 
-proc `$`(x: int): string {.magic: "IntToStr", noSideEffect.}
 proc `$`(info: InstantiationInfo): string =
   # The +1 is needed here
   # instead of overriding `$` (and changing its meaning), consider explicit name.
@@ -108,7 +107,9 @@ template doAssertRaises*(exception: typedesc, code: untyped) =
       wrong = true
     except exception:
       discard
-    except Exception as e: raiseAssert(begin & " raised '" & $e.name & "'" & msgEnd)
+    except Exception as e:
+      mixin `$` # alternatively, we could define $cstring in this module
+      raiseAssert(begin & " raised '" & $e.name & "'" & msgEnd)
     except: raisedForeign()
   if wrong:
     raiseAssert(begin & " nothing was raised" & msgEnd)

lib/system/dollars.nim

@@ -1,45 +1,31 @@
+## `$` is Nim's general way of spelling `toString`:idx:.
+runnableExamples:
+  assert $0.1 == "0.1"
+  assert $(-2*3) == "-6"
+
 import std/private/digitsutils
 import system/formatfloat
 export addFloat
 
-proc `$`*(x: int): string {.magic: "IntToStr", noSideEffect.}
-  ## The stringify operator for an integer argument. Returns `x`
-  ## converted to a decimal string. `$` is Nim's general way of
-  ## spelling `toString`:idx:.
+proc `$`*(x: int): string {.raises: [].} =
+  ## Outplace version of `addInt`.
+  result.addInt(x)
 
-template dollarImpl(x: uint | uint64, result: var string) =
-  addIntImpl(result, x)
+proc `$`*(x: int64): string {.raises: [].} =
+  ## Outplace version of `addInt`.
+  result.addInt(x)
 
-when defined(js):
-  import std/private/since
-  since (1, 3):
-    proc `$`*(x: uint): string =
-      ## Caveat: currently implemented as $(cast[int](x)), tied to current
-      ## semantics of js' Number type.
-      # for c, see strmantle.`$`
-      when nimvm:
-        dollarImpl(x, result)
-      else:
-        result = $(int(x))
+proc `$`*(x: uint64): string {.raises: [].} =
+  ## Outplace version of `addInt`.
+  addInt(result, x)
 
-    proc `$`*(x: uint64): string =
-      ## Compatibility note:
-      ## the results may change in future releases if/when js target implements
-      ## 64bit ints.
-      # pending https://github.com/nim-lang/RFCs/issues/187
-      when nimvm:
-        dollarImpl(x, result)
-      else:
-        result = $(cast[int](x))
-else:
-  proc `$`*(x: uint64): string {.noSideEffect, raises: [].} =
-    ## The stringify operator for an unsigned integer argument. Returns `x`
-    ## converted to a decimal string.
-    dollarImpl(x, result)
-
-proc `$`*(x: int64): string {.magic: "Int64ToStr", noSideEffect.}
-  ## The stringify operator for an integer argument. Returns `x`
-  ## converted to a decimal string.
+# same as old `ctfeWhitelist` behavior, whether or not this is a good idea.
+template gen(T) =
+  # xxx simplify this by supporting this in compiler: int{lit} | uint64{lit} | int64{lit}
+  func `$`*(x: T{lit}): string {.compileTime.} = result.addInt(x)
+gen(int)
+gen(uint64)
+gen(int64)
 
 func `$`*(x: float | float32): string =
   ## Outplace version of `addFloat`.

lib/system/repr_v2.nim

@@ -8,18 +8,17 @@ proc distinctBase(T: typedesc, recursive: static bool = true): typedesc {.magic:
 
 proc repr*(x: NimNode): string {.magic: "Repr", noSideEffect.}
 
-proc repr*(x: int): string {.magic: "IntToStr", noSideEffect.}
-  ## repr for an integer argument. Returns `x`
-  ## converted to a decimal string.
+proc repr*(x: int): string =
+  ## Same as $x
+  $x
 
-proc repr*(x: int64): string {.magic: "Int64ToStr", noSideEffect.}
-  ## repr for an integer argument. Returns `x`
-  ## converted to a decimal string.
+proc repr*(x: int64): string =
+  ## Same as $x
+  $x
 
 proc repr*(x: uint64): string {.noSideEffect.} =
-  ## repr for an unsigned integer argument. Returns `x`
-  ## converted to a decimal string.
-  $x #Calls `$` from system/strmantle.nim
+  ## Same as $x
+  $x
 
 proc repr*(x: float): string =
   ## Same as $x

lib/system/strmantle.nim

@@ -43,32 +43,6 @@ proc hashString(s: string): int {.compilerproc.} =
   h = h + h shl 15
   result = cast[int](h)
 
-proc addInt*(result: var string; x: int64) =
-  ## Converts integer to its string representation and appends it to `result`.
-  ##
-  ## .. code-block:: Nim
-  ##   var
-  ##     a = "123"
-  ##     b = 45
-  ##   a.addInt(b) # a <- "12345"
-  var num: uint64
-
-  if x < 0:
-    if x == low(int64):
-      num = uint64(x)
-    else:
-      num = uint64(-x)
-    let base = result.len
-    setLen(result, base + 1)
-    result[base] = '-'
-  else:
-    num = uint64(x)
-  addIntImpl(result, num)
-
-proc nimIntToStr(x: int): string {.compilerRtl.} =
-  result = newStringOfCap(sizeof(x)*4)
-  result.addInt x
-
 proc c_strtod(buf: cstring, endptr: ptr cstring): float64 {.
   importc: "strtod", header: "<stdlib.h>", noSideEffect.}
 
@@ -240,10 +214,6 @@ proc nimParseBiggestFloat(s: string, number: var BiggestFloat,
 when defined(nimHasInvariant):
   {.pop.} # staticBoundChecks
 
-proc nimInt64ToStr(x: int64): string {.compilerRtl.} =
-  result = newStringOfCap(sizeof(x)*4)
-  result.addInt x
-
 proc nimBoolToStr(x: bool): string {.compilerRtl.} =
   return if x: "true" else: "false"
 

nimsuggest/tests/tqualified_highlight.nim

@@ -10,5 +10,5 @@ highlight;;skProc;;1;;7;;4
 highlight;;skTemplate;;2;;7;;4
 highlight;;skTemplate;;2;;7;;4
 highlight;;skTemplate;;2;;7;;4
-highlight;;skProc;;3;;8;;1
+highlight;;skFunc;;3;;8;;1
 """

tests/pragmas/tinjectstmt.nim

@@ -7,15 +7,18 @@ ok0
 ok1
 onInject: 3
 onInject: 4
-0
 onInject: 5
+0
 onInject: 6
-1
 onInject: 7
 onInject: 8
+1
+onInject: 9
+onInject: 10
+onInject: 11
 2
 ok2
-onInject: 9
+onInject: 12
 '''
 """
 
@@ -25,7 +28,7 @@ onInject: 9
 {.injectStmt.} pragma can be used to inject a statement before every
 other statement in the current module. It's now undocumented and may be removed
 in the future and replaced with something more general and without its limitations.
-e.g. (e.g. doesn't work in VM or js backends).
+(e.g. doesn't work in VM or js backends).
 ]#
 
 from system/ansi_c import c_printf
@@ -44,5 +47,5 @@ proc main()=
     echo a
   echo "ok2"
 
-static: main() # xxx injectStmt not honred in VM
+static: main() # xxx injectStmt not honored in VM
 main()

tests/system/tdollars.nim

@@ -102,40 +102,41 @@ block: # #14350, #16674, #16686 for JS
     doAssert nil2 == cstring("")
 
 block:
-  block:
-    let x = -1'i8
-    let y = uint32(x)
-
-    doAssert $y == "4294967295"
-
-  block:
-    let x = -1'i16
-    let y = uint32(x)
-
-    doAssert $y == "4294967295"
-
-  block:
-    let x = -1'i32
-    let y = uint32(x)
-
-    doAssert $y == "4294967295"
+  when defined(js): # bug #18591
+    let a1 = -1'i8
+    let a2 = uint8(a1)
+    # if `uint8(a1)` changes meaning to `cast[uint8](a1)` in future, update this test;
+    # until then, this is the correct semantics.
+    let a3 = $a2
+    doAssert a2 < 3
+    doAssert a3 == "-1"
+    proc intToStr(a: uint8): cstring {.importjs: "(# + \"\")".}
+    doAssert $intToStr(a2) == "-1"
+  else:
+    block:
+      let x = -1'i8
+      let y = uint32(x)
+      doAssert $y == "4294967295"
+    block:
+      let x = -1'i16
+      let y = uint32(x)
+      doAssert $y == "4294967295"
+    block:
+      let x = -1'i32
+      let y = uint32(x)
+      doAssert $y == "4294967295"
+    block:
+      proc foo1(arg: int): string =
+        let x = uint32(arg)
+        $x
+      doAssert $foo1(-1) == "4294967295"
 
   block:
     let x = 4294967295'u32
     doAssert $x == "4294967295"
-
   block:
     doAssert $(4294967295'u32) == "4294967295"
 
-
-  block:
-    proc foo1(arg: int): string =
-      let x = uint32(arg)
-      $x
-
-    doAssert $foo1(-1) == "4294967295"
-
-
 proc main()=
   block:
     let a = -0.0
@@ -159,6 +160,32 @@ proc main()=
 
   doAssert $uint32.high == "4294967295"
 
+  block: # addInt
+    var res = newStringOfCap(24)
+    template test2(a, b) =
+      res.setLen(0)
+      res.addInt a
+      doAssert res == b
+
+    for i in 0 .. 9:
+      res.addInt int64(i)
+    doAssert res == "0123456789"
+
+    res.setLen(0)
+    for i in -9 .. 0:
+      res.addInt int64(i)
+    doAssert res == "-9-8-7-6-5-4-3-2-10"
+
+    when not defined(js):
+      test2 high(int64), "9223372036854775807"
+      test2 low(int64), "-9223372036854775808"
+
+    test2 high(int32), "2147483647"
+    test2 low(int32), "-2147483648"
+    test2 high(int16), "32767"
+    test2 low(int16), "-32768"
+    test2 high(int8), "127"
+    test2 low(int8), "-128"
 
 static: main()
 main()

tests/system/tstrmantle.nim

@@ -1,46 +0,0 @@
-var res = newStringOfCap(24)
-
-for i in 0 .. 9:
-  res.addInt int64(i)
-
-doAssert res == "0123456789"
-
-res.setLen(0)
-
-for i in -9 .. 0:
-  res.addInt int64(i)
-
-doAssert res == "-9-8-7-6-5-4-3-2-10"
-
-res.setLen(0)
-res.addInt high(int64)
-doAssert res == "9223372036854775807"
-
-res.setLen(0)
-res.addInt low(int64)
-doAssert res == "-9223372036854775808"
-
-res.setLen(0)
-res.addInt high(int32)
-doAssert res == "2147483647"
-
-res.setLen(0)
-res.addInt low(int32)
-doAssert res == "-2147483648"
-
-res.setLen(0)
-res.addInt high(int16)
-doAssert res == "32767"
-
-res.setLen(0)
-res.addInt low(int16)
-doAssert res == "-32768"
-
-
-res.setLen(0)
-res.addInt high(int8)
-doAssert res == "127"
-
-res.setLen(0)
-res.addInt low(int8)
-doAssert res == "-128"