fixes #4299 #12492 #10849; lambda lifting for JS backend (#23484)

fixes #4299 
fixes #12492 
fixes #10849

It binds `function` with `env`: `function.bind(:env)` to ease codegen
for now

changelog.md

@@ -15,6 +15,8 @@
  will no longer compile.
 - `internalNew` is removed from system, use `new` instead.
 
+- `bindMethod` in `std/jsffi` is deprecated, don't use it with closures.
+
 ## Standard library additions and changes
 
 [//]: # "Changes:"

compiler/jsgen.nim

@@ -106,25 +106,18 @@ type
  optionsStack: seq[TOptions]
  module: BModule
  g: PGlobals
- generatedParamCopies: IntSet
  beforeRetNeeded: bool
  unique: int # for temp identifier generation
  blocks: seq[TBlock]
  extraIndent: int
- up: PProc # up the call chain; required for closure support
  declaredGlobals: IntSet
  previousFileName: string # For frameInfo inside templates.
 
 template config*(p: PProc): ConfigRef = p.module.config
 
 proc indentLine(p: PProc, r: Rope): Rope =
  var p = p
- var ind = 0
- while true:
- inc ind, p.blocks.len + p.extraIndent
- if p.up == nil or p.up.prc != p.prc.owner:
- break
- p = p.up
+ let ind = p.blocks.len + p.extraIndent
  result = repeat(' ', ind*2) & r
 
 template line(p: PProc, added: string) =
@@ -843,6 +836,11 @@ proc arith(p: PProc, n: PNode, r: var TCompRes, op: TMagic) =
  gen(p, n[1], x)
  gen(p, n[2], y)
  r.res = "($# == $# && $# == $#)" % [x.address, y.address, x.res, y.res]
+ of mEqProc:
+ if skipTypes(n[1].typ, abstractInst).callConv == ccClosure:
+ binaryExpr(p, n, r, "cmpClosures", "cmpClosures($1, $2)")
+ else:
+ arithAux(p, n, r, op)
  else:
  arithAux(p, n, r, op)
  r.kind = resExpr
@@ -1204,8 +1202,16 @@ proc genIf(p: PProc, n: PNode, r: var TCompRes) =
  lineF(p, "}$n", [])
  line(p, repeat('}', toClose) & "\L")
 
-proc generateHeader(p: PProc, typ: PType): Rope =
+proc generateHeader(p: PProc, prc: PSym): Rope =
  result = ""
+ let typ = prc.typ
+ if typ.callConv == ccClosure:
+ # we treat Env as the `this` parameter of the function
+ # to keep it simple
+ let env = prc.ast[paramsPos].lastSon
+ assert env.kind == nkSym, "env is missing"
+ env.sym.loc.r = "this"
+
  for i in 1..<typ.n.len:
  assert(typ.n[i].kind == nkSym)
  var param = typ.n[i].sym
@@ -1238,7 +1244,7 @@ const
 
 proc needsNoCopy(p: PProc; y: PNode): bool =
  return y.kind in nodeKindsNeedNoCopy or
- ((mapType(y.typ) != etyBaseIndex or (y.kind == nkSym and y.sym.kind == skParam)) and
+ ((mapType(y.typ) != etyBaseIndex) and
  (skipTypes(y.typ, abstractInst).kind in
  {tyRef, tyPtr, tyLent, tyVar, tyCstring, tyProc, tyOwned} + IntegralTypes))
 
@@ -1589,27 +1595,7 @@ proc attachProc(p: PProc; s: PSym) =
 
 proc genProcForSymIfNeeded(p: PProc, s: PSym) =
  if not p.g.generatedSyms.containsOrIncl(s.id):
- let newp = genProc(p, s)
- var owner = p
- while owner != nil and owner.prc != s.owner:
- owner = owner.up
- if owner != nil: owner.locals.add(newp)
- else: attachProc(p, newp, s)
-
-proc genCopyForParamIfNeeded(p: PProc, n: PNode) =
- let s = n.sym
- if p.prc == s.owner or needsNoCopy(p, n):
- return
- var owner = p.up
- while true:
- if owner == nil:
- internalError(p.config, n.info, "couldn't find the owner proc of the closed over param: " & s.name.s)
- if owner.prc == s.owner:
- if not owner.generatedParamCopies.containsOrIncl(s.id):
- let copy = "$1 = nimCopy(null, $1, $2);$n" % [s.loc.r, genTypeInfo(p, s.typ)]
- owner.locals.add(owner.indentLine(copy))
- return
- owner = owner.up
+ attachProc(p, s)
 
 proc genVarInit(p: PProc, v: PSym, n: PNode)
 
@@ -1621,8 +1607,6 @@ proc genSym(p: PProc, n: PNode, r: var TCompRes) =
  internalError(p.config, n.info, "symbol has no generated name: " & s.name.s)
  if sfCompileTime in s.flags:
  genVarInit(p, s, if s.astdef != nil: s.astdef else: newNodeI(nkEmpty, s.info))
- if s.kind == skParam:
- genCopyForParamIfNeeded(p, n)
  let k = mapType(p, s.typ)
  if k == etyBaseIndex:
  r.typ = etyBaseIndex
@@ -1645,7 +1629,7 @@ proc genSym(p: PProc, n: PNode, r: var TCompRes) =
  if s.loc.r == "":
  internalError(p.config, n.info, "symbol has no generated name: " & s.name.s)
  r.res = s.loc.r
- of skProc, skFunc, skConverter, skMethod:
+ of skProc, skFunc, skConverter, skMethod, skIterator:
  if sfCompileTime in s.flags:
  localError(p.config, n.info, "request to generate code for .compileTime proc: " &
  s.name.s)
@@ -2076,6 +2060,17 @@ proc genVarInit(p: PProc, v: PSym, n: PNode) =
  dec p.extraIndent
  lineF(p, "}$n")
 
+proc genClosureVar(p: PProc, n: PNode) =
+ # assert n[2].kind != nkEmpty
+ # TODO: fixme transform `var env.x` into `var env.x = default()` after
+ # the order of transf and lambdalifting is fixed
+ if n[2].kind != nkEmpty:
+ genAsgnAux(p, n[0], n[2], false)
+ else:
+ var a: TCompRes = default(TCompRes)
+ gen(p, n[0], a)
+ line(p, runtimeFormat("$1 = $2;$n", [rdLoc(a), createVar(p, n[0].typ, false)]))
+
 proc genVarStmt(p: PProc, n: PNode) =
  for i in 0..<n.len:
  var a = n[i]
@@ -2085,15 +2080,17 @@ proc genVarStmt(p: PProc, n: PNode) =
  genStmt(p, unpacked)
  else:
  assert(a.kind == nkIdentDefs)
- assert(a[0].kind == nkSym)
- var v = a[0].sym
- if lfNoDecl notin v.loc.flags and sfImportc notin v.flags:
- genLineDir(p, a)
- if sfCompileTime notin v.flags:
- genVarInit(p, v, a[2])
- else:
- # lazy emit, done when it's actually used.
- if v.ast == nil: v.ast = a[2]
+ if a[0].kind == nkSym:
+ var v = a[0].sym
+ if lfNoDecl notin v.loc.flags and sfImportc notin v.flags:
+ genLineDir(p, a)
+ if sfCompileTime notin v.flags:
+ genVarInit(p, v, a[2])
+ else:
+ # lazy emit, done when it's actually used.
+ if v.ast == nil: v.ast = a[2]
+ else: # closure
+ genClosureVar(p, a)
 
 proc genConstant(p: PProc, c: PSym) =
  if lfNoDecl notin c.loc.flags and not p.g.generatedSyms.containsOrIncl(c.id):
@@ -2695,11 +2692,10 @@ proc genProc(oldProc: PProc, prc: PSym): Rope =
  #if gVerbosity >= 3:
  # echo "BEGIN generating code for: " & prc.name.s
  var p = newProc(oldProc.g, oldProc.module, prc.ast, prc.options)
- p.up = oldProc
  var returnStmt: Rope = ""
  var resultAsgn: Rope = ""
  var name = mangleName(p.module, prc)
- let header = generateHeader(p, prc.typ)
+ let header = generateHeader(p, prc)
  if prc.typ.returnType != nil and sfPure notin prc.flags:
  resultSym = prc.ast[resultPos].sym
  let mname = mangleName(p.module, resultSym)
@@ -2918,7 +2914,15 @@ proc gen(p: PProc, n: PNode, r: var TCompRes) =
  genInfixCall(p, n, r)
  else:
  genCall(p, n, r)
- of nkClosure: gen(p, n[0], r)
+ of nkClosure:
+ let tmp = getTemp(p)
+ var a: TCompRes = default(TCompRes)
+ var b: TCompRes = default(TCompRes)
+ gen(p, n[0], a)
+ gen(p, n[1], b)
+ lineF(p, "$1 = $2.bind($3); $1.ClP_0 = $2; $1.ClE_0 = $3;$n", [tmp, a.rdLoc, b.rdLoc])
+ r.res = tmp
+ r.kind = resVal
  of nkCurly: genSetConstr(p, n, r)
  of nkBracket: genArrayConstr(p, n, r)
  of nkPar, nkTupleConstr: genTupleConstr(p, n, r)
@@ -2950,9 +2954,7 @@ proc gen(p: PProc, n: PNode, r: var TCompRes) =
  let s = n[namePos].sym
  discard mangleName(p.module, s)
  r.res = s.loc.r
- if s.kind == skIterator and s.typ.callConv == TCallingConvention.ccClosure:
- globalError(p.config, n.info, "Closure iterators are not supported by JS backend!")
- elif lfNoDecl in s.loc.flags or s.magic notin generatedMagics: discard
+ if lfNoDecl in s.loc.flags or s.magic notin generatedMagics: discard
  elif not p.g.generatedSyms.containsOrIncl(s.id):
  p.locals.add(genProc(p, s))
  of nkType: r.res = genTypeInfo(p, n.typ)

compiler/lambdalifting.nim

@@ -239,11 +239,6 @@ proc interestingIterVar(s: PSym): bool {.inline.} =
 template isIterator*(owner: PSym): bool =
  owner.kind == skIterator and owner.typ.callConv == ccClosure
 
-proc liftingHarmful(conf: ConfigRef; owner: PSym): bool {.inline.} =
- ## lambda lifting can be harmful for JS-like code generators.
- let isCompileTime = sfCompileTime in owner.flags or owner.kind == skMacro
- result = conf.backend == backendJs and not isCompileTime
-
 proc createTypeBoundOpsLL(g: ModuleGraph; refType: PType; info: TLineInfo; idgen: IdGenerator; owner: PSym) =
  if owner.kind != skMacro:
  createTypeBoundOps(g, nil, refType.elementType, info, idgen)
@@ -260,7 +255,6 @@ proc genCreateEnv(env: PNode): PNode =
 
 proc liftIterSym*(g: ModuleGraph; n: PNode; idgen: IdGenerator; owner: PSym): PNode =
  # transforms (iter) to (let env = newClosure[iter](); (iter, env))
- if liftingHarmful(g.config, owner): return n
  let iter = n.sym
  assert iter.isIterator
 
@@ -883,14 +877,9 @@ proc liftIterToProc*(g: ModuleGraph; fn: PSym; body: PNode; ptrType: PType;
 
 proc liftLambdas*(g: ModuleGraph; fn: PSym, body: PNode; tooEarly: var bool;
  idgen: IdGenerator; flags: TransformFlags): PNode =
- # XXX backend == backendJs does not suffice! The compiletime stuff needs
- # the transformation even when compiling to JS ...
-
- # However we can do lifting for the stuff which is *only* compiletime.
  let isCompileTime = sfCompileTime in fn.flags or fn.kind == skMacro
 
- if body.kind == nkEmpty or (
- g.config.backend == backendJs and not isCompileTime) or
+ if body.kind == nkEmpty or
  (fn.skipGenericOwner.kind != skModule and force notin flags):
 
  # ignore forward declaration:
@@ -950,7 +939,6 @@ proc liftForLoop*(g: ModuleGraph; body: PNode; idgen: IdGenerator; owner: PSym):
  break
  ...
  """
- if liftingHarmful(g.config, owner): return body
  if not (body.kind == nkForStmt and body[^2].kind in nkCallKinds):
  localError(g.config, body.info, "ignored invalid for loop")
  return body

compiler/transf.nim

@@ -511,7 +511,6 @@ proc generateThunk(c: PTransf; prc: PNode, dest: PType): PNode =
 
  # we cannot generate a proper thunk here for GC-safety reasons
  # (see internal documentation):
- if c.graph.config.backend == backendJs: return prc
  result = newNodeIT(nkClosure, prc.info, dest)
  var conv = newNodeIT(nkHiddenSubConv, prc.info, dest)
  conv.add(newNodeI(nkEmpty, prc.info))

lib/js/jsffi.nim

@@ -468,7 +468,7 @@ proc replaceSyms(n: NimNode): NimNode =
  for i in 0..<n.len:
  result[i] = replaceSyms(n[i])
 
-macro bindMethod*(procedure: typed): auto =
+macro bindMethod*(procedure: typed): auto {.deprecated: "Don't use it with closures".} =
  ## Takes the name of a procedure and wraps it into a lambda missing the first
  ## argument, which passes the JavaScript builtin `this` as the first
  ## argument to the procedure. Returns the resulting lambda.

lib/system/jssys.nim

@@ -781,3 +781,15 @@ if (!Math.trunc) {
  };
 }
 """.}
+
+proc cmpClosures(a, b: JSRef): bool {.compilerproc, asmNoStackFrame.} =
+ # Both `a` and `b` need to be a closure
+ {.emit: """
+ if (`a` !== null && `a`.ClP_0 !== undefined &&
+ `b` !== null && `b`.ClP_0 !== undefined) {
+ return `a`.ClP_0 == `b`.ClP_0 && `a`.ClE_0 == `b`.ClE_0;
+ } else {
+ return `a` == `b`;
+ }
+ """
+ .}

tests/js/t21439.nim

@@ -1,8 +1,5 @@
-discard """
- action: "compile"
-"""
-
 proc test(a: openArray[string]): proc =
+ let a = @a
  result = proc =
  for i in a:
  discard i

tests/js/tjsffi.nim

@@ -180,7 +180,7 @@ block: # Test lit
 block: # Test bindMethod
  type TestObject = object
  a: int
- onWhatever: proc(e: int): int
+ onWhatever: proc(e: int): int {.nimcall.}
  proc handleWhatever(this: TestObject, e: int): int =
  e + this.a
  block:

tests/js/tjsffi_old.nim

@@ -279,8 +279,8 @@ block:
 block:
  type TestObject = object
  a: int
- onWhatever: proc(e: int): int
- proc handleWhatever(this: TestObject, e: int): int =
+ onWhatever: proc(e: int): int {.nimcall.}
+ proc handleWhatever(this: TestObject, e: int): int {.nimcall.} =
  e + this.a
  proc test(): bool =
  let obj = TestObject(a: 9, onWhatever: bindMethod(handleWhatever))

tests/stdlib/tclosures.nim

@@ -0,0 +1,47 @@
+discard """
+ targets: "c js"
+"""
+
+import std/assertions
+
+block: # bug #4299
+ proc scopeProc() =
+ proc normalProc() =
+ discard
+
+ proc genericProc[T]() =
+ normalProc()
+
+ genericProc[string]()
+
+ scopeProc()
+
+block: # bug #12492
+ proc foo() =
+ var i = 0
+ proc bar() =
+ inc i
+
+ bar()
+ doAssert i == 1
+
+ foo()
+ static:
+ foo()
+
+block: # bug #10849
+ type
+ Generic[T] = ref object
+ getState: proc(): T
+
+ proc newGeneric[T](): Generic[T] =
+ var state: T
+
+ proc getState[T](): T =
+ state
+
+ Generic[T](getState: getState)
+
+ let g = newGeneric[int]()
+ let state = g.getState()
+ doAssert state == 0