fixes #21245; warn about destructors that can raise (#21726)

* fixes #21245; warn about destructors that can raise

* doc update

* progress

* typo

changelogs/changelog_2_0_0.md

@@ -179,7 +179,7 @@
   for 64-bit integer types (`int64` and `uint64`) by default. As this affects
   JS code generation, code using these types to interface with the JS backend
   may need to be updated. Note that `int` and `uint` are not affected.
-  
+
   For compatibility with [platforms that do not support BigInt](https://caniuse.com/bigint)
   and in the case of potential bugs with the new implementation, the
   old behavior is currently still supported with the command line option
@@ -195,7 +195,7 @@
 
   iterator iter(): int =
     yield 123
-  
+
   proc takesProc[T: proc](x: T) = discard
   proc takesIter[T: iterator](x: T) = discard
 
@@ -221,10 +221,15 @@
 
 - Signed integer literals in `set` literals now default to a range type of
   `0..255` instead of `0..65535` (the maximum size of sets).
-  
+
 - Case statements with else branches put before elif/of branches in macros
   are rejected with "invalid order of case branches".
 
+- Destructors now default to `.raises: []` (i.e. destructors must not raise
+  unlisted exceptions) and explicitly raising destructors are implementation
+  defined behavior.
+
+
 ## Standard library additions and changes
 
 [//]: # "Changes:"

compiler/sempass2.nim

@@ -1499,7 +1499,8 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) =
   let p = s.ast[pragmasPos]
   let raisesSpec = effectSpec(p, wRaises)
   if not isNil(raisesSpec):
-    checkRaisesSpec(g, false, raisesSpec, t.exc, "can raise an unlisted exception: ",
+    let useWarning = s.name.s == "=destroy"
+    checkRaisesSpec(g, useWarning, raisesSpec, t.exc, "can raise an unlisted exception: ",
                     hints=on, subtypeRelation, hintsArg=s.ast[0])
     # after the check, use the formal spec:
     effects[exceptionEffects] = raisesSpec

compiler/semstmts.nim

@@ -1857,6 +1857,11 @@ proc semOverride(c: PContext, s: PSym, n: PNode) =
   case name
   of "=destroy":
     bindTypeHook(c, s, n, attachedDestructor)
+    if s.ast != nil:
+      if s.ast[pragmasPos].kind == nkEmpty:
+        s.ast[pragmasPos] = newNodeI(nkPragma, s.info)
+      s.ast[pragmasPos].add newTree(nkExprColonExpr,
+          newIdentNode(c.cache.getIdent("raises"),  s.info), newNodeI(nkBracket, s.info))
   of "deepcopy", "=deepcopy":
     if s.typ.len == 2 and
         s.typ[1].skipTypes(abstractInst).kind in {tyRef, tyPtr} and

doc/destructors.md

@@ -13,12 +13,12 @@ Nim Destructors and Move Semantics
 About this document
 ===================
 
-This document describes the upcoming Nim runtime which does
+This document describes the ARC/ORC Nim runtime which does
 not use classical GC algorithms anymore but is based on destructors and
-move semantics. The new runtime's advantages are that Nim programs become
+move semantics. The advantages are that Nim programs become
 oblivious to the involved heap sizes and programs are easier to write to make
 effective use of multi-core machines. As a nice bonus, files and sockets and
-the like will not require manual `close` calls anymore.
+the like can be written not to require manual `close` calls anymore.
 
 This document aims to be a precise specification about how
 move semantics and destructors work in Nim.
@@ -132,6 +132,14 @@ The general pattern in `=destroy` looks like:
       freeResource(x.field)
   ```
 
+A `=destroy` is implicitly annotated with `.raises: []`; a destructor
+should not raise exceptions. For backwards compatibility the compiler
+produces a warning for a `=destroy` that does raise.
+
+A `=destroy` can explicitly list the exceptions it can raise, if any,
+but this of little utility as a raising destructor is implementation defined
+behavior. Later versions of the language specification might cover this case precisely.
+
 
 `=sink` hook
 ------------