many bugfixes for js (#14158)

* many bugfixes for js

fixes #12672, fixes #14153, closes #14123, closes #11331, fixes #11783, fixes #13966, fixes #14087, fixes #14117, closes #12256.

mostly fixes the fact that it was allowed to assign to newly created temp variables. additionally attempts to get rid of null initialized seqs/strings (though they might pop up here and there); this simplifies a lot of things and makes code size smaller. even if null seqs/strings pop up here and there it's still better than all those bugs existing.

* formatting fixes

* CI fixes

* more CI fixes

compiler/jsgen.nim

@@ -86,6 +86,7 @@ type
     generatedSyms: IntSet
     typeInfoGenerated: IntSet
     unique: int    # for temp identifier generation
+    inSystem: bool
 
   PProc = ref TProc
   TProc = object
@@ -159,6 +160,14 @@ proc newProc(globals: PGlobals, module: BModule, procDef: PNode,
     extraIndent: int(procDef != nil))
   if procDef != nil: result.prc = procDef[namePos].sym
 
+proc initProcOptions(module: BModule): TOptions =
+  result = module.config.options
+  if PGlobals(module.graph.backend).inSystem:
+    result.excl(optStackTrace)
+
+proc newInitProc(globals: PGlobals, module: BModule): PProc =
+  result = newProc(globals, module, nil, initProcOptions(module))
+
 proc declareGlobal(p: PProc; id: int; r: Rope) =
   if p.prc != nil and not p.declaredGlobals.containsOrIncl(id):
     p.locals.addf("global $1;$n", [r])
@@ -455,7 +464,48 @@ proc maybeMakeTemp(p: PProc, n: PNode; x: TCompRes): tuple[a, tmp: Rope] =
   else:
     (a: a, tmp: b)
 
-template binaryExpr(p: PProc, n: PNode, r: var TCompRes, magic, frmt: string) =
+proc maybeMakeTempAssignable(p: PProc, n: PNode; x: TCompRes): tuple[a, tmp: Rope] =
+  var
+    a = x.rdLoc
+    b = a
+  if needsTemp(p, n):
+    # if we have tmp just use it
+    if x.tmpLoc != nil and (mapType(n.typ) == etyBaseIndex or n.kind in {nkHiddenDeref, nkDerefExpr}):
+      b = "$1[0][$1[1]]" % [x.tmpLoc]
+      result = (a: a, tmp: b)
+    elif x.tmpLoc != nil and n.kind == nkBracketExpr:
+      # genArrayAddr
+      var
+        address, index: TCompRes
+        first: Int128
+      gen(p, n[0], address)
+      gen(p, n[1], index)
+      let (m1, tmp1) = maybeMakeTemp(p, n[0], address)
+      let typ = skipTypes(n[0].typ, abstractPtrs)
+      if typ.kind == tyArray:
+        first = firstOrd(p.config, typ[0])
+      if optBoundsCheck in p.options:
+        useMagic(p, "chckIndx")
+        if first == 0: # save a couple chars
+          index.res = "chckIndx($1, 0, ($2).length-1)" % [index.res, tmp1]
+        else:
+          index.res = "chckIndx($1, $2, ($3).length+($2)-1)-($2)" % [
+            index.res, rope(first), tmp1]
+      elif first != 0:
+        index.res = "($1)-($2)" % [index.res, rope(first)]
+      else:
+        discard # index.res = index.res
+      let (n1, tmp2) = maybeMakeTemp(p, n[1], index)
+      result = (a: "$1[$2]" % [m1, n1], tmp: "$1[$2]" % [tmp1, tmp2])
+    # could also put here: nkDotExpr -> genFieldAccess, nkCheckedFieldExpr -> genCheckedFieldOp
+    # but the uses of maybeMakeTempAssignable don't need them
+    else:
+      result = (a: a, tmp: b)
+  else:
+    result = (a: a, tmp: b)
+
+template binaryExpr(p: PProc, n: PNode, r: var TCompRes, magic, frmt: string,
+                    reassign = false) =
   # $1 and $2 in the `frmt` string bind to lhs and rhs of the expr,
   # if $3 or $4 are present they will be substituted with temps for
   # lhs and rhs respectively
@@ -467,8 +517,11 @@ template binaryExpr(p: PProc, n: PNode, r: var TCompRes, magic, frmt: string) =
   var
     a, tmp = x.rdLoc
     b, tmp2 = y.rdLoc
-  when "$3" in frmt: (a, tmp) = maybeMakeTemp(p, n[1], x)
-  when "$4" in frmt: (b, tmp2) = maybeMakeTemp(p, n[2], y)
+  when reassign:
+    (a, tmp) = maybeMakeTempAssignable(p, n[1], x)
+  else:
+    when "$3" in frmt: (a, tmp) = maybeMakeTemp(p, n[1], x)
+    when "$4" in frmt: (b, tmp2) = maybeMakeTemp(p, n[2], y)
 
   r.res = frmt % [a, b, tmp, tmp2]
   r.kind = resExpr
@@ -485,13 +538,13 @@ template unsignedTrimmer(size: BiggestInt): Rope =
   size.unsignedTrimmerJS
 
 proc binaryUintExpr(p: PProc, n: PNode, r: var TCompRes, op: string,
-                    reassign = false) =
+                    reassign: static[bool] = false) =
   var x, y: TCompRes
   gen(p, n[1], x)
   gen(p, n[2], y)
   let trimmer = unsignedTrimmer(n[1].typ.skipTypes(abstractRange).size)
