rewritten goto based exception handling; much cleaner implementation;… (#13677)

* rewritten goto based exception handling; much cleaner implementation; fixes #13668

4 files changed, 79 insertions, 26 deletions

diff --git a/compiler/ccgstmts.nim b/compiler/ccgstmts.nim
index 1c63ebaea..49098959b 100644
--- a/compiler/ccgstmts.nim
+++ b/compiler/ccgstmts.nim
@@ -1040,27 +1040,14 @@ proc genTryGoto(p: BProc; t: PNode; d: var TLoc) =
   let lab = p.labels
   p.nestedTryStmts.add((fin, false, Natural lab))
 
-  if not bodyCanRaise(p, t):
-    # optimize away the 'try' block:
-    expr(p, t[0], d)
-    linefmt(p, cpsStmts, "LA$1_: ;$n", [lab])
-    if fin != nil:
-      genStmts(p, fin[0])
-    discard pop(p.nestedTryStmts)
-    return
-
   p.flags.incl nimErrorFlagAccessed
-  p.procSec(cpsLocals).add(ropecg(p.module, "NI oldNimErr$1_;$n", [lab]))
-  linefmt(p, cpsStmts, "oldNimErr$1_ = *nimErr_; *nimErr_ = 0;;$n", [lab])
-
   expr(p, t[0], d)
 
   if 1 < t.len and t[1].kind == nkExceptBranch:
     startBlock(p, "if (NIM_UNLIKELY(*nimErr_)) {$n")
   else:
     startBlock(p)
-  # pretend we did handle the error for the safe execution of the sections:
-  linefmt(p, cpsStmts, "LA$1_: oldNimErr$1_ = *nimErr_; *nimErr_ = 0;$n", [lab])
+  linefmt(p, cpsStmts, "LA$1_:;$n", [lab])
 
   p.nestedTryStmts[^1].inExcept = true
   var i = 1
@@ -1077,7 +1064,7 @@ proc genTryGoto(p: BProc; t: PNode; d: var TLoc) =
       if i > 1: lineF(p, cpsStmts, "else", [])
       startBlock(p)
       # we handled the exception, remember this:
-      linefmt(p, cpsStmts, "--oldNimErr$1_;$n", [lab])
+      linefmt(p, cpsStmts, "*nimErr_ = NIM_FALSE;$n", [])
       expr(p, t[i][0], d)
     else:
       var orExpr: Rope = nil
@@ -1094,7 +1081,7 @@ proc genTryGoto(p: BProc; t: PNode; d: var TLoc) =
       if i > 1: line(p, cpsStmts, "else ")
       startBlock(p, "if ($1) {$n", [orExpr])
       # we handled the exception, remember this:
-      linefmt(p, cpsStmts, "--oldNimErr$1_;$n", [lab])
+      linefmt(p, cpsStmts, "*nimErr_ = NIM_FALSE;$n", [])
       expr(p, t[i][^1], d)
 
     linefmt(p, cpsStmts, "#popCurrentException();$n", [])
@@ -1105,7 +1092,6 @@ proc genTryGoto(p: BProc; t: PNode; d: var TLoc) =
   discard pop(p.nestedTryStmts)
   endBlock(p)
 
-  #linefmt(p, cpsStmts, "LA$1_:;$n", [lab+1])
   if i < t.len and t[i].kind == nkFinally:
     startBlock(p)
     if not bodyCanRaise(p, t[i][0]):
@@ -1114,8 +1100,8 @@ proc genTryGoto(p: BProc; t: PNode; d: var TLoc) =
       genStmts(p, t[i][0])
     else:
       # pretend we did handle the error for the safe execution of the 'finally' section:
-      p.procSec(cpsLocals).add(ropecg(p.module, "NI oldNimErrFin$1_;$n", [lab]))
-      linefmt(p, cpsStmts, "oldNimErrFin$1_ = *nimErr_; *nimErr_ = 0;$n", [lab])
+      p.procSec(cpsLocals).add(ropecg(p.module, "NIM_BOOL oldNimErrFin$1_;$n", [lab]))
+      linefmt(p, cpsStmts, "oldNimErrFin$1_ = *nimErr_; *nimErr_ = NIM_FALSE;$n", [lab])
       genStmts(p, t[i][0])
       # this is correct for all these cases:
       # 1. finally is run during ordinary control flow
@@ -1123,10 +1109,8 @@ proc genTryGoto(p: BProc; t: PNode; d: var TLoc) =
       #    error back to nil.
       # 3. finally is run for exception handling code without any 'except'
       #    handler present or only handlers that did not match.
-      linefmt(p, cpsStmts, "*nimErr_ += oldNimErr$1_ + (*nimErr_ - oldNimErrFin$1_); oldNimErr$1_ = 0;$n", [lab])
+      linefmt(p, cpsStmts, "*nimErr_ = oldNimErrFin$1_;$n", [lab])
     endBlock(p)
-  # restore the real error value:
-  linefmt(p, cpsStmts, "*nimErr_ += oldNimErr$1_;$n", [lab])
   if p.prc != nil: raiseExit(p)
 
 proc genTrySetjmp(p: BProc, t: PNode, d: var TLoc) =
diff --git a/compiler/cgen.nim b/compiler/cgen.nim
index 06507b6e6..10dda0ca9 100644
--- a/compiler/cgen.nim
+++ b/compiler/cgen.nim
@@ -975,7 +975,7 @@ proc genProcBody(p: BProc; procBody: PNode) =
   genStmts(p, procBody) # modifies p.locals, p.init, etc.
   if {nimErrorFlagAccessed, nimErrorFlagDeclared} * p.flags == {nimErrorFlagAccessed}:
     p.flags.incl nimErrorFlagDeclared
-    p.blocks[0].sections[cpsLocals].add(ropecg(p.module, "NI* nimErr_;$n", []))
+    p.blocks[0].sections[cpsLocals].add(ropecg(p.module, "NIM_BOOL* nimErr_;$n", []))
     p.blocks[0].sections[cpsInit].add(ropecg(p.module, "nimErr_ = #nimErrorFlag();$n", []))
 
 proc genProcAux(m: BModule, prc: PSym) =
diff --git a/lib/system/excpt.nim b/lib/system/excpt.nim
index a9c7ba02a..8337e0351 100644
--- a/lib/system/excpt.nim
+++ b/lib/system/excpt.nim
@@ -420,16 +420,16 @@ proc nimLeaveFinally() {.compilerRtl.} =
       quit(1)
 
 when gotoBasedExceptions:
-  var nimInErrorMode {.threadvar.}: int
+  var nimInErrorMode {.threadvar.}: bool
 
-  proc nimErrorFlag(): ptr int {.compilerRtl, inl.} =
+  proc nimErrorFlag(): ptr bool {.compilerRtl, inl.} =
     result = addr(nimInErrorMode)
 
   proc nimTestErrorFlag() {.compilerRtl.} =
     ## This proc must be called before ``currException`` is destroyed.
     ## It also must be called at the end of every thread to ensure no
     ## error is swallowed.
-    if currException != nil:
+    if nimInErrorMode and currException != nil:
       reportUnhandledError(currException)
       currException = nil
       quit(1)
diff --git a/tests/destructor/tgotoexceptions8.nim b/tests/destructor/tgotoexceptions8.nim
new file mode 100644
index 000000000..e968cdce2
--- /dev/null
+++ b/tests/destructor/tgotoexceptions8.nim
@@ -0,0 +1,69 @@
+discard """
+  output: '''A
+B
+X
+inner finally
+Y
+outer finally
+msg1
+msg2
+finally2
+finally1'''
+  cmd: "nim c --gc:arc $file"
+"""
+
+# bug #13668
+
+proc main =
+  try:
+    try:
+      raise newException(IOError, "IOError")
+
+    except:
+      echo "A"
+      raise newException(CatchableError, "CatchableError")
+
+  except:
+    echo "B"
+    #discard
+
+proc mainB =
+  try:
+    try:
+      raise newException(IOError, "IOError")
+
+    except:
+      echo "X"
+      raise newException(CatchableError, "CatchableError")
+    finally:
+      echo "inner finally"
+
+  except:
+    echo "Y"
+    #discard
+  finally:
+    echo "outer finally"
+
+main()
+mainB()
+
+when true:
+  #bug 7204
+  proc nested_finally =
+    try:
+      raise newException(KeyError, "msg1")
+    except KeyError as ex:
+      echo ex.msg
+      try:
+        # pop exception
+        raise newException(ValueError, "msg2") # push: exception stack (1 entry)
+      except:
+        echo getCurrentExceptionMsg()
+        # pop exception (except)
+      finally:
+        echo "finally2"
+      # pop exception (except KeyError as ex)
+    finally:
+      echo "finally1"
+
+  nested_finally()