From f98e81760161ef3fe303999468fdfe97e4914aee Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Sat, 16 Mar 2024 17:38:54 +0100
Subject: [PATCH 1/3] forbid hook routines raising exceptions (#1236)

## Summary

This is enforced as follows:
* if a hook routine potentially raises an exception, a compile-time
  error is reported
* if a hook routine raises a defect at run-time, the program panics

Exception effects of hooks side-steps effect tracking for
procedures, and if a hook does raise at run-time, behaviour was
previously undefined. Disallowing hooks to raise exceptions resolves
both issues.

Performance of the produced executables also improves significantly
(depending on the code), as destroy hooks not being able to raise
results in better code generation.

## Details

The implementation is made up of three parts:
* statically enforcing that no exceptions are raised by hooks (in
  `sempass2`)
* preventing exceptions from exiting hooks at run-time (in `mirgen`/
  `liftdestructors`)
* support in the runtime for panicking on unhandled exceptions

### Static Detection

* for hooks, identified by the presence of the `sfOverriden` flag,
  `sempass2` tests against an empty `.raises` specification, ensuring
  that no (tracked) exceptions can be raised
* the existing exception specification is always replaced
* no error is reported when `.raises: []` was explicitly specified, to
  give precedence to the `can raise unlisted exception` error.
* the symbol of hooks is marked with `sfNeverRaises`, to enforce at
  run-time that no exceptions (defects) leave the routine

The meaning of `sfNeverRaises` is changed from being a hint/guarantee
to being a request.

### Enforcement

* `mirgen` wraps the body of `sfNeverRaises` procedure in a
  `try: ... except: nimUnhandledException()`
* elimination of unreachable code in `cgirgen` removes the `except` if
  it's not used in practice
* all synthesized hook procedure (`liftdestructors`) are flagged with
  `sfNeverRaises`; the `canRaise` tracking is removed

### Runtimes

* C runtime: `nimUnhandledException` displays the exception and quits
  the process
* node.js JS runtime: `nimUnhandledException` displays the exception
  and quits the process
* non-node.js JS runtime: `nimUnhandledException` re-raises the
  exception (there's currently no way to terminate the program)
* VM: `nimUnhandledException` is a  `vmop` that raises a
  `vmEvtUnhandledException` event, which aborts execution

### Standard Library and Tests

* fix the `Task` `=destroy` hook being inferred to raise exceptions
* fix `GC_fullCollect` being inferred to raise exceptions

### Tests

Three individual tests need to be adjusted to the language change:
* `tnew` and `gctest` use `debugEcho` instead of `write` (the former
  has no effects)
* `tarcmisc` has to cast away the raise effects of `Stream.close` for
  now

### Specification

The beginnings of a specification test category for hook routines is
added. At the moment, it only covers the exception behaviour.
---
 compiler/ast/report_enums.nim                 |  1 +
 compiler/front/cli_reporter.nim               |  3 ++
 compiler/mir/mirgen.nim                       | 19 +++++++++++
 compiler/sem/injectdestructors.nim            | 34 +------------------
 compiler/sem/liftdestructors.nim              | 11 ++----
 compiler/sem/sempass2.nim                     | 15 ++++++++
 compiler/vm/compilerbridge.nim                |  4 ++-
 compiler/vm/vm.nim                            | 18 ++--------
 compiler/vm/vmdeps.nim                        | 10 ++++++
 compiler/vm/vmops.nim                         | 12 +++++++
 lib/std/tasks.nim                             |  2 +-
 lib/system.nim                                |  3 ++
 lib/system/excpt.nim                          |  6 ++++
 lib/system/jssys.nim                          | 32 +++++++++++++++--
 lib/system/orc.nim                            |  4 +--
 tests/arc/tarcmisc.nim                        |  3 +-
 tests/arc/topt_no_cursor.nim                  | 14 ++++----
 tests/errmsgs/tprefer_raise_spec_error.nim    | 16 +++++++++
 tests/gc/gctest.nim                           |  3 +-
 tests/lang/s02_core/s99_hooks/README.md       | 21 ++++++++++++
 .../s99_hooks/s99_escaping_defects/README.md  |  3 ++
 .../s99_escaping_defects/t01_copy_hook.nim    | 17 ++++++++++
 .../s99_escaping_defects/t02_sink_hook.nim    | 17 ++++++++++
 .../s99_escaping_defects/t03_destroy_hook.nim | 17 ++++++++++
 .../s99_escaping_defects/t04_trace_hook.nim   | 17 ++++++++++
 .../t05_deepcopy_hook.nim                     | 16 +++++++++
 .../s02_core/s99_hooks/t99_cannot_raise.nim   | 30 ++++++++++++++++
 tests/misc/tnew.nim                           |  3 +-
 28 files changed, 274 insertions(+), 77 deletions(-)
 create mode 100644 tests/errmsgs/tprefer_raise_spec_error.nim
 create mode 100644 tests/lang/s02_core/s99_hooks/README.md
 create mode 100644 tests/lang/s02_core/s99_hooks/s99_escaping_defects/README.md
 create mode 100644 tests/lang/s02_core/s99_hooks/s99_escaping_defects/t01_copy_hook.nim
 create mode 100644 tests/lang/s02_core/s99_hooks/s99_escaping_defects/t02_sink_hook.nim
 create mode 100644 tests/lang/s02_core/s99_hooks/s99_escaping_defects/t03_destroy_hook.nim
 create mode 100644 tests/lang/s02_core/s99_hooks/s99_escaping_defects/t04_trace_hook.nim
 create mode 100644 tests/lang/s02_core/s99_hooks/s99_escaping_defects/t05_deepcopy_hook.nim
 create mode 100644 tests/lang/s02_core/s99_hooks/t99_cannot_raise.nim

diff --git a/compiler/ast/report_enums.nim b/compiler/ast/report_enums.nim
index d4da99e7f74b4..4b4afeeef41c9 100644
--- a/compiler/ast/report_enums.nim
+++ b/compiler/ast/report_enums.nim
@@ -423,6 +423,7 @@ type
     rsemCallingConventionMismatch
     rsemHasSideEffects
     rsemCantPassProcvar
+    rsemHookCannotRaise
     rsemUnlistedRaises
     rsemUnlistedEffects
     rsemOverrideSafetyMismatch
diff --git a/compiler/front/cli_reporter.nim b/compiler/front/cli_reporter.nim
index cee55b31a275b..de634c0cc3326 100644
--- a/compiler/front/cli_reporter.nim
+++ b/compiler/front/cli_reporter.nim
@@ -1303,6 +1303,9 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string =
     of rsemXCannotRaiseY:
       result = "'$1' cannot raise '$2'" % [r.ast.render, r.raisesList.render]
 
+    of rsemHookCannotRaise:
+      result = "a hook routine is not allowed to raise. ($1)" % r.typ.render
+
     of rsemUnlistedRaises, rsemWarnUnlistedRaises:
       result.add("$1 can raise an unlisted exception: " % r.ast.render,
                  r.typ.render)
diff --git a/compiler/mir/mirgen.nim b/compiler/mir/mirgen.nim
index 572b334956ec4..cbbfd5e186094 100644
--- a/compiler/mir/mirgen.nim
+++ b/compiler/mir/mirgen.nim
@@ -2058,6 +2058,10 @@ proc generateCode*(graph: ModuleGraph, env: var MirEnv, owner: PSym,
 
   c.scopeDepth = 1
   c.add MirNode(kind: mnkScope)
+  if sfNeverRaises in owner.flags:
+    c.add MirNode(kind: mnkTry, len: 1)
+    c.add MirNode(kind: mnkStmtList)
+
   if owner.kind in routineKinds:
     # add a 'def' for each ``sink`` parameter. This simplifies further
     # processing and analysis
@@ -2070,6 +2074,21 @@ proc generateCode*(graph: ModuleGraph, env: var MirEnv, owner: PSym,
           c.add MirNode(kind: mnkNone)
 
   gen(c, body)
+
+  if sfNeverRaises in owner.flags:
+    # if it's enforced that the procedure never raises, exceptions escaping
+    # the procedure terminate the program. This is achieved by wrapping the
+    # body in a catch-all exception handler
+    c.add endNode(mnkStmtList)
+    c.subTree MirNode(kind: mnkExcept, len: 1):
+      c.subTree mnkBranch:
+        c.subTree mnkVoid:
+          let p = c.graph.getCompilerProc("nimUnhandledException")
+          c.builder.buildCall c.env.procedures.add(p), p.typ,
+                              typeOrVoid(c, p.typ[0]):
+            discard
+    c.add endNode(mnkTry)
+
   c.add endNode(mnkScope)
 
   swap(c.env, env) # swap back
diff --git a/compiler/sem/injectdestructors.nim b/compiler/sem/injectdestructors.nim
index 8b5330280f1ae..8ad20cf2469d0 100644
--- a/compiler/sem/injectdestructors.nim
+++ b/compiler/sem/injectdestructors.nim
@@ -63,31 +63,6 @@
 ## subsequently turning the assignment into a move and thus making the
 ## assertion fail with an ``IndexDefect``.
 
-# XXX: there exists an effect-related problem with the lifetime-tracking hooks
-#      (i.e. ``=copy``, ``=sink``, ``=destroy``). The assignment rewriting and,
-#      to some degree, the destructor injection can be seen as a
-#      refinement/expansion/lowering and should thus not introduce (observable)
-#      side-effects (mutation of global state, exceptional control-flow, etc.) --
-#      it also violates the MIR specification. All three hooks are currently
-#      allowed to have side-effects, which violates the aforementioned rules.
-#      It also causes the concrete issue of cyclic dependencies: for example,
-#      the move analyser uses data-flow analysis (which requires a control-flow
-#      graph) in order to decide where to move and where to copy. If whether a
-#      copy or move is used affects the control-flow graph, the move analyser
-#      depends on its own output, which while possible to make work, would
-#      likely introduce a large amount of complexity.
-#      There are two possible solutions:
-#      1. disallow lifetime-tracking hooks from having any side-effects
-#      2. at least for the ``=copy`` and ``=sink`` hooks, each assignment
-#         could be said to have the union of the effects from both hooks.
-#         Those can be computed when generating the MIR code, as types and
-#         their type-bound operations are already figured out at that point.
-#         It's more complicated for ``=destroy`` hooks, since they are
-#         injected rather than being the result of an expansion. The current
-#         plan is to introduce the MIR concept of dedicated "scope finalizers",
-#         which could be used to attach the effects gathered from all possible
-#         destructor calls to
-
 # XXX: not being able to rewrite an assignment into a call to the copy hook
 #      because it is disabled is a semantic error, meaning that it should
 #      be detected and reported during semantic analysis, not as part of
@@ -527,14 +502,7 @@ template buildVoidCall*(bu: var MirBuilder, env: var MirEnv, p: PSym,
                        body: untyped) =
   let prc = p # prevent multi evaluation
   bu.subTree mnkVoid:
-    let kind =
-      if canRaise(optPanics in graph.config.globalOptions, prc.ast[namePos]):
-        mnkCheckedCall
-      else:
-        mnkCall
-
-    # XXX: injected procedures should not introduce new control-flow paths
-    bu.subTree MirNode(kind: kind, typ: getVoidType(graph)):
+    bu.subTree MirNode(kind: mnkCall, typ: getVoidType(graph)):
       bu.use toValue(env.procedures.add(prc), prc.typ)
       body
 
diff --git a/compiler/sem/liftdestructors.nim b/compiler/sem/liftdestructors.nim
index 7493f01316d9c..8c66b42541622 100644
--- a/compiler/sem/liftdestructors.nim
+++ b/compiler/sem/liftdestructors.nim
@@ -57,7 +57,6 @@ type
     asgnForType: PType
     recurse: bool
     addMemReset: bool    # add wasMoved() call after destructor call
-    canRaise: bool
     filterDiscriminator: PSym  # we generating destructor for case branch
     c: PContext # c can be nil, then we are called from lambdalifting!
     idgen: IdGenerator
@@ -152,8 +151,6 @@ proc genContainerOf(c: var TLiftCtx; objType: PType, field, x: PSym): PNode =
 proc destructorCall(c: var TLiftCtx; op: PSym; x: PNode): PNode =
   var destroy = newTreeIT(nkCall, x.info, op.typ[0]):
     [newSymNode(op), genAddr(c, x)]
-  if sfNeverRaises notin op.flags:
-    c.canRaise = true
   if c.addMemReset:
     result = newTree(nkStmtList):
       [destroy, genBuiltin(c, mWasMoved,  "wasMoved", x)]
@@ -303,8 +300,6 @@ proc newHookCall(c: var TLiftCtx; op: PSym; x, y: PNode): PNode =
   #  localReport(c.config, x.info, "usage of '$1' is a user-defined error" % op.name.s)
   result = newNodeI(nkCall, x.info)
   result.add newSymNode(op)
-  if sfNeverRaises notin op.flags:
-    c.canRaise = true
   if op.typ.sons[1].kind == tyVar:
     result.add genAddr(c, x)
   else:
@@ -322,8 +317,6 @@ proc newHookCall(c: var TLiftCtx; op: PSym; x, y: PNode): PNode =
 proc newOpCall(c: var TLiftCtx; op: PSym; x: PNode): PNode =
   result = newTreeIT(nkCall, x.info, op.typ[0]):
     [newSymNode(op), x]
-  if sfNeverRaises notin op.flags:
-    c.canRaise = true
 
 proc newDeepCopyCall(c: var TLiftCtx; op: PSym; x, y: PNode): PNode =
   result = newAsgnStmt(x, newOpCall(c, op, y))
@@ -948,7 +941,7 @@ proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp;
         # bug #19205: Do not forget to also copy the hidden type field:
         genTypeFieldCopy(a, typ, result.ast[bodyPos], d, src)
 
-  if not a.canRaise: incl result.flags, sfNeverRaises
+  incl result.flags, sfNeverRaises
   completePartialOp(g, idgen.module, typ, kind, result)
 
 
@@ -972,7 +965,7 @@ proc produceDestructorForDiscriminator*(g: ModuleGraph; typ: PType; field: PSym,
   result.ast[bodyPos].add v
   let placeHolder = newNodeIT(nkSym, info, getSysType(g, info, tyPointer))
   fillBody(a, typ, result.ast[bodyPos], d, placeHolder)
-  if not a.canRaise: incl result.flags, sfNeverRaises
+  incl result.flags, sfNeverRaises
 
 
 template liftTypeBoundOps*(c: PContext; typ: PType; info: TLineInfo) =
diff --git a/compiler/sem/sempass2.nim b/compiler/sem/sempass2.nim
index 74b6e2e2c8984..d60c21463459b 100644
--- a/compiler/sem/sempass2.nim
+++ b/compiler/sem/sempass2.nim
@@ -1788,6 +1788,21 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) =
   else:
     effects[tagEffects] = t.tags
 
+  # ensure that user-provided hooks have no effects and don't raise
+  if sfOverriden in s.flags:
+    # if raising was explicitly disabled (i.e., via ``.raises: []``),
+    # exceptions, if any, were already reported; don't report errors again in
+    # that case
+    if raisesSpec.isNil or raisesSpec.len > 0:
+      let newSpec = newNodeI(nkArgList, s.info)
+      checkRaisesSpec(g, rsemHookCannotRaise, newSpec,
+                      t.exc, hints=off, nil)
+      # override the raises specification to prevent cascading errors:
+      effects[exceptionEffects] = newSpec
+
+    # enforce that no defects escape the routine at run-time:
+    s.flags.incl sfNeverRaises
+
   var mutationInfo = MutationInfo()
   var hasMutationSideEffect = false
   if {strictFuncs, views} * c.features != {}:
diff --git a/compiler/vm/compilerbridge.nim b/compiler/vm/compilerbridge.nim
index 6e14773492430..27ee3988b7a8f 100644
--- a/compiler/vm/compilerbridge.nim
+++ b/compiler/vm/compilerbridge.nim
@@ -225,7 +225,9 @@ proc buildError(c: TCtx, thread: VmThread, event: sink VmEvent): ExecErrorReport
   ## Creates an `ExecErrorReport` with the `event` and a stack-trace for
   ## `thread`
   let stackTrace =
-    if event.kind == vmEvtUnhandledException:
+    if event.kind == vmEvtUnhandledException and event.trace.len > 0:
+      # HACK: an unhandled exception can be reported without providing a trace.
+      #       Ideally, that shouldn't happen
       createStackTrace(c, event.trace)
     else:
       createStackTrace(c, thread)
diff --git a/compiler/vm/vm.nim b/compiler/vm/vm.nim
index 67301e53a1ca3..cd22818da907c 100644
--- a/compiler/vm/vm.nim
+++ b/compiler/vm/vm.nim
@@ -286,22 +286,8 @@ template toException(x: DerefFailureCode): untyped =
 
 proc reportException(c: TCtx; trace: sink VmRawStackTrace, raised: LocHandle) =
   ## Reports the exception represented by `raised` by raising a `VmError`
-
-  let name = $raised.getFieldHandle(1.fpos).deref().strVal
-  let msg = $raised.getFieldHandle(2.fpos).deref().strVal
-
-  # The reporter expects the exception as a deserialized PNode-tree. Only the
-  # 2nd (name) and 3rd (msg) field are actually used, so instead of running
-  # full deserialization (which is also not possible due to no `PType` being
-  # available), we just create the necessary parts manually
-
-  # TODO: the report should take the two strings directly instead
-  let empty = newNode(nkEmpty)
-  let ast = newTree(nkObjConstr,
-                    empty, # constructor type; unused
-                    empty, # unused
-                    newStrNode(nkStrLit, name),
-                    newStrNode(nkStrLit, msg))
+  let ast = toExceptionAst($raised.getFieldHandle(1.fpos).deref().strVal,
+                           $raised.getFieldHandle(2.fpos).deref().strVal)
   raiseVmError(VmEvent(kind: vmEvtUnhandledException, exc: ast, trace: trace))
 
 func cleanUpReg(r: var TFullReg, mm: var VmMemoryManager) =
diff --git a/compiler/vm/vmdeps.nim b/compiler/vm/vmdeps.nim
index 59918cdb2af2f..95318902f0de3 100644
--- a/compiler/vm/vmdeps.nim
+++ b/compiler/vm/vmdeps.nim
@@ -428,3 +428,13 @@ proc errorReportToString*(c: ConfigRef, error: Report): string =
               # the report, so need to add `"Error: "`
               # manally to stay consistent with the old
               # output.
+
+proc toExceptionAst*(name, msg: sink string): PNode =
+  ## Creates the AST as for an exception object as expected by the report.
+  # TODO: the report should take the two strings directly instead
+  let empty = newNode(nkEmpty)
+  newTree(nkObjConstr,
+          empty, # constructor type; unused
+          empty, # unused
+          newStrNode(nkStrLit, name),
+          newStrNode(nkStrLit, msg))
diff --git a/compiler/vm/vmops.nim b/compiler/vm/vmops.nim
index 70201fd21aec1..3d7db17358704 100644
--- a/compiler/vm/vmops.nim
+++ b/compiler/vm/vmops.nim
@@ -170,6 +170,17 @@ proc prepareExceptionWrapper(a: VmArgs) {.nimcall.} =
                  deref(a.getHandle(1)).strVal,
                  a.mem.allocator)
 
+proc nimUnhandledExceptionWrapper(a: VmArgs) {.nimcall.} =
+  # setup the exception AST:
+  let
+    exc = a.heap[].tryDeref(a.currentException, noneType).value()
+    ast = toExceptionAst($exc.getFieldHandle(1.fpos).deref().strVal,
+                         $exc.getFieldHandle(2.fpos).deref().strVal)
+  # report the unhandled exception:
+  # XXX: the current stack-trace should be passed along, but we don't
+  #      have access to it here
+  raiseVmError(VmEvent(kind: vmEvtUnhandledException, exc: ast))
+
 proc prepareMutationWrapper(a: VmArgs) {.nimcall.} =
   discard "no-op"
 
@@ -247,6 +258,7 @@ iterator basicOps*(): Override =
   systemop(getCurrentExceptionMsg)
   systemop(getCurrentException)
   systemop(prepareException)
+  systemop(nimUnhandledException)
   systemop(prepareMutation)
   override("stdlib.system.closureIterSetupExc",
            setCurrentExceptionWrapper)
diff --git a/lib/std/tasks.nim b/lib/std/tasks.nim
index e2ea5377f4388..ac18862228ee2 100644
--- a/lib/std/tasks.nim
+++ b/lib/std/tasks.nim
@@ -61,7 +61,7 @@ type
   Task* = object ## `Task` contains the callback and its arguments.
     callback: proc (args: pointer) {.nimcall, gcsafe.}
     args: pointer
-    destroy: proc (args: pointer) {.nimcall, gcsafe.}
+    destroy: proc (args: pointer) {.nimcall, gcsafe, raises: [].}
 
 
 proc `=copy`*(x: var Task, y: Task) {.error.}
diff --git a/lib/system.nim b/lib/system.nim
index 550b60ac13c17..72927a6b123c8 100644
--- a/lib/system.nim
+++ b/lib/system.nim
@@ -2348,6 +2348,9 @@ elif isNimVmTarget:
   proc prepareException(e: ref Exception, ename: cstring) {.compilerproc.} =
     discard
 
+  proc nimUnhandledException() {.compilerproc.} =
+    discard
+
   proc closureIterSetupExc(e: ref Exception) {.compilerproc, inline.} =
     ## Used by the closure transformation pass for preparing for exception
     ## handling. Implemented as a callback.
diff --git a/lib/system/excpt.nim b/lib/system/excpt.nim
index 8603fce28b78d..961d02da475ce 100644
--- a/lib/system/excpt.nim
+++ b/lib/system/excpt.nim
@@ -427,6 +427,12 @@ when true:
       currException = nil
       quit(1)
 
+proc nimUnhandledException() {.compilerproc, noreturn.} =
+  ## Called from generated code when propgation of an exception crosses a
+  ## routine boundary it shouldn't.
+  reportUnhandledError(currException)
+  quit(1)
+
 proc pushActiveException(e: sink(ref Exception)) =
   e.up = activeException
   activeException = e
diff --git a/lib/system/jssys.nim b/lib/system/jssys.nim
index 86b749c47ada0..363d20a976af7 100644
--- a/lib/system/jssys.nim
+++ b/lib/system/jssys.nim
@@ -115,8 +115,7 @@ proc writeStackTrace() =
 proc getStackTrace*(): string = rawWriteStackTrace()
 proc getStackTrace*(e: ref Exception): string = e.trace
 
-proc unhandledException(e: ref Exception) {.
-    compilerproc, asmNoStackFrame.} =
+proc unhandledExceptionString(e: ref Exception): string =
   var buf = ""
   if e.msg.len != 0:
     add(buf, "Error: unhandled exception: ")
@@ -128,7 +127,11 @@ proc unhandledException(e: ref Exception) {.
   add(buf, "]\n")
   when NimStackTrace:
     add(buf, rawWriteStackTrace())
-  let cbuf = cstring(buf)
+  result = buf
+
+proc unhandledException(e: ref Exception) {.
+    compilerproc, asmNoStackFrame.} =
+  let cbuf = cstring(unhandledExceptionString(e))
   framePtr = nil
   {.emit: """
   if (typeof(Error) !== "undefined") {
@@ -139,6 +142,29 @@ proc unhandledException(e: ref Exception) {.
   }
   """.}
 
+proc nimUnhandledException() {.compilerproc, asmNoStackFrame.} =
+  # |NimSkull| exceptions are turned into JavaScript errors for the purpose
+  # of better error messages
+  when defined(nodejs):
+    {.emit: """
+      if (lastJSError.m_type !== undefined) {
+        console.log(`toJSStr`(`unhandledExceptionString`(`lastJSError`)));
+      } else {
+        console.log('Error: unhandled exception: ', `lastJSError`)
+      }
+      process.exit(1);
+    """.}
+  else:
+    # it's currently not possible to truly panic (abort excution) for non-
+    # node.js JavaScript
+    {.emit: """
+      if (lastJSError.m_type !== undefined) {
+        `unhandledException`(lastJSError);
+      } else {
+        throw lastJSError;
+      }
+    """.}
+
 proc prepareException(e: ref Exception, ename: cstring) {.
     compilerproc, asmNoStackFrame.} =
   if e.name.isNil:
diff --git a/lib/system/orc.nim b/lib/system/orc.nim
index 38da71d9cdaba..ee44add02a0d2 100644
--- a/lib/system/orc.nim
+++ b/lib/system/orc.nim
@@ -27,7 +27,7 @@ const
   logOrc = defined(nimArcIds)
 
 type
-  TraceProc = proc (p, env: pointer) {.nimcall, benign.}
+  TraceProc = proc (p, env: pointer) {.nimcall, benign, raises: [], tags: [].}
   DisposeProc = proc (p: pointer) {.nimcall, benign.}
 
 template color(c): untyped = c.rc and colorMask
@@ -472,7 +472,7 @@ proc GC_prepareOrc*(): int {.inline.} = roots.len
 proc GC_partialCollect*(limit: int) =
   partialCollect(limit)
 
-proc GC_fullCollect* =
+proc GC_fullCollect*() {.raises: [].} =
   ## Forces a full garbage collection pass. With `--gc:orc` triggers the cycle
   ## collector. This is an alias for `GC_runOrc`.
   collectCycles()
diff --git a/tests/arc/tarcmisc.nim b/tests/arc/tarcmisc.nim
index 0e1dda7fb33b9..3747f6240970d 100644
--- a/tests/arc/tarcmisc.nim
+++ b/tests/arc/tarcmisc.nim
@@ -112,7 +112,8 @@ type
     x: int
 
 proc `=destroy`(x: var AObj) =
-  close(x.io)
+  {.cast(raises: []).}:
+    close(x.io)
   echo "closed"
 
 var x = B(io: newStringStream("thestream"))
diff --git a/tests/arc/topt_no_cursor.nim b/tests/arc/topt_no_cursor.nim
index 398a0fe593fb8..491fc9aefd3c7 100644
--- a/tests/arc/topt_no_cursor.nim
+++ b/tests/arc/topt_no_cursor.nim
@@ -33,17 +33,17 @@ scope:
   def_cursor _0: Node = target[]
   def_cursor _1: Node = _0[].parent
   def sibling: Node
-  =copy(name sibling, arg _1[].left) (raises)
+  =copy(name sibling, arg _1[].left)
   def_cursor _2: Node = sibling
   def saved: Node
-  =copy(name saved, arg _2[].right) (raises)
+  =copy(name saved, arg _2[].right)
   def_cursor _3: Node = sibling
   def_cursor _4: Node = saved
   def_cursor _6: Node = _4[].left
-  =copy(name _3[].right, arg _6) (raises)
+  =copy(name _3[].right, arg _6)
   def_cursor _5: Node = sibling
-  =sink(name _5[].parent, arg saved) (raises)
-  =destroy(name sibling) (raises)
+  =sink(name _5[].parent, arg saved)
+  =destroy(name sibling)
 -- end of expandArc ------------------------
 --expandArc: p1
 
@@ -130,7 +130,7 @@ scope:
 scope:
   try:
     def shadowScope: Scope
-    =copy(name shadowScope, arg c[].currentScope) (raises)
+    =copy(name shadowScope, arg c[].currentScope)
     rawCloseScope(arg c) (raises)
     scope:
       def_cursor _0: Scope = shadowScope
@@ -157,7 +157,7 @@ scope:
                   addInterfaceDecl(arg c, consume _6) (raises)
                 i = addI(arg i, arg 1) (raises)
   finally:
-    =destroy(name shadowScope) (raises)
+    =destroy(name shadowScope)
 -- end of expandArc ------------------------
 --expandArc: treturn
 
diff --git a/tests/errmsgs/tprefer_raise_spec_error.nim b/tests/errmsgs/tprefer_raise_spec_error.nim
new file mode 100644
index 0000000000000..4d92f4de3f03e
--- /dev/null
+++ b/tests/errmsgs/tprefer_raise_spec_error.nim
@@ -0,0 +1,16 @@
+discard """
+  description: '''
+    An error for violating the explicit `.raises` specification is preferred
+    over the error that hooks cannot raise
+  '''
+  errormsg: "doRaise() can raise an unlisted exception: ref CatchableError"
+  line: 16
+"""
+
+type Obj = object
+
+proc doRaise() =
+  raise CatchableError.newException("")
+
+proc `=copy`(a: var Obj, b: Obj) {.raises: [].} =
+  doRaise()
diff --git a/tests/gc/gctest.nim b/tests/gc/gctest.nim
index f241bfaf2f052..36138ce1f1d35 100644
--- a/tests/gc/gctest.nim
+++ b/tests/gc/gctest.nim
@@ -60,8 +60,7 @@ proc caseTree(lvl: int = 0): PCaseNode =
 
 proc `=destroy`(n: var TNode) =
   assert(addr(n) != nil)
-  write(stdout, "finalizing: ")
-  writeLine(stdout, "not nil")
+  debugEcho "finalizing: not nil"
 
 var
   id: int = 1
diff --git a/tests/lang/s02_core/s99_hooks/README.md b/tests/lang/s02_core/s99_hooks/README.md
new file mode 100644
index 0000000000000..786bff439dae6
--- /dev/null
+++ b/tests/lang/s02_core/s99_hooks/README.md
@@ -0,0 +1,21 @@
+## What belongs here
+
+This section contains tests related to hook procedures, that is, procedures:
+- to which calls are statically inserted by the compiler
+- that are invoked by the runtime at run-time
+
+This should cover:
+- syntax
+- restrictions on the routine definitions
+- restrictions on the run-time behaviour (if any)
+- where the hooks are injected
+
+## Assumptions
+
+- nothing beyond a single module/file
+- assertions may still be built-ins
+- user-defined types are supported and work
+- procedures and calls thereof work
+- `var T` is supported as a parameter's type
+- raising and catching exceptions work
+- tag effect tracking works
\ No newline at end of file
diff --git a/tests/lang/s02_core/s99_hooks/s99_escaping_defects/README.md b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/README.md
new file mode 100644
index 0000000000000..1538b39ff42f5
--- /dev/null
+++ b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/README.md
@@ -0,0 +1,3 @@
+If a Defect is raised from a hook routine at run-time, the program immediately
+terminates (i.e., panics) and an unhandled exception is reported. This is the
+case for both automatic and explicit calls of the hooks.
\ No newline at end of file
diff --git a/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t01_copy_hook.nim b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t01_copy_hook.nim
new file mode 100644
index 0000000000000..af708b1088644
--- /dev/null
+++ b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t01_copy_hook.nim
@@ -0,0 +1,17 @@
+discard """
+  description: "`=copy` hooks panic when a defect escapes"
+  outputsub: "Error: unhandled exception: error [Defect]"
+  exitcode: 1
+"""
+
+type Type = object
+
+proc `=copy`(a: var Type, b: Type) =
+  raise (ref Defect)(msg: "error")
+
+try:
+  var x: Type
+  `=copy`(x, Type())
+finally:
+  # finally sections are not reached and no cleanup is performed
+  doAssert false
diff --git a/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t02_sink_hook.nim b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t02_sink_hook.nim
new file mode 100644
index 0000000000000..e6f7af55f62f4
--- /dev/null
+++ b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t02_sink_hook.nim
@@ -0,0 +1,17 @@
+discard """
+  description: "`=sink` hooks panic when a defect escapes"
+  outputsub: "Error: unhandled exception: error [Defect]"
+  exitcode: 1
+"""
+
+type Type = object
+
+proc `=sink`(a: var Type, b: Type) =
+  raise (ref Defect)(msg: "error")
+
+try:
+  var x: Type
+  `=sink`(x, Type())
+finally:
+  # finally sections are not reached and no cleanup is performed
+  doAssert false
diff --git a/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t03_destroy_hook.nim b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t03_destroy_hook.nim
new file mode 100644
index 0000000000000..5433c981d39a7
--- /dev/null
+++ b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t03_destroy_hook.nim
@@ -0,0 +1,17 @@
+discard """
+  description: "`=destroy` hooks panic when a defect escapes"
+  outputsub: "Error: unhandled exception: error [Defect]"
+  exitcode: 1
+"""
+
+type Type = object
+
+proc `=destroy`(a: var Type) =
+  raise (ref Defect)(msg: "error")
+
+try:
+  var x: Type
+  `=destroy`(x)
+finally:
+  # finally sections are not reached and no cleanup is performed
+  doAssert false
diff --git a/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t04_trace_hook.nim b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t04_trace_hook.nim
new file mode 100644
index 0000000000000..65d62b289fc99
--- /dev/null
+++ b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t04_trace_hook.nim
@@ -0,0 +1,17 @@
+discard """
+  description: "`=trace` hooks panic when a defect escapes"
+  outputsub: "Error: unhandled exception: error [Defect]"
+  exitcode: 1
+"""
+
+type Type = object
+
+proc `=trace`(a: var Type, p: pointer) =
+  raise (ref Defect)(msg: "error")
+
+try:
+  var x: Type
+  `=trace`(x, nil)
+finally:
+  # finally sections are not reached and no cleanup is performed
+  doAssert false
diff --git a/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t05_deepcopy_hook.nim b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t05_deepcopy_hook.nim
new file mode 100644
index 0000000000000..a4196256bc992
--- /dev/null
+++ b/tests/lang/s02_core/s99_hooks/s99_escaping_defects/t05_deepcopy_hook.nim
@@ -0,0 +1,16 @@
+discard """
+  description: "`=deepcopy` hooks panic when a defect escapes"
+  outputsub: "Error: unhandled exception: error [Defect]"
+  exitcode: 1
+"""
+
+type Type = ref object
+
+proc `=deepcopy`(a: Type): Type =
+  raise (ref Defect)(msg: "error")
+
+try:
+  var x = `=deepcopy`(Type())
+finally:
+  # finally sections are not reached and no cleanup is performed
+  doAssert false
diff --git a/tests/lang/s02_core/s99_hooks/t99_cannot_raise.nim b/tests/lang/s02_core/s99_hooks/t99_cannot_raise.nim
new file mode 100644
index 0000000000000..0232aeed375b8
--- /dev/null
+++ b/tests/lang/s02_core/s99_hooks/t99_cannot_raise.nim
@@ -0,0 +1,30 @@
+discard """
+  description: '''
+    Hook routines are not allowed to raise exception. If they're inferred to
+    raise, a compile-time error is reported.
+  '''
+  action: reject
+  matrix: "--errorMax:5"
+"""
+
+type Type = object
+
+proc `=copy`(a: var Type, b: Type) =
+  raise (ref CatchableError)() #[tt.Error
+  ^ a hook routine is not allowed to raise. (ref CatchableError)]#
+
+proc `=sink`(a: var Type, b: Type) =
+  raise (ref CatchableError)() #[tt.Error
+  ^ a hook routine is not allowed to raise. (ref CatchableError)]#
+
+proc `=destroy`(a: var Type) =
+  raise (ref CatchableError)() #[tt.Error
+  ^ a hook routine is not allowed to raise. (ref CatchableError)]#
+
+proc `=trace`(a: var Type, env: pointer) =
+  raise (ref CatchableError)() #[tt.Error
+  ^ a hook routine is not allowed to raise. (ref CatchableError)]#
+
+proc `=deepCopy`(a: ref Type): ref Type =
+  raise (ref CatchableError)() #[tt.Error
+  ^ a hook routine is not allowed to raise. (ref CatchableError)]#
diff --git a/tests/misc/tnew.nim b/tests/misc/tnew.nim
index 318fa644be3f5..1c44bc7b457a8 100644
--- a/tests/misc/tnew.nim
+++ b/tests/misc/tnew.nim
@@ -20,8 +20,7 @@ type
   TStressTest = ref array[0..45, array[1..45, TNode]]
 
 proc `=destroy`(n: var TNode) =
