From 9ab1823d01c302034f9ddf8a59a386988e7b47f3 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Thu, 5 Dec 2024 00:27:17 +0000
Subject: [PATCH] fix compiler crash with `.global` initializers

Summary
=======

Fix the compiler crashing for initializers of `.global` variables where
the expression contains locals or temporaries requiring destruction.

Details
=======

MIR generation for `.global` init fragments is done via
`mirgen.generateAssignment`, which didn't push a scope prior to
translation. This caused any call to `mirgen_blocks.register` to crash
the compiler, as the scope/block list was empty.

A scope is now pushed prior to translating the expression, fixing the
crash. In addition to fixing the crash, this slightly changes
behaviour: unscoped temporaries/locals created as part of a `.global`
initializer are now destroyed immediately after the initialization,
instead of at the end of the module's pre-init procedure. Destruction
order w.r.t. `.global` initializers is unspecified, so this change
shouldn't be a problem.
---
 compiler/mir/mirgen.nim                       | 10 ++++++----
 .../twith_local_requiring_destruction.nim     | 20 +++++++++++++++++++
 2 files changed, 26 insertions(+), 4 deletions(-)
 create mode 100644 tests/global/twith_local_requiring_destruction.nim

diff --git a/compiler/mir/mirgen.nim b/compiler/mir/mirgen.nim
index 0367f555648..47af1a47368 100644
--- a/compiler/mir/mirgen.nim
+++ b/compiler/mir/mirgen.nim
@@ -2388,16 +2388,18 @@ proc generateAssignment*(graph: ModuleGraph, env: var MirEnv,
   ## `builder`'s currently selected buffer.
   assert n.kind == nkIdentDefs and n.len == 3
   var c = initCtx(graph, config, nil, move env)
-  # treat the code as top-level code so that no 'def' is generated for
-  # assignments to globals
-  c.scopeDepth = 1
 
   template swapState() =
     swap(c.sp.map, source)
     swap(c.builder, builder)
 
   swapState()
-  genLocInit(c, n[0], n[2])
+  # treat the code as top-level code so that no 'def' is generated for
+  # assignments to globals
+  c.scope(true):
+    # a scope is required, otherwise locals/temporaries cannot be registered
+    # for destruction
+    genLocInit(c, n[0], n[2])
   swapState()
   env = move c.env # move back
 
diff --git a/tests/global/twith_local_requiring_destruction.nim b/tests/global/twith_local_requiring_destruction.nim
new file mode 100644
index 00000000000..710712f3b2c
--- /dev/null
+++ b/tests/global/twith_local_requiring_destruction.nim
@@ -0,0 +1,20 @@
+discard """
+  description: '''
+    Ensure that not explicitly scoped locals/temporaries in `.global`
+    intializer expressions are destroyed.
+  '''
+  targets: c js vm
+  output: "destroy: 1"
+"""
+
+type Object = object
+  val: int
+
+proc `=destroy`(x: var Object) =
+  echo "destroy: ", x.val
+
+proc test() =
+  # the local (`x`) has no explicit scope
+  var g {.global.} = (var x = Object(val: 1); x.val)
+
+test()