Fixes #21235, #23602, #24978, #25018 (#25030)

Reworked closureiter transformation.

- Convolutedly nested finallies should cause no problems now.
- CurrentException state now follows nim runtime rules (pushes and pops
appropriately), and mimics normal code, which is somewhat buggy, see
#25031
- Previously state optimization (removing empty states or extra jumps)
missed some opportunities, I've reimplemented it to do everything
possible to optimize the states. At this point any extra states or jumps
should be considered a bug.

The resulting codegen (compiled binaries) is also slightly smaller.

**BUT:**
- I had to change C++ reraising logic, see expt.nim. Because with
closure iters `currentException` is not always in sync with C++'s notion
of current exception. From my tests and understanding of C++ runtime
there should not be any problems, but I'm only 99% sure :)
- I've reused `nfNoRewrite` flag in one specific case during the
transformation. This flag is also used in term-rewriting logic. Again,
99% sure, these 2 scenarios will never intersect.

(cherry picked from commit 36f8cefa85)

lib/system/embedded.nim

@@ -50,9 +50,6 @@ proc writeStackTrace() = discard
 proc unsetControlCHook() = discard
 proc setControlCHook(hook: proc () {.noconv.}) = discard
 
-proc closureIterSetupExc(e: ref Exception) {.compilerproc, inline.} =
-  sysFatal(ReraiseDefect, "exception handling is not available")
-
 when gotoBasedExceptions:
   var nimInErrorMode {.threadvar.}: bool
 

lib/system/excpt.nim

@@ -161,9 +161,6 @@ proc popCurrentException {.compilerRtl, inl.} =
 proc popCurrentExceptionEx(id: uint) {.compilerRtl.} =
   discard "only for bootstrapping compatbility"
 
-proc closureIterSetupExc(e: ref Exception) {.compilerproc, inline.} =
-  currException = e
-
 # some platforms have native support for stack traces:
 const
   nativeStackTraceSupported = (defined(macosx) or defined(linux)) and
@@ -464,11 +461,9 @@ proc raiseExceptionAux(e: sink(ref Exception)) {.nodestroy.} =
   if globalRaiseHook != nil:
     if not globalRaiseHook(e): return
   when defined(cpp) and not defined(noCppExceptions) and not gotoBasedExceptions:
-    if e == currException:
-      {.emit: "throw;".}
-    else:
+    if e != currException:
       pushCurrentException(e)
-      {.emit: "throw `e`;".}
+    {.emit: "throw `e`;".}
   elif quirkyExceptions or gotoBasedExceptions:
     pushCurrentException(e)
     when gotoBasedExceptions:

lib/system/jssys.nim

@@ -72,8 +72,10 @@ proc getCurrentExceptionMsg*(): string =
 proc setCurrentException*(exc: ref Exception) =
   lastJSError = cast[PJSError](exc)
 
-proc closureIterSetupExc(e: ref Exception) {.compilerproc, inline.} =
-  ## Used to set up exception handling for closure iterators
+proc pushCurrentException(e: sink(ref Exception)) {.compilerRtl, inline.} =
+  ## Used to set up exception handling for closure iterators.
+
+  # XXX Shouldn't there be exception stack like in excpt.nim?
   setCurrentException(e)
 
 proc auxWriteStackTrace(f: PCallFrame): string =

tests/async/t23602.nim

@@ -0,0 +1,27 @@
+import std/asyncdispatch
+
+proc errval {.async.} =
+  raise newException(ValueError, "err")
+  
+proc err {.async.} =
+  try:
+    doAssert false
+  finally:
+    echo "finally"
+    # removing the following code will propagate the AssertionDefect
+    try:
+      await errval()
+    except ValueError:
+      echo "valueError"
+
+proc main {.async.} =
+  let errFut = err()
+  await errFut
+
+var ok = false
+try:
+  waitFor main()
+except AssertionDefect:
+  ok = true
+
+doAssert(ok)

tests/iter/tyieldintry.nim