-  write(stdout, n.data)
-  write(stdout, " is now freed\n")
+  debugEcho n.data, " is now freed"
 
 proc newNode(data: int, le, ri: PNode): PNode =
   new(result)

From 8a55ef7ca560444d80ca06ae14b2a9a986c1305b Mon Sep 17 00:00:00 2001
From: Clyybber <darkmine956@gmail.com>
Date: Sun, 17 Mar 2024 12:05:29 +0100
Subject: [PATCH 2/3] sem: `NimNode` equality respects `nkType`'s type (#1245)

## Summary
`exprStructuralEquivalentStrictSymAndComm`  no longer ignores type
equality for  `nkType`  nodes.
This affects  ``macros.`==` ``  for NimNodes and  `macrocache.incl`
(and  `ic` ).

## Details
A test using ``macros.`==` `` has been added.
---
 compiler/ast/trees.nim                        |  2 +-
 .../lang_callable/macros/tmacros_various.nim  | 40 +++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/compiler/ast/trees.nim b/compiler/ast/trees.nim
index 317ec8896e75b..24473ece70656 100644
--- a/compiler/ast/trees.nim
+++ b/compiler/ast/trees.nim
@@ -97,7 +97,7 @@ makeTreeEquivalenceProc(exprStructuralEquivalentStrictSymAndComm,
   relaxedKindCheck = false,
   symCheck     = a.sym == b.sym,
   floatCheck   = sameFloatIgnoreNan(a.floatVal, b.floatVal),
-  typeCheck    = true,
+  typeCheck    = a.typ == b.typ,
   commentCheck = a.comment == b.comment
 )
 export exprStructuralEquivalentStrictSymAndComm
