somewhat working closures

8 files changed, 95 insertions, 33 deletions

diff --git a/compiler/cgen.nim b/compiler/cgen.nim
index e8073b555..73bd79ce8 100755
--- a/compiler/cgen.nim
+++ b/compiler/cgen.nim
@@ -571,7 +571,10 @@ proc deinitFrame(p: BProc): PRope =
 proc closureSetup(p: BProc, prc: PSym) =
   if prc.typ.callConv != ccClosure: return
   # prc.ast[paramsPos].last contains the type we're after:
-  var env = lastSon(prc.ast[paramsPos]).sym
+  var ls = lastSon(prc.ast[paramsPos])
+  if ls.kind != nkSym:
+    InternalError(prc.info, "closure generation failed")
+  var env = ls.sym
   #echo "created environment: ", env.id, " for ", prc.name.s
   assignLocalVar(p, env)
   # generate cast assignment:
diff --git a/compiler/evals.nim b/compiler/evals.nim
index 5c77a4d94..eb193cb9e 100755
--- a/compiler/evals.nim
+++ b/compiler/evals.nim
@@ -501,7 +501,8 @@ proc evalSym(c: PEvalContext, n: PNode, flags: TEvalFlags): PNode =
       result = evalGlobalVar(c, s, flags)
   of skParam:
     # XXX what about LValue?
-    result = c.tos.params[s.position + 1]
+    if s.position + 1 <% c.tos.params.len:
+      result = c.tos.params[s.position + 1]
   of skConst: result = s.ast
   of skEnumField: result = newIntNodeT(s.position, n)
   else: result = nil
diff --git a/compiler/lambdalifting.nim b/compiler/lambdalifting.nim
index 2b0aba147..219fead2c 100644
--- a/compiler/lambdalifting.nim
+++ b/compiler/lambdalifting.nim
@@ -218,8 +218,29 @@ proc isInnerProc(s, outerProc: PSym): bool {.inline.} =
     s.owner == outerProc and not isGenericRoutine(s)
   #s.typ.callConv == ccClosure
 
+proc addClosureParam(i: PInnerContext, e: PEnv) =
+  var cp = newSym(skParam, getIdent(paramname), i.fn)
+  cp.info = i.fn.info
+  incl(cp.flags, sfFromGeneric)
+  cp.typ = newType(tyRef, i.fn)
+  addSon(cp.typ, e.tup)
+  i.closureParam = cp
+  addHiddenParam(i.fn, i.closureParam)
+  #echo "closure param added for ", i.fn.name.s, " ", i.fn.id
+
+proc dummyClosureParam(o: POuterContext, i: PInnerContext) =
+  var e = o.currentEnv
+  if IdTableGet(o.lambdasToEnv, i.fn) == nil:
+    IdTablePut(o.lambdasToEnv, i.fn, e)
+  if i.closureParam == nil: addClosureParam(i, e)
+
 proc captureVar(o: POuterContext, i: PInnerContext, local: PSym, 
                 info: TLineInfo) =
+  # for inlined variables the owner is still wrong, so it can happen that it's
+  # not a captured variable at all ... *sigh* 
+  var it = PEnv(IdTableGet(o.localsToEnv, local))
+  if it == nil: return
+
   # we need to remember which inner most closure belongs to this lambda:
   var e = o.currentEnv
   if IdTableGet(o.lambdasToEnv, i.fn) == nil:
@@ -227,19 +248,10 @@ proc captureVar(o: POuterContext, i: PInnerContext, local: PSym,
 
   # variable already captured:
   if IdNodeTableGet(i.localsToAccess, local) != nil: return
-  if i.closureParam == nil:
-    var cp = newSym(skParam, getIdent(paramname), i.fn)
-    cp.info = i.fn.info
-    incl(cp.flags, sfFromGeneric)
-    cp.typ = newType(tyRef, i.fn)
-    addSon(cp.typ, e.tup)
-    i.closureParam = cp
-    addHiddenParam(i.fn, i.closureParam)
+  if i.closureParam == nil: addClosureParam(i, e)
   
   # check which environment `local` belongs to:
   var access = newSymNode(i.closureParam)
-  var it = PEnv(IdTableGet(o.localsToEnv, local))
-  assert it != nil
   addCapturedVar(it, local)
   if it == e:
     # common case: local directly in current environment:
@@ -325,6 +337,9 @@ proc searchForInnerProcs(o: POuterContext, n: PNode) =
       var inner = newInnerContext(n.sym)
       let body = n.sym.getBody
       gatherVars(o, inner, body)
+      # dummy closure param needed?
+      if inner.closureParam == nil and n.sym.typ.callConv == ccClosure:
+        dummyClosureParam(o, inner)
       let ti = transformInnerProc(o, inner, body)
       if ti != nil: n.sym.ast.sons[bodyPos] = ti
   of nkLambdaKinds:
@@ -425,9 +440,19 @@ proc transformOuterProc(o: POuterContext, n: PNode): PNode =
     if closure != nil:
       # we need to replace the lambda with '(lambda, env)': 
       let a = closure.closure
-      assert a != nil
-      return makeClosure(local, a, n.info)
-  
+      if a != nil:
+        return makeClosure(local, a, n.info)
+      else:
+        # can happen for dummy closures:
+        var scope = closure.attachedNode
+        assert scope.kind == nkStmtList
+        if scope.sons[0].kind == nkEmpty:
+          # change the empty node to contain the closure construction:
+          scope.sons[0] = generateClosureCreation(o, closure)
+        let x = closure.closure
+        assert x != nil
+        return makeClosure(local, x, n.info)
+    
     if not contains(o.capturedVars, local.id): return
     var env = PEnv(IdTableGet(o.localsToEnv, local))
     if env == nil: return
@@ -474,5 +499,6 @@ proc liftLambdas(fn: PSym, body: PNode): PNode =
 
 proc liftLambdas*(n: PNode): PNode =
   assert n.kind in procDefs
+  if gCmd == cmdCompileToEcmaScript: return n
   var s = n.sons[namePos].sym
   result = liftLambdas(s, s.getBody)
diff --git a/compiler/seminst.nim b/compiler/seminst.nim
index 85c68923c..f44b80223 100755
--- a/compiler/seminst.nim
+++ b/compiler/seminst.nim
@@ -145,7 +145,9 @@ proc generateInstance(c: PContext, fn: PSym, pt: TIdTable,
   c.friendModule = getModule(fn)
   result = copySym(fn, false)
   incl(result.flags, sfFromGeneric)
-  result.owner = getCurrOwner().owner
+  # keep the owner if it's an inner proc (for proper closure transformations):
+  if fn.owner.kind == skModule:
+    result.owner = getCurrOwner().owner
   # careful! we copy the whole AST including the possibly nil body!
   var n = copyTree(fn.ast)
   result.ast = n
diff --git a/compiler/transf.nim b/compiler/transf.nim
index b10310e14..c9e226c87 100755
--- a/compiler/transf.nim
+++ b/compiler/transf.nim
@@ -377,6 +377,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 gCmd == cmdCompileToEcmaScript: return prc
   result = newNodeIT(nkClosure, prc.info, dest)
   var conv = newNodeIT(nkHiddenStdConv, prc.info, dest)
   conv.add(emptyNode)
@@ -506,15 +507,18 @@ proc transformFor(c: PTransf, n: PNode): PTransNode =
   if call.kind notin nkCallKinds or call.sons[0].kind != nkSym:
     InternalError(call.info, "transformFor")
   
-  var newC = newTransCon(call.sons[0].sym)
+  # Bugfix: inlined locals belong to the invoking routine, not to the invoked
+  # iterator!
+  let iter = call.sons[0].sym
+  var newC = newTransCon(getCurrOwner(c))
   newC.forStmt = n
   newC.forLoopBody = loopBody
-  if newC.owner.kind != skIterator: InternalError(call.info, "transformFor") 
+  if iter.kind != skIterator: InternalError(call.info, "transformFor") 
   # generate access statements for the parameters (unless they are constant)
   pushTransCon(c, newC)
   for i in countup(1, sonsLen(call) - 1): 
     var arg = transform(c, call.sons[i]).pnode
-    var formal = skipTypes(newC.owner.typ, abstractInst).n.sons[i].sym 
+    var formal = skipTypes(iter.typ, abstractInst).n.sons[i].sym 
     case putArgInto(arg, formal.typ)
     of paDirectMapping: 
       IdNodeTablePut(newC.mapping, formal, arg)
@@ -528,7 +532,7 @@ proc transformFor(c: PTransf, n: PNode): PTransNode =
       assert(skipTypes(formal.typ, abstractInst).kind == tyVar)
       IdNodeTablePut(newC.mapping, formal, arg)
       # XXX BUG still not correct if the arg has a side effect!
-  var body = newC.owner.getBody
+  var body = iter.getBody
   pushInfoContext(n.info)
   inc(c.inlining)
   add(result, transform(c, body))
@@ -647,6 +651,9 @@ proc transform(c: PTransf, n: PNode): PTransNode =
     if n.sons[genericParamsPos].kind == nkEmpty:
       var s = n.sons[namePos].sym
       n.sons[bodyPos] = PNode(transform(c, s.getBody))
+      if s.ast.sons[bodyPos] != n.sons[bodyPos]:
+        # somehow this can happen ... :-/
+        s.ast.sons[bodyPos] = n.sons[bodyPos]
       n.sons[bodyPos] = liftLambdas(n)
       if n.kind == nkMethodDef: methodDef(s, false)
     result = PTransNode(n)
diff --git a/tests/reject/tinvalidclosure.nim b/tests/reject/tinvalidclosure.nim
new file mode 100644
index 000000000..f5ac3a5f0
--- /dev/null
+++ b/tests/reject/tinvalidclosure.nim
@@ -0,0 +1,7 @@
+discard """
+  line: 6
+  errormsg: "'ugh' cannot have 'closure' calling convention"
+"""
+
+proc ugh[T](x: T) {.closure.} =
+  echo "ugha"
diff --git a/tests/run/tclosure2.nim b/tests/run/tclosure2.nim
index 5a1cb8075..113c98250 100644
--- a/tests/run/tclosure2.nim
+++ b/tests/run/tclosure2.nim
@@ -1,20 +1,30 @@
 discard """
-  output: '''1
+  output: '''0
+11
+1
+11
 2
+11
 3
+11
 4
+11
 5
+11
 6
+11
 7
+11
 8
-9
-10
 11
+9
 11
 py
 py
 py
-py'''
+py
+px
+6'''
 """
 
 when true:
@@ -34,7 +44,7 @@ when true:
 
   ax()
 
-when false:
+when true:
   proc accumulator(start: int): (proc(): int {.closure.}) =
     var x = start-1
     #let dummy = proc =
@@ -62,8 +72,14 @@ when false:
   outer()
 
 
-when false:
-  proc outer =
+when true:
+  proc outer2 =
+    var errorValue = 3
+    proc fac[T](n: T): T =
+      if n < 0: result = errorValue
+      elif n <= 1: result = 1
+      else: result = n * fac(n-1)
+  
     proc px() {.closure.} =
       echo "px"
 
@@ -76,7 +92,9 @@ when false:
         "xyz": py
       }
     mapping[0][1]()
+    
+    echo fac(3)
 
 
-  outer()
+  outer2()
 
diff --git a/todo.txt b/todo.txt
index aa914bf84..6bbfe0289 100755
--- a/todo.txt
+++ b/todo.txt
@@ -8,12 +8,9 @@ version 0.9.0
 - ``=`` should be overloadable; requires specialization for ``=``
 - fix remaining generics bugs
 - fix remaining closure bugs:
+  - make toplevel but in a scope vars local; make procs there inner procs
   - fix evals.nim with closures
-  - deactivate lambda lifting for JS backend
-  - Test capture of for loop vars; test generics;
-  - test constant closures
-  - implement closures that support nesting of blocks > 1
-  - implement closures that support nesting of *procs* > 1
+  - test sequence of closures; especially that the GC does not leak for those!
 - implement proper coroutines
 
 - document 'do' notation
@@ -129,6 +126,7 @@ Low priority
 - activate more thread tests
 - implement ``--script:sh|bat`` command line option; think about script 
   generation
+- implement closures that support nesting of *procs* > 1
 
 
 Further optimization ideas