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

Reverts nim-lang/Nim#25729

compiler/liftdestructors.nim

@@ -42,8 +42,6 @@ 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)
 
@@ -686,7 +684,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) =
-  createSingleTypeBoundOp(c.g, c.c, t, c.kind, body.info, c.idgen)
+  createTypeBoundOps(c.g, c.c, t, body.info, c.idgen)
   # recursions are tricky, so we might need to forward the generated
   # operation here:
   var t = t
@@ -705,12 +703,8 @@ 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
-    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)
+    doAssert t.destructor != nil
+    moveCall.add destructorCall(c, t.destructor, x)
     body.add moveCall
     # alternatively we could do this:
     when false:
@@ -795,7 +789,7 @@ proc atomicRefOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   var actions = newNodeI(nkStmtList, c.info)
   let elemType = t.elementType
 
-  createSingleTypeBoundOp(c.g, c.c, elemType, c.kind, c.info, c.idgen)
+  createTypeBoundOps(c.g, c.c, elemType, c.info, c.idgen)
 
   # YRC uses dedicated runtime procs for the entire write barrier:
   if c.g.config.selectedGC == gcYrc:
@@ -828,8 +822,6 @@ 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))
@@ -1423,32 +1415,6 @@ 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,10 +127,11 @@ proc collectObjectTree(graph: ModuleGraph, n: PNode) =
             else:
               graph.objectTree[root].add (depthLevel, typ)
 
-proc createTypeBoundOps(tracked: PEffects, typ: PType; info: TLineInfo) =
-  if typ == nil or sfGeneratedOp in tracked.owner.flags:
+proc createTypeBoundOps(tracked: PEffects, typ: PType; info: TLineInfo; explicit = false) =
+  if typ == nil or (sfGeneratedOp in tracked.owner.flags and not explicit):
     # 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)
@@ -1165,8 +1166,7 @@ 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):
-        # generate called hook regardless of `nodestroy` for explicit call, bug #24626
-        createSingleTypeBoundOp(tracked.graph, tracked.c, t, opKind, n.info, tracked.c.idgen)
+        createTypeBoundOps(tracked, t, n.info, explicit = true)
         # replace builtin hooks with lifted ones
         n = replaceHookMagic(tracked.c, n, opKind)
 

tests/destructor/trecursivedestructor.nim

@@ -1,63 +0,0 @@
-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()