-  if reassign:
-    let (a, tmp) = maybeMakeTemp(p, n[1], x)
+  when reassign:
+    let (a, tmp) = maybeMakeTempAssignable(p, n[1], x)
     r.res = "$1 = (($5 $2 $3) $4)" % [a, rope op, y.rdLoc, trimmer, tmp]
   else:
     r.res = "(($1 $2 $3) $4)" % [x.rdLoc, rope op, y.rdLoc, trimmer]
@@ -1161,7 +1214,11 @@ proc genArrayAddr(p: PProc, n: PNode, r: var TCompRes) =
     first = firstOrd(p.config, typ[0])
   if optBoundsCheck in p.options:
     useMagic(p, "chckIndx")
-    r.res = "chckIndx($1, $2, ($3 != null ? $3.length : 0)+$2-1)-($2)" % [b.res, rope(first), tmp]
+    if first == 0: # save a couple chars
+      r.res = "chckIndx($1, 0, ($2).length-1)" % [b.res, tmp]
+    else:
+      r.res = "chckIndx($1, $2, ($3).length+($2)-1)-($2)" % [
+        b.res, rope(first), tmp]
   elif first != 0:
     r.res = "($1)-($2)" % [b.res, rope(first)]
   else:
@@ -1631,7 +1688,11 @@ proc createVar(p: PProc, typ: PType, indirect: bool): Rope =
       result = putToSeq("[null, 0]", indirect)
     else:
       result = putToSeq("null", indirect)
-  of tySequence, tyOpt, tyString, tyCString, tyProc:
+  of tySequence, tyString:
+    result = putToSeq("[]", indirect)
+  of tyCString:
+    result = putToSeq("\"\"", indirect)
+  of tyOpt, tyProc:
     result = putToSeq("null", indirect)
   of tyStatic:
     if t.n != nil:
@@ -1862,7 +1923,7 @@ proc genReset(p: PProc, n: PNode) =
   if x.typ == etyBaseIndex:
     lineF(p, "$1 = null, $2 = 0;$n", [x.address, x.res])
   else:
-    let (a, tmp) = maybeMakeTemp(p, n[1], x)
+    let (a, tmp) = maybeMakeTempAssignable(p, n[1], x)
     lineF(p, "$1 = genericReset($3, $2);$n", [a,
                   genTypeInfo(p, n[1].typ), tmp])
 
