Skip to content

Commit

Permalink
many bugfixes for js (#14158)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
metagn authored Apr 29, 2020
1 parent a297c01 commit 707367e
Show file tree
Hide file tree
Showing 10 changed files with 240 additions and 65 deletions.
120 changes: 90 additions & 30 deletions compiler/jsgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ type
generatedSyms: IntSet
typeInfoGenerated: IntSet
unique: int # for temp identifier generation
inSystem: bool

PProc = ref TProc
TProc = object
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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]
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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])

Expand All @@ -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)
Expand Down Expand Up @@ -1950,32 +2007,31 @@ 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)
gen(p, n[2], y)
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)")
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand Down
25 changes: 16 additions & 9 deletions lib/system/jssys.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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;
"""

Expand All @@ -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;
"""

Expand Down
14 changes: 0 additions & 14 deletions lib/system/reprjs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions tests/js/t12672.nim
Original file line number Diff line number Diff line change
@@ -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()
22 changes: 22 additions & 0 deletions tests/js/t14153.nim
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 707367e

Please sign in to comment.