Fix nested finally handling in closureiters [backport] (#19933)

* Fix nested finally handling in closureiters

* Fix CI

* review comment

* third time the charm

* Update compiler/closureiters.nim

Co-authored-by: Dominik Picheta <dominikpicheta@googlemail.com>

Co-authored-by: Dominik Picheta <dominikpicheta@googlemail.com>

compiler/closureiters.nim

@@ -121,7 +121,10 @@
 #   yield 2
 #   if :unrollFinally: # This node is created by `newEndFinallyNode`
 #     if :curExc.isNil:
-#       return :tmpResult
+#       if nearestFinally == 0:
+#         return :tmpResult
+#       else:
+#         :state = nearestFinally # bubble up
 #     else:
 #       closureIterSetupExc(nil)
 #       raise
@@ -807,7 +810,10 @@ proc newEndFinallyNode(ctx: var Ctx, info: TLineInfo): PNode =
   # Generate the following code:
   #   if :unrollFinally:
   #       if :curExc.isNil:
-  #         return :tmpResult
+  #         if nearestFinally == 0:
+  #           return :tmpResult
+  #         else:
+  #           :state = nearestFinally # bubble up
   #       else:
   #         raise
   let curExc = ctx.newCurExcAccess()
@@ -816,11 +822,17 @@ proc newEndFinallyNode(ctx: var Ctx, info: TLineInfo): PNode =
   let cmp = newTree(nkCall, newSymNode(ctx.g.getSysMagic(info, "==", mEqRef), info), curExc, nilnode)
   cmp.typ = ctx.g.getSysType(info, tyBool)
 
-  let asgn = newTree(nkFastAsgn,
-    newSymNode(getClosureIterResult(ctx.g, ctx.fn, ctx.idgen), info),
-    ctx.newTmpResultAccess())
+  let retStmt =
+    if ctx.nearestFinally == 0:
+      # last finally, we can return
+      let asgn = newTree(nkFastAsgn,
+        newSymNode(getClosureIterResult(ctx.g, ctx.fn, ctx.idgen), info),
+        ctx.newTmpResultAccess())
+      newTree(nkReturnStmt, asgn)
+    else:
+      # bubble up to next finally
+      newTree(nkGotoState, ctx.g.newIntLit(info, ctx.nearestFinally))
 
-  let retStmt = newTree(nkReturnStmt, asgn)
   let branch = newTree(nkElifBranch, cmp, retStmt)
 
   let nullifyExc = newTree(nkCall, newSymNode(ctx.g.getCompilerProc("closureIterSetupExc")), nilnode)
@@ -864,6 +876,13 @@ proc transformReturnsInTry(ctx: var Ctx, n: PNode): PNode =
 
   of nkSkip:
     discard
+  of nkTryStmt:
+    if n.hasYields:
+      # the inner try will handle these transformations
+      discard
+    else:
+      for i in 0..<n.len:
+        n[i] = ctx.transformReturnsInTry(n[i])
   else:
     for i in 0..<n.len:
       n[i] = ctx.transformReturnsInTry(n[i])

tests/iter/titer_issues.nim

@@ -30,6 +30,14 @@ end
 @[1, 2]
 @[1, 2, 3]
 1
+nested finally
+outer finally
+nested finally
+outer finally
+nested finally
+outer finally
+nested finally
+outer finally
 '''
 """
 
@@ -287,3 +295,70 @@ block:
 
   for i in a(5):
     echo i
+
+block:
+  # bug #19911 (return in nested try)
+
+  # try yield -> try
+  iterator p1: int {.closure.} =
+    try:
+      yield 0
+      try:
+        return
+      finally:
+        echo "nested finally"
+      echo "shouldn't run"
+    finally:
+      echo "outer finally"
+    echo "shouldn't run"
+
+  for _ in p1():
+    discard
+
+  # try -> try yield
+  iterator p2: int {.closure.} =
+    try:
+      try:
+        yield 0
+        return
+      finally:
+        echo "nested finally"
+      echo "shouldn't run"
+    finally:
+      echo "outer finally"
+    echo "shouldn't run"
+
+  for _ in p2():
+    discard
+
+  # try yield -> try yield
+  iterator p3: int {.closure.} =
+    try:
+      yield 0
+      try:
+        yield 0
+        return
+      finally:
+        echo "nested finally"
+      echo "shouldn't run"
+    finally:
+      echo "outer finally"
+    echo "shouldn't run"
+
+  for _ in p3():
+    discard
+
+  # try -> try
+  iterator p4: int {.closure.} =
+    try:
+      try:
+        return
+      finally:
+        echo "nested finally"
+      echo "shouldn't run"
+    finally:
+      echo "outer finally"
+    echo "shouldn't run"
+
+  for _ in p4():
+    discard