@@ -1888,37 +1949,33 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) =
   of mSwap: genSwap(p, n)
   of mAppendStrCh:
     binaryExpr(p, n, r, "addChar",
-        "if ($1 != null) { addChar($3, $2); } else { $3 = [$2]; }")
+        "addChar($1, $2);")
   of mAppendStrStr:
     var lhs, rhs: TCompRes
     gen(p, n[1], lhs)
     gen(p, n[2], rhs)
 
-    let rhsIsLit = n[2].kind in nkStrKinds
-    let (a, tmp) = maybeMakeTemp(p, n[1], lhs)
     if skipTypes(n[1].typ, abstractVarRange).kind == tyCString:
-      r.res = "if ($1 != null) { $3 += $2; } else { $3 = $2; }" % [
-        a, rhs.rdLoc, tmp]
+      r.res = "$1 += $2;" % [lhs.rdLoc, rhs.rdLoc]
     else:
-      r.res = "if ($1 != null) { $4 = ($4).concat($2); } else { $4 = $2$3; }" % [
-          lhs.rdLoc, rhs.rdLoc, if rhsIsLit: nil else: ~".slice()", tmp]
+      let (a, tmp) = maybeMakeTemp(p, n[1], lhs)
+      r.res = "$1.push.apply($3, $2);" % [a, rhs.rdLoc, tmp]
     r.kind = resExpr
   of mAppendSeqElem:
     var x, y: TCompRes
     gen(p, n[1], x)
     gen(p, n[2], y)
-    let (a, tmp) = maybeMakeTemp(p, n[1], x)
     if mapType(n[2].typ) == etyBaseIndex:
       let c = "[$1, $2]" % [y.address, y.res]
-      r.res = "if ($1 != null) { $3.push($2); } else { $3 = [$2]; }" % [a, c, tmp]
+      r.res = "$1.push($2);" % [x.rdLoc, c]
     elif needsNoCopy(p, n[2]):
-      r.res = "if ($1 != null) { $3.push($2); } else { $3 = [$2]; }" % [a, y.rdLoc, tmp]
+      r.res = "$1.push($2);" % [x.rdLoc, y.rdLoc]
     else:
       useMagic(p, "nimCopy")
       let c = getTemp(p, defineInLocals=false)
       lineF(p, "var $1 = nimCopy(null, $2, $3);$n",
             [c, y.rdLoc, genTypeInfo(p, n[2].typ)])
-      r.res = "if ($1 != null) { $3.push($2); } else { $3 = [$2]; }" % [a, c, tmp]
+      r.res = "$1.push($2);" % [x.rdLoc, c]
     r.kind = resExpr
   of mConStrStr:
     genConStrStr(p, n, r)
@@ -1950,23 +2007,23 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) =
   of mDestroy: discard "ignore calls to the default destructor"
   of mOrd: genOrd(p, n, r)
   of mLengthStr, mLengthSeq, mLengthOpenArray, mLengthArray:
-    unaryExpr(p, n, r, "", "($1 != null ? $2.length : 0)")
+    unaryExpr(p, n, r, "", "($1).length")
   of mHigh:
-    unaryExpr(p, n, r, "", "($1 != null ? ($2.length-1) : -1)")
+    unaryExpr(p, n, r, "", "(($1).length-1)")
   of mInc:
     if n[1].typ.skipTypes(abstractRange).kind in {tyUInt..tyUInt64}:
       binaryUintExpr(p, n, r, "+", true)
     else:
       if optOverflowCheck notin p.options: binaryExpr(p, n, r, "", "$1 += $2")
-      else: binaryExpr(p, n, r, "addInt", "$1 = addInt($3, $2)")
+      else: binaryExpr(p, n, r, "addInt", "$1 = addInt($3, $2)", true)
   of ast.mDec:
     if n[1].typ.skipTypes(abstractRange).kind in {tyUInt..tyUInt64}:
       binaryUintExpr(p, n, r, "-", true)
     else:
       if optOverflowCheck notin p.options: binaryExpr(p, n, r, "", "$1 -= $2")
