From e81f5b58900baa01b4f4c7fba78c8a10b4bcb6a2 Mon Sep 17 00:00:00 2001 From: Andreas Rumpf Date: Mon, 13 Apr 2026 12:02:53 +0200 Subject: [PATCH] Revert "only generate called hook for explicit or generated destructor calls [backport]" (#25741) Reverts nim-lang/Nim#25729 --- compiler/liftdestructors.nim | 42 ++------------- compiler/sempass2.nim | 8 +-- tests/destructor/trecursivedestructor.nim | 63 ----------------------- 3 files changed, 8 insertions(+), 105 deletions(-) delete mode 100644 tests/destructor/trecursivedestructor.nim diff --git a/compiler/liftdestructors.nim b/compiler/liftdestructors.nim index 748e0ac201..15c60363f8 100644 --- a/compiler/liftdestructors.nim +++ b/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 diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 9530abdd06..35901ed960 100644 --- a/compiler/sempass2.nim +++ b/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) diff --git a/tests/destructor/trecursivedestructor.nim b/tests/destructor/trecursivedestructor.nim deleted file mode 100644 index 9a8159aa72..0000000000 --- a/tests/destructor/trecursivedestructor.nim +++ /dev/null @@ -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()