do not track 'raise Defect' in the .raises: [] clause anymore (#14298)

* do not track 'raise Defect' in the .raises: [] clause anymore

* --panics:on maps 'raise Defect' to an unrecoverable fatal error

* make tests green again

* update the documentation too

changelog.md

@@ -90,7 +90,8 @@
   hangs if a process had both reads from stdin and writes (eg to stdout).
 
 ## Language changes
-- In newruntime it is now allowed to assign discriminator field without restrictions as long as case object doesn't have custom destructor. Discriminator value doesn't have to be a constant either. If you have custom destructor for case object and you do want to freely assign discriminator fields, it is recommended to refactor object into 2 objects like this:
+- In the newruntime it is now allowed to assign discriminator field without restrictions as long as case object doesn't have custom destructor. Discriminator value doesn't have to be a constant either. If you have custom destructor for case object and you do want to freely assign discriminator fields, it is recommended to refactor object into 2 objects like this:
+
   ```nim
   type
     MyObj = object
@@ -124,6 +125,30 @@
   with command line switch `--useVersion:1.0`.
 
 - The keyword `from` is now usable as an operator.
+- Exceptions inheriting from `system.Defect` are no longer tracked with
+  the `.raises: []` exception tracking mechanism. This is more consistent with the
+  built-in operations. The following always used to compile (and still does):
+
+```nim
+
+proc mydiv(a, b): int {.raises: [].} =
+  a div b # can raise an DivByZeroDefect
+
+```
+
+  Now also this compiles:
+
+```nim
+
+proc mydiv(a, b): int {.raises: [].} =
+  if b == 0: raise newException(DivByZeroDefect, "division by zero")
+  else: result = a div b
+
+```
+
+  The reason for this is that `DivByZeroDefect` inherits from `Defect` and
+  with `--panics:on` `Defects` become unrecoverable errors.
+
 
 ## Compiler changes
 

compiler/sempass2.nim

@@ -315,7 +315,10 @@ proc addEffect(a: PEffects, e, comesFrom: PNode) =
     # we only track the first node that can have the effect E in order
     # to safe space and time.
     if sameType(a.graph.excType(aa[i]), a.graph.excType(e)): return
-  throws(a.exc, e, comesFrom)
+
+  if e.typ != nil:
+    if optNimV1Emulation in a.config.globalOptions or not isDefectException(e.typ):
+      throws(a.exc, e, comesFrom)
 
 proc addTag(a: PEffects, e, comesFrom: PNode) =
   var aa = a.tags

compiler/types.nim

@@ -1672,6 +1672,17 @@ proc isException*(t: PType): bool =
     t = skipTypes(t[0], abstractPtrs)
   return false
 
+proc isDefectException*(t: PType): bool =
+  var t = t.skipTypes(abstractPtrs)
+  while t.kind == tyObject:
+    if t.sym != nil and t.sym.owner != nil and
+        sfSystemModule in t.sym.owner.flags and
+        t.sym.name.s == "Defect":
+      return true
+    if t[0] == nil: break
+    t = skipTypes(t[0], abstractPtrs)
+  return false
+
 proc isSinkTypeForParam*(t: PType): bool =
   # a parameter like 'seq[owned T]' must not be used only once, but its
   # elements must, so we detect this case here:

doc/manual.rst

@@ -4338,6 +4338,28 @@ Rules 1-2 ensure the following works:
 So in many cases a callback does not cause the compiler to be overly
 conservative in its effect analysis.
 
+Exceptions inheriting from ``system.Defect`` are not tracked with
+the ``.raises: []`` exception tracking mechanism. This is more consistent with the
+built-in operations. The following code is valid::
+
+.. code-block:: nim
+
+  proc mydiv(a, b): int {.raises: [].} =
+    a div b # can raise an DivByZeroDefect
+
+And so is::
+
+.. code-block:: nim
+
+  proc mydiv(a, b): int {.raises: [].} =
+    if b == 0: raise newException(DivByZeroDefect, "division by zero")
+    else: result = a div b
+
+
+The reason for this is that ``DivByZeroDefect`` inherits from ``Defect`` and
+with ``--panics:on`` Defects become unrecoverable errors.
+(Since version 1.4 of the language.)
+
 
 Tag tracking
 ------------

lib/system/excpt.nim

@@ -406,7 +406,7 @@ proc reportUnhandledError(e: ref Exception) {.nodestroy.} =
   when hostOS != "any":
     reportUnhandledErrorAux(e)
   else:
-    discard()
+    discard ()
 
 proc nimLeaveFinally() {.compilerRtl.} =
   when defined(cpp) and not defined(noCppExceptions) and not gotoBasedExceptions:
@@ -434,6 +434,11 @@ when gotoBasedExceptions:
       quit(1)
 
 proc raiseExceptionAux(e: sink(ref Exception)) {.nodestroy.} =
+  when defined(nimPanics):
+    if e of Defect:
+      reportUnhandledError(e)
+      quit(1)
+
   if localRaiseHook != nil:
     if not localRaiseHook(e): return
   if globalRaiseHook != nil:

tests/effects/teffects7.nim

@@ -1,5 +1,5 @@
 discard """
-  errormsg: "can raise an unlisted exception: ref FloatingPointDefect"
+  errormsg: "can raise an unlisted exception: ref ValueError"
   line: 10
 """
 
@@ -7,7 +7,7 @@ proc foo() {.raises: [].} =
   try:
     discard
   except KeyError:
-    raise newException(FloatingPointDefect, "foo")
+    raise newException(ValueError, "foo")
   except Exception:
     discard