-      else: binaryExpr(p, n, r, "subInt", "$1 = subInt($3, $2)")
+      else: binaryExpr(p, n, r, "subInt", "$1 = subInt($3, $2)", true)
   of mSetLengthStr:
-    binaryExpr(p, n, r, "mnewString", "($1 == null ? $3 = mnewString($2) : $3.length = $2)")
+    binaryExpr(p, n, r, "mnewString", "($1.length = $2)")
   of mSetLengthSeq:
     var x, y: TCompRes
     gen(p, n[1], x)
@@ -1974,8 +2031,7 @@ proc genMagic(p: PProc, n: PNode, r: var TCompRes) =
     let t = skipTypes(n[1].typ, abstractVar)[0]
     let (a, tmp) = maybeMakeTemp(p, n[1], x)
     let (b, tmp2) = maybeMakeTemp(p, n[2], y)
-    r.res = """if ($1 === null) $4 = [];
-               if ($4.length < $2) { for (var i=$4.length;i<$5;++i) $4.push($3); }
+    r.res = """if ($1.length < $2) { for (var i=$4.length;i<$5;++i) $4.push($3); }
                else { $4.length = $5; }""" % [a, b, createVar(p, t, false), tmp, tmp2]
     r.kind = resExpr
   of mCard: unaryExpr(p, n, r, "SetCard", "SetCard($1)")
@@ -2482,6 +2538,8 @@ proc newModule(g: ModuleGraph; module: PSym): BModule =
     g.backend = newGlobals()
   result.graph = g
   result.config = g.config
+  if sfSystemModule in module.flags:
+    PGlobals(g.backend).inSystem = true
 
 proc genHeader(): Rope =
   result = rope("""/* Generated by the Nim Compiler v$1 */
@@ -2555,7 +2613,7 @@ proc myProcess(b: PPassContext, n: PNode): PNode =
   if passes.skipCodegen(m.config, n): return n
   if m.module == nil: internalError(m.config, n.info, "myProcess")
   let globals = PGlobals(m.graph.backend)
-  var p = newProc(globals, m, nil, m.module.options)
+  var p = newInitProc(globals, m)
   p.unique = globals.unique
   genModule(p, n)
   p.g.code.add(p.locals)
@@ -2565,14 +2623,14 @@ proc wholeCode(graph: ModuleGraph; m: BModule): Rope =
   let globals = PGlobals(graph.backend)
   for prc in globals.forwarded:
     if not globals.generatedSyms.containsOrIncl(prc.id):
-      var p = newProc(globals, m, nil, m.module.options)
+      var p = newInitProc(globals, m)
       attachProc(p, prc)
 
   var disp = generateMethodDispatchers(graph)
   for i in 0..<disp.len:
     let prc = disp[i].sym
     if not globals.generatedSyms.containsOrIncl(prc.id):
-      var p = newProc(globals, m, nil, m.module.options)
+      var p = newInitProc(globals, m)
       attachProc(p, prc)
 
   result = globals.typeInfo & globals.constants & globals.code
@@ -2592,6 +2650,8 @@ proc myClose(graph: ModuleGraph; b: PPassContext, n: PNode): PNode =
   if sfMainModule in m.module.flags:
     for destructorCall in graph.globalDestructors:
       n.add destructorCall
+  if sfSystemModule in m.module.flags:
+    PGlobals(graph.backend).inSystem = false
   if passes.skipCodegen(m.config, n): return n
   if sfMainModule in m.module.flags:
     var code = genHeader() & wholeCode(graph, m)

lib/system/jssys.nim