diff --git a/tests/lang_callable/macros/tmacros_various.nim b/tests/lang_callable/macros/tmacros_various.nim
index 5d7d62dffb1bb..398028604531d 100644
--- a/tests/lang_callable/macros/tmacros_various.nim
+++ b/tests/lang_callable/macros/tmacros_various.nim
@@ -12,6 +12,8 @@ Infix
 macrocache ok
 CommentStmt "comment 1"
 CommentStmt "comment 2"
+false
+false
 '''
 
   output: '''
@@ -346,3 +348,41 @@ block:
   static:
     echo treeRepr(C1[1])
     echo treeRepr(C2[1])
+
+block:
+  # Ensure nkType equality is not ignored by `==` for NimNode
+  macro checkEq(a, b: typed) =
+    echo a == b
+
+  type Exception1 = object of Exception
+  type Exception2 = object of Exception
+  checkEq (;
+    try:
+      discard
+    except Exception1:
+      discard
+  ), (;
+    try:
+      discard
+    except Exception2:
+      discard
+  )
+
+  macro checkEqOfTry(a, b: typed) =
+    echo a[0][1][1] == b[0][1][1]
+
+  checkEqOfTry (;
+    block:
+      type E = object of Exception1
+      try:
+        discard
+      except E:
+        discard
+  ), (;
+    block:
+      type E = object of Exception2
+      try:
+        discard
+      except E:
+        discard
+  )

