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,12 +106,10 @@ 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.
 
@@ -119,12 +117,7 @@ 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