@@ -397,24 +397,29 @@ else:
     """.}
 
 # Arithmetic:
+proc checkOverflowInt(a: int) {.asmNoStackFrame, compilerproc.} =
+  asm """
+    if (`a` > 2147483647 || `a` < -2147483648) `raiseOverflow`();
+  """
+
 proc addInt(a, b: int): int {.asmNoStackFrame, compilerproc.} =
   asm """
     var result = `a` + `b`;
-    if (result > 2147483647 || result < -2147483648) `raiseOverflow`();
+    `checkOverflowInt`(result);
     return result;
   """
 
 proc subInt(a, b: int): int {.asmNoStackFrame, compilerproc.} =
   asm """
     var result = `a` - `b`;
-    if (result > 2147483647 || result < -2147483648) `raiseOverflow`();
+    `checkOverflowInt`(result);
     return result;
   """
 
 proc mulInt(a, b: int): int {.asmNoStackFrame, compilerproc.} =
   asm """
     var result = `a` * `b`;
-    if (result > 2147483647 || result < -2147483648) `raiseOverflow`();
+    `checkOverflowInt`(result);
     return result;
   """
 
@@ -432,27 +437,29 @@ proc modInt(a, b: int): int {.asmNoStackFrame, compilerproc.} =
     return Math.trunc(`a` % `b`);
   """
 
+proc checkOverflowInt64(a: int) {.asmNoStackFrame, compilerproc.} =
+  asm """
+    if (`a` > 9223372036854775807 || `a` < -9223372036854775808) `raiseOverflow`();
+  """
+
 proc addInt64(a, b: int): int {.asmNoStackFrame, compilerproc.} =
   asm """
     var result = `a` + `b`;
-    if (result > 9223372036854775807
-    || result < -9223372036854775808) `raiseOverflow`();
+    `checkOverflowInt64`(result);
     return result;
   """
 
 proc subInt64(a, b: int): int {.asmNoStackFrame, compilerproc.} =
   asm """
     var result = `a` - `b`;
-    if (result > 9223372036854775807
-    || result < -9223372036854775808) `raiseOverflow`();
+    `checkOverflowInt64`(result);
     return result;
   """
 
 proc mulInt64(a, b: int): int {.asmNoStackFrame, compilerproc.} =
   asm """
     var result = `a` * `b`;
-    if (result > 9223372036854775807
-    || result < -9223372036854775808) `raiseOverflow`();
+    `checkOverflowInt64`(result);
     return result;
   """
 

lib/system/reprjs.nim

@@ -130,20 +130,6 @@ proc reprAux(result: var string, p: pointer, typ: PNimType, cl: var ReprClosure)
 
 proc reprArray(a: pointer, typ: PNimType,
               cl: var ReprClosure): string {.compilerRtl.} =
-  var isNilArrayOrSeq: bool
-  # isnil is not enough here as it would try to deref `a` without knowing what's inside
-  {. emit: """
-    if (`a` == null) {
-      `isNilArrayOrSeq` = true;
-    } else if (`a`[0] == null) {
-      `isNilArrayOrSeq` = true;
-    } else {
-      `isNilArrayOrSeq` = false;
-    };
-    """ .}
-  if typ.kind == tySequence and isNilArrayOrSeq:
-    return "nil"
-
   # We prepend @ to seq, the C backend prepends the pointer to the seq.
   result = if typ.kind == tySequence: "@[" else: "["
   var len: int = 0

tests/js/t12672.nim

@@ -0,0 +1,12 @@
+discard """
+  output: ""
+"""
+
+proc foo =
+  var x: seq[seq[int]]
+  for row in x.mitems:
+    let i = 1
+    echo row
+    inc row[i-1]
+
+foo()

tests/js/t14153.nim

@@ -0,0 +1,22 @@
+discard """
+  output: '''
+index 5 not in 0 .. 2
+index 5 not in 0 .. 2
+'''
+"""
+
+var x = @[1, 2, 3]
+
+try:
+  echo x[5]
+except IndexError:
+  echo getCurrentExceptionMsg()
+except:
+  doAssert false
+
+try:
+  x[5] = 8
+except IndexError:
+  echo getCurrentExceptionMsg()
+except:
+  doAssert false