From 8d0db1d38140f25044b7d7d4fdad4b741f9edf4e Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Mon, 18 Mar 2024 00:48:08 +0100
Subject: [PATCH 3/3] make locals' lifetimes bounded by lexical scope for `if`
 (#1247)

## Summary

Temporaries and locals that start their storage duration within
`if`/`elif` condition expressions are now destroyed, if necessary,
at the end of the `if`/`elif` branch, not at the end of the `if`'s
enclosing scope. This makes the end of their storage duration
consistent with the end of their lexical scope.

## Details

Making the storage duration end at the end of the `if`/`elif` is
achieved by `mirgen` wrapping the code for each `nkElifBranch` in a
`scope`.

### `while` Lowering

Each `nkElifBranch` being wrapped in a (lifetime) scope, means that
lowering `while cond` into `while true: (if not cond: break; body)`
no longer works, since locals and temporaries starting their
storage duration during evaluation of the `cond` expression would
be destroyed at the end of the `if`, instead of at the end of the
`while`'s body.

To ensure correct and expected lifetimes, `cond` is first assigned
to a temporary, which the `if` then uses for branching. For
example, `while (let x = y; x > 1): body` is lowered by `transf`
into:
```nim
while true:
  let tmp = (let x = y; x > 1)
  if not tmp:
    break
  body
```

Turning `while cond` into `while true: (if cond: body else: break)`
would work too, but it's decide against, because:
* it, currently, results in worse cursor inference results (meaning
  more copies)
* it expands to more MIR code (more work for data-flow analysis)
---
 compiler/mir/mirgen.nim                   | 13 ++--
 compiler/sem/transf.nim                   | 16 +++-
 tests/arc/tcontrolflow.nim                |  7 +-
 tests/arc/topt_cursor.nim                 | 21 +++---
 tests/arc/topt_no_cursor.nim              | 90 ++++++++++++-----------
 tests/arc/topt_refcursors.nim             | 24 +++---
 tests/arc/topt_wasmoved_destroy_pairs.nim | 65 ++++++++--------
 7 files changed, 136 insertions(+), 100 deletions(-)

diff --git a/compiler/mir/mirgen.nim b/compiler/mir/mirgen.nim
index cbbfd5e186094..b2e0aa7b93064 100644
--- a/compiler/mir/mirgen.nim
+++ b/compiler/mir/mirgen.nim
@@ -1459,12 +1459,13 @@ proc genIf(c: var TCtx, n: PNode, dest: Destination) =
 
   template genElifBranch(branch: PNode, extra: untyped) =
     ## Generates the code for a single ``nkElif(Branch|Expr)``
-    let v = genUse(c, branch[0])
-    c.subTree mnkIf:
-      c.use v
-      c.scope:
-        genBranch(c, branch.lastSon, dest)
-        extra
+    c.scope:
+      let v = genUse(c, branch[0])
+      c.subTree mnkIf:
+        c.use v
+        c.scope:
+          genBranch(c, branch.lastSon, dest)
+          extra
 
   if n.len == 1:
     # an ``if`` statement/expression with a single branch. Don't emit the
diff --git a/compiler/sem/transf.nim b/compiler/sem/transf.nim
index 829171cb34c26..0728f544adb86 100644
--- a/compiler/sem/transf.nim
+++ b/compiler/sem/transf.nim
@@ -373,9 +373,9 @@ proc transformWhile(c: PTransf; n: PNode): PNode =
       loop[0] = newIntTypeNode(1, c.graph.getSysType(info, tyBool))
       loop[0].info = info
 
-      # XXX: we need to help ``closureiters`` (which doesn't support 'yield' in
-      #      if conditions...) here and unpack complex condition expressions;
-      #      'yield' in 'while' conditions would not work otherwise
+      # unwrap the statement list expression. It helps with the following
+      # lowering, and it's also necessary for the closure iterator
+      # transformation
       var preamble = PNode(nil)
       if cond.kind in {nkStmtListExpr, nkStmtList}:
         preamble = newNodeI(nkStmtList, info, cond.len - 1)
@@ -384,6 +384,16 @@ proc transformWhile(c: PTransf; n: PNode): PNode =
 
         cond = cond[^1]
 
+      # all definitions part of the condition expression are part of the while's
+      # scope, placing the expression into the if's condition slot would thus
+      # result in incorrect scoping
+      if not isAtom(cond):
+        let tmp = newTemp(c, cond.typ, cond.info)
+        if preamble.isNil:
+          preamble = newTree(nkStmtList)
+        preamble.add newTree(nkLetSection, newIdentDefs(tmp, cond))
+        cond = tmp
+
       let exit =
         newTreeI(nkIfStmt, info,
           newTreeI(nkElifBranch, info,
diff --git a/tests/arc/tcontrolflow.nim b/tests/arc/tcontrolflow.nim
index 80cc2b187eb8c..fd99f9141a9f7 100644
--- a/tests/arc/tcontrolflow.nim
+++ b/tests/arc/tcontrolflow.nim
@@ -1,12 +1,12 @@
 discard """
   output: '''begin A
 elif
-end A
 destroyed
+end A
 begin false
 if
-end false
 destroyed
+end false
 begin true
 if
 end true
@@ -19,6 +19,9 @@ true
 # we use the -d:danger switch to detect uninitialized stack
 # slots more reliably (there shouldn't be any, of course).
 
+# XXX: the test here need to be improved and turned into a proper
+#      specification
+
 type
   Foo = object
     id: int
diff --git a/tests/arc/topt_cursor.nim b/tests/arc/topt_cursor.nim
index 2085b3c17d927..4f4cb9f59ff63 100644
--- a/tests/arc/topt_cursor.nim
+++ b/tests/arc/topt_cursor.nim
@@ -7,10 +7,11 @@ scope:
   try:
     def_cursor x: (string, int) = <D0>
     block L0:
-      if cond:
-        scope:
-          x = <D1>
-          break L0
+      scope:
+        if cond:
+          scope:
+            x = <D1>
+            break L0
       scope:
         x = <D2>
     def_cursor _0: (string, int) = x
@@ -35,11 +36,13 @@ scope:
               while true:
                 scope:
                   def_cursor _1: File = f
-                  def _2: bool = readLine(arg _1, name res) (raises)
-                  def _3: bool = not(arg _2)
-                  if _3:
-                    scope:
-                      break L0
+                  def :tmp: bool = readLine(arg _1, name res) (raises)
+                  scope:
+                    def_cursor _2: bool = :tmp
+                    def _3: bool = not(arg _2)
+                    if _3:
+                      scope:
+                        break L0
                   scope:
                     scope:
                       def_cursor x: string = res
diff --git a/tests/arc/topt_no_cursor.nim b/tests/arc/topt_no_cursor.nim
index 491fc9aefd3c7..1dc6bcab5a425 100644
--- a/tests/arc/topt_no_cursor.nim
+++ b/tests/arc/topt_no_cursor.nim
@@ -98,11 +98,13 @@ scope:
           while true:
             scope:
               def_cursor _1: int = i
-              def _2: bool = ltI(arg _1, arg L)
-              def _3: bool = not(arg _2)
-              if _3:
-                scope:
-                  break L0
+              def :tmp: bool = ltI(arg _1, arg L)
+              scope:
+                def_cursor _2: bool = :tmp
+                def _3: bool = not(arg _2)
+                if _3:
+                  scope:
+                    break L0
               scope:
                 scope:
                   try:
@@ -110,12 +112,13 @@ scope:
                     def line: lent string = borrow a[_4]
                     def_cursor _5: string = line[]
                     def splitted: seq[string] = split(arg _5, arg " ", arg -1) (raises)
-                    def_cursor _6: string = splitted[0]
-                    def _7: bool = eqStr(arg _6, arg "opt")
-                    if _7:
-                      scope:
-                        def_cursor _10: string = splitted[1]
-                        =copy(name lan_ip, arg _10)
+                    scope:
+                      def_cursor _6: string = splitted[0]
+                      def _7: bool = eqStr(arg _6, arg "opt")
+                      if _7:
+                        scope:
+                          def_cursor _10: string = splitted[1]
+                          =copy(name lan_ip, arg _10)
                     def_cursor _8: string = lan_ip
                     echo(arg type(array[0..0, string]), arg _8) (raises)
                     def_cursor _9: string = splitted[1]
@@ -143,11 +146,13 @@ scope:
           while true:
             scope:
               def_cursor _2: int = i
-              def _3: bool = ltI(arg _2, arg L)
-              def _4: bool = not(arg _3)
-              if _4:
-                scope:
-                  break L0
+              def :tmp: bool = ltI(arg _2, arg L)
+              scope:
+                def_cursor _3: bool = :tmp
+                def _4: bool = not(arg _3)
+                if _4:
+                  scope:
+                    break L0
               scope:
                 scope:
                   def_cursor _5: int = i
@@ -164,14 +169,15 @@ scope:
 scope:
   try:
     def x: sink string
-    def_cursor _0: sink string = x
-    def _1: int = lengthStr(arg _0)
-    def _2: bool = eqI(arg _1, arg 2)
-    if _2:
-      scope:
-        result := move x
-        wasMoved(name x)
-        return
+    scope:
+      def_cursor _0: sink string = x
+      def _1: int = lengthStr(arg _0)
+      def _2: bool = eqI(arg _1, arg 2)
+      if _2:
+        scope:
+          result := move x
+          wasMoved(name x)
+          return
     def_cursor _3: sink string = x
     def _4: int = lengthStr(arg _3)
     def _5: string = $(arg _4) (raises)
@@ -189,14 +195,15 @@ scope:
     this[].isValid = fileExists(arg _0) (raises)
     def _1: tuple[dir: string, front: string]
     block L0:
-      def_cursor _2: string = this[].value
-      def _3: bool = dirExists(arg _2) (raises)
-      if _3:
-        scope:
-          def _4: string
-          =copy(name _4, arg this[].value)
-          _1 := construct (consume _4, consume "")
-          break L0
+      scope:
+        def_cursor _2: string = this[].value
+        def _3: bool = dirExists(arg _2) (raises)
+        if _3:
+          scope:
+            def _4: string
+            =copy(name _4, arg this[].value)
+            _1 := construct (consume _4, consume "")
+            break L0
       scope:
         try:
           def_cursor _5: string = this[].value
@@ -214,15 +221,16 @@ scope:
           =destroy(name _6)
     def par: tuple[dir: string, front: string] = move _1
     block L1:
-      def_cursor _10: string = par.0
-      def _11: bool = dirExists(arg _10) (raises)
-      if _11:
-        scope:
-          def_cursor _12: string = par.0
-          def_cursor _13: string = par.1
-          def _14: seq[string] = getSubDirs(arg _12, arg _13) (raises)
-          =sink(name this[].matchDirs, arg _14)
-          break L1
+      scope:
+        def_cursor _10: string = par.0
+        def _11: bool = dirExists(arg _10) (raises)
+        if _11:
+          scope:
+            def_cursor _12: string = par.0
+            def_cursor _13: string = par.1
+            def _14: seq[string] = getSubDirs(arg _12, arg _13) (raises)
+            =sink(name this[].matchDirs, arg _14)
+            break L1
       scope:
         def _15: seq[string] = construct ()
         =sink(name this[].matchDirs, arg _15)
diff --git a/tests/arc/topt_refcursors.nim b/tests/arc/topt_refcursors.nim
index 20b5823ee6680..e0d466b1f3ef5 100644
--- a/tests/arc/topt_refcursors.nim
+++ b/tests/arc/topt_refcursors.nim
@@ -11,11 +11,13 @@ scope:
         scope:
           def_cursor _0: Node = it
           def _1: bool = eqRef(arg _0, arg nil)
-          def _2: bool = not(arg _1)
-          def _3: bool = not(arg _2)
-          if _3:
-            scope:
-              break L0
+          def :tmp: bool = not(arg _1)
+          scope:
+            def_cursor _2: bool = :tmp
+            def _3: bool = not(arg _2)
+            if _3:
+              scope:
+                break L0
           scope:
             def_cursor _4: Node = it
             def_cursor _5: string = _4[].s
@@ -29,11 +31,13 @@ scope:
         scope:
           def_cursor _7: Node = jt
           def _8: bool = eqRef(arg _7, arg nil)
-          def _9: bool = not(arg _8)
-          def _10: bool = not(arg _9)
-          if _10:
-            scope:
-              break L1
+          def :tmp: bool = not(arg _8)
+          scope:
+            def_cursor _9: bool = :tmp
+            def _10: bool = not(arg _9)
+            if _10:
+              scope:
+                break L1
           scope:
             def_cursor _11: Node = jt
             def_cursor ri: Node = _11[].ri
diff --git a/tests/arc/topt_wasmoved_destroy_pairs.nim b/tests/arc/topt_wasmoved_destroy_pairs.nim
index 3e23ce3ee2555..2d21d939f5bbd 100644
--- a/tests/arc/topt_wasmoved_destroy_pairs.nim
+++ b/tests/arc/topt_wasmoved_destroy_pairs.nim
@@ -8,11 +8,12 @@ scope:
   def b: seq[seq[int]]
   def x: seq[int] = f() (raises)
   block L0:
-    if cond:
-      scope:
-        def _0: seq[int] = move x
-        add(name a, consume _0)
-        break L0
+    scope:
+      if cond:
+        scope:
+          def _0: seq[int] = move x
+          add(name a, consume _0)
+          break L0
     scope:
       def _1: seq[int] = move x
       add(name b, consume _1)
@@ -35,29 +36,33 @@ scope:
           while true:
             scope:
               def_cursor _0: int = i
-              def _1: bool = ltI(arg _0, arg b)
-              def _2: bool = not(arg _1)
-              if _2:
-                scope:
-                  break L0
+              def :tmp: bool = ltI(arg _0, arg b)
+              scope:
+                def_cursor _1: bool = :tmp
+                def _2: bool = not(arg _1)
+                if _2:
+                  scope:
+                    break L0
               scope:
                 scope:
                   def_cursor i: int = i
-                  def _3: bool = eqI(arg i, arg 2)
-                  if _3:
-                    scope:
-                      return
+                  scope:
+                    def _3: bool = eqI(arg i, arg 2)
+                    if _3:
+                      scope:
+                        return
                   def _4: seq[int]
                   =copy(name _4, arg x)
                   add(name a, consume _4)
                 i = addI(arg i, arg 1) (raises)
     block L1:
-      if cond:
-        scope:
-          def _5: seq[int] = move x
-          wasMoved(name x)
-          add(name a, consume _5)
-          break L1
+      scope:
+        if cond:
+          scope:
+            def _5: seq[int] = move x
+            wasMoved(name x)
+            add(name a, consume _5)
+            break L1
       scope:
         def _6: seq[int] = move x
         wasMoved(name x)
@@ -72,17 +77,19 @@ scope:
   try:
     def str: string
     def x: string = boolToStr(arg cond)
-    if cond:
-      scope:
-        return
+    scope:
+      if cond:
+        scope:
+          return
     def _0: string = boolToStr(arg cond)
     str := move _0
-    def _1: bool = not(arg cond)
-    if _1:
-      scope:
-        result := move str
-        wasMoved(name str)
-        return
+    scope:
+      def _1: bool = not(arg cond)
+      if _1:
+        scope:
+          result := move str
+          wasMoved(name str)
+          return
   finally:
     =destroy(name x)
     =destroy(name str)