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

6 files changed, 74 insertions, 8 deletions

diff --git a/changelog.md b/changelog.md
index f1ffb4a3a..9bf372dc4 100644
--- a/changelog.md
+++ b/changelog.md
@@ -46,11 +46,11 @@
   used must be castable to `ptr string`, any incompatible pointer type will not
   work. The `lexbase` and `streams` modules used to fail to compile on
   NimScript due to a bug, but this has been fixed.
-  
+
   The following modules now compile on both JS and NimScript: `parsecsv`,
   `parsecfg`, `parsesql`, `xmlparser`, `htmlparser` and `ropes`. Additionally
   supported for JS is `cstrutils.startsWith` and `cstrutils.endsWith`, for
-  NimScript: `json`, `parsejson`, `strtabs` and `unidecode`. 
+  NimScript: `json`, `parsejson`, `strtabs` and `unidecode`.
 
 - Added `streams.readStr` and `streams.peekStr` overloads to
   accept an existing string to modify, which avoids memory
@@ -68,7 +68,7 @@
 - `sugar.=>` and `sugar.->` changes: Previously `(x, y: int)` was transformed
   into `(x: auto, y: int)`, it now becomes `(x: int, y: int)` in consistency
   with regular proc definitions (although you cannot use semicolons).
-  
+
   Pragmas and using a name are now allowed on the lefthand side of `=>`. Here
   is an aggregate example of these changes:
   ```nim
@@ -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
 
diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim
index e1521475a..a7b01a75e 100644
--- a/compiler/sempass2.nim
+++ b/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
diff --git a/compiler/types.nim b/compiler/types.nim
index 241229df6..709e5bb2e 100644
--- a/compiler/types.nim
+++ b/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:
diff --git a/doc/manual.rst b/doc/manual.rst
index 02ccace2c..6feb8aebb 100644
--- a/doc/manual.rst
+++ b/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
 ------------
diff --git a/lib/system/excpt.nim b/lib/system/excpt.nim
index 747b145ae..bbb890cd4 100644
--- a/lib/system/excpt.nim
+++ b/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:
diff --git a/tests/effects/teffects7.nim b/tests/effects/teffects7.nim
index aee5e027d..73865b18d 100644
--- a/tests/effects/teffects7.nim
+++ b/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