tests/js/tjshello.nim

@@ -1,4 +1,5 @@
 discard """
+  cmd: "nim $target $options --stackTrace:off --lineTrace:off $file"
   output: "Hello World"
   maxcodesize: 1000
   ccodecheck: "!@'function'"
@@ -7,4 +8,3 @@ discard """
 import jsconsole
 
 console.log "Hello World"
-

tests/js/tjshello_stacktrace.nim

@@ -0,0 +1,9 @@
+discard """
+  output: "Hello World"
+  maxcodesize: 4500
+  ccodecheck: "!@'function'"
+"""
+
+import jsconsole
+
+console.log "Hello World"

tests/js/tmangle.nim

@@ -62,8 +62,8 @@ block:
     result = result and obj1.`&&`.addr[] == "bar".cstring
     result = result and obj2.`if` == 0
     result = result and obj2.`for` == 0
-    result = result and obj2.`==`.isNil()
-    result = result and obj2.`&&`.isNil()
+    result = result and obj2.`==`.len == 0
+    result = result and obj2.`&&`.len == 0
   echo test()
 
 # Test codegen for fields with uppercase letters:

tests/js/trepr.nim

@@ -308,21 +308,21 @@ b = 0,
 c = 0.0,
 d = '\0',
 e = eA,
-f = nil,
+f = "",
 g = {},
 h = {},
-i = [nil, nil, nil],
-j = nil,
+i = ["", "", ""],
+j = @[],
 k = 0,
-l = [a = nil,
-b = nil],
+l = [a = "",
+b = @[]],
 m = nil,
 n = nil,
-o = [Field0 = [a = nil,
-b = nil],
-Field1 = nil],
+o = [Field0 = [a = "",
+b = @[]],
+Field1 = ""],
 p = nil,
-q = nil]
+q = ""]
 """)
   doAssert(repr(cc) == """
 [a = 12,

tests/js/ttempgen.nim

@@ -0,0 +1,79 @@
+discard """
+  output: '''
+foo
+'''
+"""
+
+block: # #12672
+  var a = @[1]
+  let i = 1
+  inc a[i-1]
+
+  var b: seq[int]
+  doAssertRaises(IndexDefect): inc b[0]
+  doAssertRaises(IndexDefect): inc b[i-1]
+
+  var x: seq[seq[int]]
+  doAssertRaises(IndexDefect): # not TypeError
+    inc x[0][i-1]
+
+block: # #14087
+  type Obj = object
+    str: string
+
+  var s = @[Obj(str: "abc"), Obj(str: "def")]
+  s[1].str.add("ghi")
+  s[s.len - 1].str.add("jkl")
+  s[^1].str.add("mno")
+  s[s.high].str.add("pqr")
+
+  let slen = s.len
+  s[slen - 1].str.add("stu")
+
+  let shigh = s.high
+  s[shigh].str.add("vwx")
+
+  proc foo(): int =
+    echo "foo"
+    shigh
+  s[foo()].str.add("yz")
+  doAssert s[1].str == "defghijklmnopqrstuvwxyz"
+
+block: # #14117
+  type
+    A = object
+      case kind: bool
+      of true:
+        sons: seq[int]
+      else: discard
+
+  var a = A(kind: true)
+  doAssert a.sons.len == 0
+  a.sons.add(1)
+  doAssert a.sons.len == 1
+
+import tables
+
+block: # #13966
+  var t: Table[int8, array[int8, seq[tuple[]]]]
+
+  t[0] = default(array[int8, seq[tuple[]]])
+  t[0][0].add ()
+
+block: # #11783
+  proc fun(): string =
+    discard
+
+  var ret: string
+  ret.add fun()
+  doAssert ret == ""
+
+block: # #12256
+  var x: bool
+
+  doAssert x == false
+
+  reset x
+
+  doAssert x == false
+  doAssert x != true