From 19b0b9f8b6b9cf8f854f53f53f0013de26604fe4 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Thu, 21 Mar 2024 19:13:44 +0100
Subject: [PATCH] fix(sem): crash with forward-declared `=destroy` hook (#1252)

## Summary

Fix the compiler crashing when using a type with an, at that point,
forward-declared `=destroy` hook.

## Details

If the `=destroy` hook is forward-declared, its body is an `nkEmpty`
node, which caused a `FieldDefect` error in
`liftdestructors.createTypeBoundOps`, where the body AST is expected
to support `len`.

To resolve the issue, the `getBody(...).len` access is guarded by first
checking that the destroy hook is not overridden (indicated by the
`sfOverriden`  flag); only overridden hooks can be forward-declared.
This
works because `sfOverriden` implies that the attached-to type has
the `tfHasAsgn` flag already included.
---
 compiler/sem/liftdestructors.nim              |  4 ++-
 .../tforward_declared_destructor.nim          | 27 +++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 tests/lang_objects/destructor/tforward_declared_destructor.nim

diff --git a/compiler/sem/liftdestructors.nim b/compiler/sem/liftdestructors.nim
index 8c66b425416..5712b14c2b9 100644
--- a/compiler/sem/liftdestructors.nim
+++ b/compiler/sem/liftdestructors.nim
@@ -1062,7 +1062,9 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf
       setAttachedOp(g, idgen.module, orig, k, getAttachedOp(g, canon, k))
 
   let op = getAttachedOp(g, orig, attachedDestructor)
-  if op != nil and getBody(g, op).len != 0:
+  # if the destructor is overridden, the ``tfHasAsgn`` flag was already
+  # included
+  if op != nil and sfOverriden notin op.flags and getBody(g, op).len != 0:
     #or not isTrival(orig.assignment) or
     # not isTrival(orig.sink):
     orig.flags.incl tfHasAsgn
diff --git a/tests/lang_objects/destructor/tforward_declared_destructor.nim b/tests/lang_objects/destructor/tforward_declared_destructor.nim
new file mode 100644
index 00000000000..73ccce83a9e
--- /dev/null
+++ b/tests/lang_objects/destructor/tforward_declared_destructor.nim
@@ -0,0 +1,27 @@
+discard """
+  description: '''
+    Regression test for a forward-declared destroy hook causing a compiler
+    crash when hooks are requested before the forward-declaration is
+    completed
+  '''
+"""
+
+# note: the comments refer to implementation details that were present at the
+# time of writing
+
+type
+  Object* = object
+
+# it's important that the destroy hook is forward-declared here
+proc `=destroy`(x: var Object)
+
+# use `Object` in a way so that no hooks are requested before `p` is
+# instantiated
+proc test(x: Object) =
+  # request hooks for the type with the still-forward-declared destroy hook,
+  # resulting in a compiler crash
+  var x = x
+
+# finish the forward declaration
+proc `=destroy`(x: var Object) =
+  discard