only generate called hook for explicit or generated destructor calls [backport] (#25729)

fixes #25727, regression from #24627 which was backported to 2.2.2 and
2.0.16

Instead of calling `createTypeBoundOps` for explicit hook calls and when
generating default hooks, only the called destructor is generated at a
time. This allows defining more than 1 hook for recursive types.

`=sink` for `useSeqOrStrOp` and also `atomicRefOp` always need a
`=destroy` hook generated so that is also generated separately. There
might be more that I missed, only the atomicRefOp one failed `trtree` in
CI, and it was just from a compiler assert that got triggered, otherwise
it would still have functioned.

compiler/liftdestructors.nim

@@ -42,6 +42,8 @@ proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode)
 proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp;
               info: TLineInfo; idgen: IdGenerator): PSym
 
+proc createSingleTypeBoundOp*(g: ModuleGraph; c: PContext; orig: PType; op: TTypeAttachedOp;
+                             info: TLineInfo; idgen: IdGenerator)
 proc createTypeBoundOps*(g: ModuleGraph; c: PContext; orig: PType; info: TLineInfo;
                          idgen: IdGenerator)
 
@@ -684,7 +686,7 @@ proc fillSeqOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "wasMoved", x)
 
 proc useSeqOrStrOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
-  createTypeBoundOps(c.g, c.c, t, body.info, c.idgen)
+  createSingleTypeBoundOp(c.g, c.c, t, c.kind, body.info, c.idgen)
   # recursions are tricky, so we might need to forward the generated
   # operation here:
   var t = t
@@ -703,8 +705,12 @@ proc useSeqOrStrOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     # we always inline the move for better performance:
     let moveCall = genBuiltin(c, mMove, "move", x)
     moveCall.add y
-    doAssert t.destructor != nil
-    moveCall.add destructorCall(c, t.destructor, x)
+    var destructor = t.destructor
+    if destructor == nil or destructor.ast.isGenericRoutine:
+      createSingleTypeBoundOp(c.g, c.c, t, attachedDestructor, body.info, c.idgen)
+      destructor = t.destructor
+    doAssert destructor != nil
+    moveCall.add destructorCall(c, destructor, x)
     body.add moveCall
     # alternatively we could do this:
     when false:
@@ -789,7 +795,7 @@ proc atomicRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   var actions = newNodeI(nkStmtList, c.info)
   let elemType = t.elementType
 
-  createTypeBoundOps(c.g, c.c, elemType, c.info, c.idgen)
+  createSingleTypeBoundOp(c.g, c.c, elemType, c.kind, c.info, c.idgen)
 
   # YRC uses dedicated runtime procs for the entire write barrier:
   if c.g.config.selectedGC == gcYrc:
@@ -822,6 +828,8 @@ proc atomicRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     else:
       x
 
+  if t.destructor == nil or t.destructor.ast.isGenericRoutine:
+    createSingleTypeBoundOp(c.g, c.c, elemType, attachedDestructor, body.info, c.idgen)
   if isFinal(elemType):
     addDestructorCall(c, elemType, actions, genDeref(tmp, nkDerefExpr))
     var alignOf = genBuiltin(c, mAlignOf, "alignof", newNodeIT(nkType, c.info, elemType))
@@ -1415,6 +1423,32 @@ proc inst(g: ModuleGraph; c: PContext; t: PType; kind: TTypeAttachedOp; idgen: I
 proc isTrivial*(s: PSym): bool {.inline.} =
   s == nil or (s.ast != nil and s.ast[bodyPos].len == 0)
 
+proc createSingleTypeBoundOp(g: ModuleGraph; c: PContext; orig: PType; op: TTypeAttachedOp;
+                             info: TLineInfo; idgen: IdGenerator) =
+  ## like `createTypeBoundOps` but only generates a single hook
+  if orig == nil or {tfCheckedForDestructor, tfHasMeta} * orig.flags != {}: return
+
+  let skipped = orig.skipTypes({tyGenericInst, tyAlias, tySink})
+  if isEmptyContainer(skipped) or skipped.kind == tyStatic: return
+
+  let h = sighashes.hashType(skipped, g.config, {CoType, CoConsiderOwned, CoDistinct})
+  var canon = g.canonTypes.getOrDefault(h)
+  if canon == nil:
+    g.canonTypes[h] = skipped
+    canon = skipped
+
+  let generic = getAttachedOp(g, canon, op) != nil
+  if not generic:
+    setAttachedOp(g, idgen.module, canon, op,
+        symPrototype(g, canon, canon.owner, op, info, idgen))
+
+  if not generic:
+    discard produceSym(g, c, canon, op, info, idgen)
+  else:
+    inst(g, c, canon, op, idgen, info)
+  if canon != orig:
+    setAttachedOp(g, idgen.module, orig, op, getAttachedOp(g, canon, op))
+
 proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInfo;
                         idgen: IdGenerator) =
   ## In the semantic pass this is called in strategic places

