only allow re-raise in scope of except

compiler/ast/report_enums.nim

@@ -498,6 +498,7 @@ type
     rsemExpectedObjectForMethod
     rsemUnexpectedPragmaInDefinitionOf
     rsemMisplacedRunnableExample
+    rsemCannotReraise
 
     # Expressions
     rsemConstantOfTypeHasNoValue

compiler/front/cli_reporter.nim

@@ -1350,6 +1350,10 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string =
     of rsemMisplacedRunnableExample:
       result = "runnableExamples must appear before the first non-comment statement"
 
+    of rsemCannotReraise:
+      result = "an exception can only be re-raised within the scope of an" &
+               " except, with no finally in-between"
+
     of rsemCannotInferTypeOfLiteral:
       result = "cannot infer the type of the $1" % r.typ.kind.toHumanStr
 

compiler/sem/sempass2.nim

@@ -121,6 +121,8 @@ type
     exc: PNode  ## stack of exceptions
     tags: PNode ## list of tags
     bottom, inTryStmt, inExceptOrFinallyStmt, leftPartOfAsgn: int
+    isReraiseAllowed: int
+      ## > 0 if a re-raise statement is allowed
     owner: PSym
     ownerModule: PSym
     init: seq[int] ## list of initialized variables
@@ -577,12 +579,19 @@ proc trackTryStmt(tracked: PEffects, n: PNode) =
     let b = n[i]
     if b.kind == nkExceptBranch:
       setLen(tracked.init, oldState)
+      inc tracked.isReraiseAllowed
       track(tracked, b[^1])
+      dec tracked.isReraiseAllowed
       for i in oldState..<tracked.init.len:
         addToIntersection(inter, tracked.init[i])
     else:
       setLen(tracked.init, oldState)
+      let prev = tracked.isReraiseAllowed
+      # re-raising in a finally clause would allow handling the exception,
+      # which is illegal. Therefore, re-raising is disallowed:
+      tracked.isReraiseAllowed = 0
       track(tracked, b[^1])
+      tracked.isReraiseAllowed = prev
       hasFinally = true
 
   tracked.bottom = oldBottom
@@ -1170,11 +1179,15 @@ proc track(tracked: PEffects, n: PNode) =
       for i in 0..<n.safeLen:
         track(tracked, n[i])
       createTypeBoundOps(tracked, n[0].typ, n.info)
-    else:
+    elif tracked.isReraiseAllowed > 0:
       # A `raise` with no arguments means we're going to re-raise the exception
-      # being handled or, if outside of an `except` block, a `ReraiseDefect`.
-      # Here we add a `Exception` tag in order to cover both the cases.
+      # being handled
+      # XXX: using an `Exception` tag is overly conservative. It is statically
+      #      known which exceptions an except branch covers, but this
+      #      information isn't available here, at the moment.
       addRaiseEffect(tracked, createRaise(tracked.graph, n), nil)
+    else:
+      localReport(tracked.config, n, reportSem(rsemCannotReraise))
   of nkCallKinds:
     trackCall(tracked, n)
   of nkDotExpr:

doc/manual.rst

@@ -4690,10 +4690,13 @@ the `raise` statement is the only way to raise an exception.
 
 .. XXX document this better!
 
-If no exception name is given, the current exception is `re-raised`:idx:. The
-`ReraiseDefect`:idx: exception is raised if there is no exception to
-re-raise. It follows that the `raise` statement *always* raises an
-exception.
+If no exception name is given, the current exception is `re-raised`:idx:.
+There are two restrictions for `raise` statements without an exception name:
+
+1. They must appear within the scope of an `except`.
+2. They must not appear within the direct scope of a `finally`. A re-raise
+   statement within an `except` that appears within a `finally` is okay, but
+   the other way around is not.
 
 
 Exception hierarchy

tests/exception/tdisallowed_reraise.nim

@@ -0,0 +1,62 @@
+discard """
+  description: '''
+    Ensure that re-raise statements are rejected where required by the
+    language specification.
+  '''
+  cmd: "$nim check --hints:off $options $file"
+  action: reject
+  nimoutFull: true
+  nimout: '''
+tdisallowed_reraise.nim(26, 5) Error: an exception can only be re-raised within the scope of an except, with no finally in-between
+tdisallowed_reraise.nim(20, 3) Error: an exception can only be re-raised within the scope of an except, with no finally in-between
+tdisallowed_reraise.nim(39, 5) Error: an exception can only be re-raised within the scope of an except, with no finally in-between
+tdisallowed_reraise.nim(50, 7) Error: an exception can only be re-raised within the scope of an except, with no finally in-between
+'''
+"""
+
+
+block standalone_reraise:
+  # a standalone re-raise statement is disallowed
+  raise
+
+block reraise_in_procedure:
+  proc reraise() =
+    # the re-raise statement doesn't appear within an ``except`` clause's
+    # scope
+    raise
+
+  try:
+    raise CatchableError.newException("")
+  except:
+    # where the procedure is called has no effect on the rules
+    reraise()
+
+block reraise_in_finally:
+  try:
+    raise CatchableError.newException("")
+  finally:
+    # a re-raise statement within a finally clause is disallowed
+    raise
+
+block reraise_in_nested_finally:
+  try:
+    raise CatchableError.newException("")
+  except:
+    try:
+      break
+    finally:
+      # a re-raise statement within a finally clause that's within an except
+      # clause is still disallowed
+      raise
+
+block reraise_in_nested_try:
+  # no error is reported here
+  try:
+    raise CatchableError.newException("")
+  except:
+    try:
+      # a re-raise within a try's scope is allowed, if the try is within the
+      # scope of an except
+      raise
+    finally:
+      discard

tests/exception/tfinally6.nim

@@ -49,24 +49,6 @@ proc raiseFromFinally() =
 
 test raiseFromFinally(), [1, 2, 3]
 
-proc reraiseFromFinally() =
-  try:
-    try:
-      raise ValueError.newException("a")
-    finally:
-      steps.add 1
-      # abort the exception but immediately re-raise it
-      raise
-    doAssert false, "unreachable"
-  except ValueError as e:
-    steps.add 2
-    doAssert e.msg == "a"
-    doAssert getCurrentException() == e
-
-  steps.add 3
-
-test reraiseFromFinally(), [1, 2, 3]
-
 proc exceptionInFinally() =
   ## Raise, and fully handle, an exception within a finally clause that was
   ## entered through exceptional control-flow.

tests/lang_callable/macros/t18203_consumed_ast_is_not_unused.nim

@@ -11,6 +11,6 @@ action: compile
 import std/macros
 
 macro foo(x: typed) = newProc ident"bar"
-proc bar() {.foo.} = raise
+proc bar() {.foo.} = raise CatchableError.newException("")
 bar()