@@ -26,10 +26,10 @@ proc testClosureIterAux(it: iterator(): int, exceptionExpected: bool, expectedRe
   if closureIterResult != @expectedResults or exceptionCaught != exceptionExpected:
     if closureIterResult != @expectedResults:
       echo "Expected: ", @expectedResults
-      echo "Actual: ", closureIterResult
+      echo "Actual:   ", closureIterResult
     if exceptionCaught != exceptionExpected:
       echo "Expected exception: ", exceptionExpected
-      echo "Got exception: ", exceptionCaught
+      echo "Got exception:      ", exceptionCaught
     doAssert(false)
 
 proc test(it: iterator(): int, expectedResults: varargs[int]) =
@@ -182,6 +182,57 @@ block:
 
   test(it, 0, 1, 2, 3)
 
+block: # Wrong except
+  iterator it(): int {.closure.} =
+    try:
+      try:
+        yield 0
+        raiseTestError()
+      except ValueError:
+        doAssert(false, "Unreachable")
+      finally:
+        checkpoint(1)
+    except ValueError:
+      yield 123
+      return
+
+    checkpoint(123)
+
+  testExc(it, 0, 1)
+
+block: # Nested except without finally
+  iterator it(): int {.closure.} =
+    try:
+      try:
+        yield 0
+        raiseTestError()
+      except ValueError:
+        doAssert(false, "Unreachable")
+    except ValueError:
+      yield 123
+
+    checkpoint(123)
+
+  testExc(it, 0)
+
+block: # Return in except with no finallies around
+  iterator it(): int {.closure.} =
+    try:
+      try:
+        yield 0
+        raiseTestError()
+      except ValueError:
+        doAssert(false, "Unreachable")
+      finally:
+        checkpoint(1)
+    except TestError:
+      yield 2
+      return
+
+    checkpoint(123)
+
+  test(it, 0, 1, 2)
+
 block:
   iterator it(): int {.closure.} =
     try:
@@ -527,3 +578,164 @@ block: # Locals present in only 1 state should be on the stack
     yield a
     yield b
   test(it, 1, 2)
+
+block: # Complex finallies (#24978)
+  iterator it(): int {.closure.} =
+    try:
+      for i in 1..2:
+        try:
+          yield i + 10
+        except:
+          doAssert(false, "Should not get here")
+        checkpoint(i + 20)
+      raiseTestError()
+    finally:
+      for i in 3..4:
+        try:
+          yield i + 30
+        except:
+          doAssert(false, "Should not get here")
+        finally:
+          checkpoint(i + 40)
+        checkpoint(i + 50)
+      checkpoint(100)
+
+  testExc(it, 11, 21, 12, 22, 33, 43, 53, 34, 44, 54, 100)
+
+block: # break
+  iterator it(): int {.closure.} =
+    while true:
+      try:
+        yield 1
+        while true:
+          yield 2
+          if true:
+            break
+        break
+      finally:
+        var localHere = 3
+        checkpoint(localHere)
+
+  test(it, 1, 2, 3)
+
+block: # break
+  iterator it(): int {.closure.} =
+    while true:
+      try:
+        try:
+          yield 1
+          while true:
+            yield 2
+            break
+          break
+        finally:
+          var localHere = 4
+          yield 3
+          checkpoint(localHere)
+        doAssert(false, "Should not get here")
+      finally:
+        yield 5
+        checkpoint(6)
+      doAssert(false, "Should not reach here")
+
+  test(it, 1, 2, 3, 4, 5, 6)
+
+block: # continue
+  iterator it(): int {.closure.} =
+    for i in 1 .. 3:
+      try:
+        try:
+          yield i + 10
+          while true:
+            yield i + 20
+            break
+          if i == 2:
+            continue
+          checkpoint(i + 30)
+        finally:
+          yield 3
+          checkpoint(4)
+        checkpoint(5)
+      finally:
+        yield 6
+        checkpoint(7)
+
+  test(it, 11, 21, 31, 3, 4, 5, 6, 7, 12, 22, 3, 4, 6, 7, 13, 23, 33, 3, 4, 5, 6, 7)
+
+block: # return without finally
+  iterator it(): int {.closure.} =
+    try:
+      yield 1
+      if true:
+        return
+    except:
+      doAssert(false, "Unreachable")
+    yield 2
+
+  test(it, 1)
+
+block: # return in finally
+  iterator it(): int {.closure.} =
+    try:
+      yield 1
+    except:
+      doAssert(false, "Unreachable")
+    finally:
+      return
+    yield 2
+
+  test(it, 1)
+
+block: # launch iter with current exception
+  iterator it(): int {.closure.} =
+    try:
+      yield 1
+    finally:
+      discard
+
+  try:
+    raise newException(ValueError, "")
+  except:
+    test(it, 1)
+
+block: #21235
+  proc myFunc() =
+    iterator myFuncIter(): int {.closure.} =
+      if false:
+        try:
+          yield 5
+        except:
+          discard
+    var nameIterVar = myFuncIter
+    discard nameIterVar()
+
+  var ok = false
+  try:
+    try:
+      raise ValueError.newException("foo")
+    finally:
+      myFunc()
+  except ValueError:
+    ok = true
+  doAssert(ok)
+
+block: # break in for without yield in try
+  iterator it(): int {.closure.} =
+    try:
+      block:
+        checkpoint(1)
+        for i in 0 .. 10:
+          checkpoint(2)
+          break
+        checkpoint(3)
+
+      try:
+        yield 4
+      except:
+        checkpoint(123)
+    except:
+      discard
+    finally:
+      checkpoint(5)
+
+  test(it, 1, 2, 3, 4, 5)