implement `legacy:jsNoLambdaLifting` for compatibility (#23727)

6 files changed, 76 insertions, 19 deletions

diff --git a/changelog.md b/changelog.md
index a137b3e42..53d88b19f 100644
--- a/changelog.md
+++ b/changelog.md
@@ -17,6 +17,8 @@
 
 - `bindMethod` in `std/jsffi` is deprecated, don't use it with closures.
 
+- JS backend now supports lambda lifting for closures. Use `--legacy:jsNoLambdaLifting` to emulate old behavior.
+
 ## Standard library additions and changes
 
 [//]: # "Changes:"
diff --git a/compiler/jsgen.nim b/compiler/jsgen.nim
index 382f12a9d..1805d07b2 100644
--- a/compiler/jsgen.nim
+++ b/compiler/jsgen.nim
@@ -111,13 +111,25 @@ type
     blocks: seq[TBlock]
     extraIndent: int
     previousFileName: string  # For frameInfo inside templates.
+    # legacy: generatedParamCopies and up fields are used for jsNoLambdaLifting
+    generatedParamCopies: IntSet
+    up: PProc     # up the call chain; required for closure support
 
 template config*(p: PProc): ConfigRef = p.module.config
 
 proc indentLine(p: PProc, r: Rope): Rope =
   var p = p
-  let ind = p.blocks.len + p.extraIndent
-  result = repeat(' ', ind*2) & r
+  if jsNoLambdaLifting in p.config.legacyFeatures:
+    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
+    result = repeat(' ', ind*2) & r
+  else:
+    let ind = p.blocks.len + p.extraIndent
+    result = repeat(' ', ind*2) & r
 
 template line(p: PProc, added: string) =
   p.body.add(indentLine(p, rope(added)))
@@ -1200,12 +1212,13 @@ proc genIf(p: PProc, n: PNode, r: var TCompRes) =
 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"
+  if jsNoLambdaLifting notin p.config.legacyFeatures:
+    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)
@@ -1239,7 +1252,8 @@ const
 
 proc needsNoCopy(p: PProc; y: PNode): bool =
   return y.kind in nodeKindsNeedNoCopy or
-        ((mapType(y.typ) != etyBaseIndex) and
+        ((mapType(y.typ) != etyBaseIndex or
+          (jsNoLambdaLifting in p.config.legacyFeatures and y.kind == nkSym and y.sym.kind == skParam)) and
           (skipTypes(y.typ, abstractInst).kind in
             {tyRef, tyPtr, tyLent, tyVar, tyCstring, tyProc, tyOwned, tyOpenArray} + IntegralTypes))
 
@@ -1590,7 +1604,30 @@ proc attachProc(p: PProc; s: PSym) =
 
 proc genProcForSymIfNeeded(p: PProc, s: PSym) =
   if not p.g.generatedSyms.containsOrIncl(s.id):
-    attachProc(p, s)
+    if jsNoLambdaLifting in p.config.legacyFeatures:
+      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)
+    else:
+      attachProc(p, 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
 
 proc genVarInit(p: PProc, v: PSym, n: PNode)
 
@@ -1602,6 +1639,8 @@ 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 jsNoLambdaLifting in p.config.legacyFeatures and s.kind == skParam:
+      genCopyForParamIfNeeded(p, n)
     let k = mapType(p, s.typ)
     if k == etyBaseIndex:
       r.typ = etyBaseIndex
@@ -2688,6 +2727,7 @@ 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)
@@ -2911,14 +2951,17 @@ proc gen(p: PProc, n: PNode, r: var TCompRes) =
     else:
       genCall(p, n, 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
+    if jsNoLambdaLifting in p.config.legacyFeatures:
+      gen(p, n[0], r)
+    else:
+      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)
diff --git a/compiler/lambdalifting.nim b/compiler/lambdalifting.nim
index faa043cb4..e51fb3ae0 100644
--- a/compiler/lambdalifting.nim
+++ b/compiler/lambdalifting.nim
@@ -239,6 +239,11 @@ proc interestingIterVar(s: PSym): bool {.inline.} =
 template isIterator*(owner: PSym): bool =
   owner.kind == skIterator and owner.typ.callConv == ccClosure
 
+template liftingHarmful(conf: ConfigRef; owner: PSym): bool =
+  ## lambda lifting can be harmful for JS-like code generators.
+  let isCompileTime = sfCompileTime in owner.flags or owner.kind == skMacro
+  jsNoLambdaLifting in conf.legacyFeatures and 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)
@@ -255,6 +260,7 @@ 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
 
@@ -879,7 +885,8 @@ proc liftLambdas*(g: ModuleGraph; fn: PSym, body: PNode; tooEarly: var bool;
                   idgen: IdGenerator; flags: TransformFlags): PNode =
   let isCompileTime = sfCompileTime in fn.flags or fn.kind == skMacro
 
-  if body.kind == nkEmpty or
+  if body.kind == nkEmpty or (jsNoLambdaLifting in g.config.legacyFeatures and
+      g.config.backend == backendJs and not isCompileTime) or
       (fn.skipGenericOwner.kind != skModule and force notin flags):
 
     # ignore forward declaration:
@@ -939,6 +946,7 @@ 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
diff --git a/compiler/options.nim b/compiler/options.nim
index 356aa6cc8..a6a2b084e 100644
--- a/compiler/options.nim
+++ b/compiler/options.nim
@@ -246,6 +246,8 @@ type
     emitGenerics
       ## generics are emitted in the module that contains them.
       ## Useful for libraries that rely on local passC
+    jsNoLambdaLifting
+      ## Old transformation for closures in JS backend
 
   SymbolFilesOption* = enum
     disabledSf, writeOnlySf, readOnlySf, v2Sf, stressTest
diff --git a/compiler/transf.nim b/compiler/transf.nim
index 6888fc223..8dcb74729 100644
--- a/compiler/transf.nim
+++ b/compiler/transf.nim
@@ -513,6 +513,7 @@ proc generateThunk(c: PTransf; prc: PNode, dest: PType): PNode =
 
   # we cannot generate a proper thunk here for GC-safety reasons
   # (see internal documentation):
+  if jsNoLambdaLifting in c.graph.config.legacyFeatures and 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))
diff --git a/tests/js/tjsffi.nim b/tests/js/tjsffi.nim
index 265ae52e9..f27ea5546 100644
--- a/tests/js/tjsffi.nim
+++ b/tests/js/tjsffi.nim
@@ -1,4 +1,5 @@
 discard """
+matrix: "--legacy:jsnolambdalifting;"
 output: '''
 3
 2