compiler/sempass2.nim

@@ -127,11 +127,10 @@ proc collectObjectTree(graph: ModuleGraph, n: PNode) =
             else:
               graph.objectTree[root].add (depthLevel, typ)
 
-proc createTypeBoundOps(tracked: PEffects, typ: PType; info: TLineInfo; explicit = false) =
-  if typ == nil or (sfGeneratedOp in tracked.owner.flags and not explicit):
+proc createTypeBoundOps(tracked: PEffects, typ: PType; info: TLineInfo) =
+  if typ == nil or sfGeneratedOp in tracked.owner.flags:
     # don't create type bound ops for anything in a function with a `nodestroy` pragma
     # bug #21987
-    # unless this is an explicit call, bug #24626
     return
   when false:
     let realType = typ.skipTypes(abstractInst)
@@ -1166,7 +1165,8 @@ proc trackCall(tracked: PEffects; n: PNode) =
       # rebind type bounds operations after createTypeBoundOps call
       let t = n[1].typ.skipTypes({tyAlias, tyVar, tySink})
       if a.sym != getAttachedOp(tracked.graph, t, opKind):
-        createTypeBoundOps(tracked, t, n.info, explicit = true)
+        # generate called hook regardless of `nodestroy` for explicit call, bug #24626
+        createSingleTypeBoundOp(tracked.graph, tracked.c, t, opKind, n.info, tracked.c.idgen)
         # replace builtin hooks with lifted ones
         n = replaceHookMagic(tracked.c, n, opKind)
 

tests/destructor/trecursivedestructor.nim

@@ -0,0 +1,63 @@
+discard """
+  matrix: "--mm:refc; --mm:orc; --mm:none"
+"""
+
+# issue #25727
+
+type ObjWithSeq = object
+  x: seq[ObjWithSeq]
+
+when not defined(gcDestructors):
+  proc `=destroy`(a: var ObjWithSeq) {.nodestroy.} =
+    `=destroy`(a.x)
+else:
+  proc `=destroy`(a: ObjWithSeq) {.nodestroy.} =
+    `=destroy`(a.x)
+
+proc `=copy`(a: var ObjWithSeq, b: ObjWithSeq) {.nodestroy.} =
+  `=copy`(a.x, b.x)
+  
+proc `=sink`(a: var ObjWithSeq, b: ObjWithSeq) {.nodestroy.} =
+  `=sink`(a.x, b.x)
+
+proc `=dup`(a: ObjWithSeq): ObjWithSeq {.nodestroy.} =
+  ObjWithSeq(x: a.x)
+
+proc `=trace`(a: var ObjWithSeq, env: pointer) {.nodestroy.} =
+  `=trace`(a.x, env)
+
+proc fooSeq() =
+  let a = ObjWithSeq(x: @[ObjWithSeq()])
+  let b = a
+  let c = a
+  let d = b
+fooSeq()
+
+type ObjWithRef = object
+  x: ref ObjWithRef
+
+when not defined(gcDestructors):
+  proc `=destroy`(a: var ObjWithRef) {.nodestroy.} =
+    `=destroy`(a.x)
+else:
+  proc `=destroy`(a: ObjWithRef) {.nodestroy.} =
+    `=destroy`(a.x)
+
+proc `=copy`(a: var ObjWithRef, b: ObjWithRef) {.nodestroy.} =
+  `=copy`(a.x, b.x)
+  
+proc `=sink`(a: var ObjWithRef, b: ObjWithRef) {.nodestroy.} =
+  `=sink`(a.x, b.x)
+
+proc `=dup`(a: ObjWithRef): ObjWithRef {.nodestroy.} =
+  ObjWithRef(x: a.x)
+
+proc `=trace`(a: var ObjWithRef, env: pointer) {.nodestroy.} =
+  `=trace`(a.x, env)
+
+proc fooRef() =
+  let a = ObjWithRef(x: (ref ObjWithRef)())
+  let b = a
+  let c = a
+  let d = b
+fooRef()