only skip generic instances of structural types for destructors

compiler/ast.nim

@@ -1628,6 +1628,20 @@ proc skipTypesOrNil*(t: PType, kinds: TTypeKinds): PType =
     if result.sons.len == 0: return nil
     result = last(result)
 
+proc skipStructuralGenerics*(t: PType, otherKinds: TTypeKinds = {}): PType =
+  ## skips `otherKinds` and generic instantiations in `t`,
+  ## given that the generic instantiations are not of direct
+  ## object/enum/distinct types
+  # note: ref/ptr types are excluded (i.e. `type Foo[T] = ref object`)
+  # in practice this is not an issue since destructors are defined on
+  # direct `object` etc types, but in general the underlying `Foo:Obj`
+  # type will not have/use a corresponding `tyGenericInst`
+  result = t
+  while result.kind in otherKinds or
+      (result.kind == tyGenericInst and
+        result.skipModifier.kind notin {tyObject, tyEnum, tyDistinct}):
+    result = result.last
+
 proc isGCedMem*(t: PType): bool {.inline.} =
   result = t.kind in {tyString, tyRef, tySequence} or
            t.kind == tyProc and t.callConv == ccClosure

compiler/injectdestructors.nim

@@ -251,11 +251,11 @@ proc genOp(c: var Con; t: PType; kind: TTypeAttachedOp; dest, ri: PNode): PNode
   c.genOp(op, dest)
 
 proc genDestroy(c: var Con; dest: PNode): PNode =
-  let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink})
+  let t = dest.typ.skipStructuralGenerics({tyAlias, tySink})
   result = c.genOp(t, attachedDestructor, dest, nil)
 
 proc canBeMoved(c: Con; t: PType): bool {.inline.} =
-  let t = t.skipTypes({tyGenericInst, tyAlias, tySink})
+  let t = t.skipStructuralGenerics({tyAlias, tySink})
   if optOwnedRefs in c.graph.config.globalOptions:
     result = t.kind != tyRef and getAttachedOp(c.graph, t, attachedSink) != nil
   else:
@@ -281,7 +281,7 @@ proc genSink(c: var Con; s: var Scope; dest, ri: PNode; flags: set[MoveOrCopyFla
     # optimize sink call into a bitwise memcopy
     result = newTree(nkFastAsgn, dest, ri)
   else:
-    let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink})
+    let t = dest.typ.skipStructuralGenerics({tyAlias, tySink})
     if getAttachedOp(c.graph, t, attachedSink) != nil:
       result = c.genOp(t, attachedSink, dest, ri)
       result.add ri
@@ -336,7 +336,7 @@ proc genMarkCyclic(c: var Con; result, dest: PNode) =
         result.add callCodegenProc(c.graph, "nimMarkCyclic", dest.info, xenv)
 
 proc genCopyNoCheck(c: var Con; dest, ri: PNode; a: TTypeAttachedOp): PNode =
-  let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink})
+  let t = dest.typ.skipStructuralGenerics({tyAlias, tySink})
   result = c.genOp(t, a, dest, ri)
   assert ri.typ != nil
 
@@ -392,7 +392,7 @@ It is best to factor out piece of object that needs custom destructor into separ
   result.add newTree(nkFastAsgn, le, tmp)
 
 proc genWasMoved(c: var Con, n: PNode): PNode =
-  let typ = n.typ.skipTypes({tyGenericInst, tyAlias, tySink})
+  let typ = n.typ.skipStructuralGenerics({tyAlias, tySink})
   let op = getAttachedOp(c.graph, n.typ, attachedWasMoved)
   if op != nil:
     if sfError in op.flags:
@@ -450,7 +450,7 @@ proc passCopyToSink(n: PNode; c: var Con; s: var Scope): PNode =
   let nTyp = n.typ.skipTypes(tyUserTypeClasses)
   let tmp = c.getTemp(s, nTyp, n.info)
   if hasDestructor(c, nTyp):
-    let typ = nTyp.skipTypes({tyGenericInst, tyAlias, tySink})
+    let typ = nTyp.skipStructuralGenerics({tyAlias, tySink})
     let op = getAttachedOp(c.graph, typ, attachedDup)
     if op != nil and tfHasOwned notin typ.flags:
       if sfError in op.flags:

compiler/liftdestructors.nim

@@ -38,7 +38,7 @@ template assignment*(t: PType): PSym = getAttachedOp(c.g, t, attachedAsgn)
 template dup*(t: PType): PSym = getAttachedOp(c.g, t, attachedDup)
 template asink*(t: PType): PSym = getAttachedOp(c.g, t, attachedSink)
 
-proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode)
+proc fillBody(c: var TLiftCtx; orig: PType; body, x, y: PNode)
 proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp;
               info: TLineInfo; idgen: IdGenerator; isDistinct = false): PSym
 
@@ -369,7 +369,7 @@ proc instantiateGeneric(c: var TLiftCtx; op: PSym; t, typeInst: PType): PSym =
       "cannot generate destructor for generic type: " & typeToString(t))
     result = nil
 
-proc considerAsgnOrSink(c: var TLiftCtx; t: PType; body, x, y: PNode;
+proc considerAsgnOrSink(c: var TLiftCtx; orig, t: PType; body, x, y: PNode;
                         field: var PSym): bool =
   if optSeqDestructors in c.g.config.globalOptions:
     var op = field
@@ -409,7 +409,7 @@ proc considerAsgnOrSink(c: var TLiftCtx; t: PType; body, x, y: PNode;
     onUse(c.info, op)
     # We also now do generic instantiations in the destructor lifting pass:
     if op.ast.isGenericRoutine:
-      op = instantiateGeneric(c, op, t, t.typeInst)
+      op = instantiateGeneric(c, op, t, orig)
       field = op
       #echo "trying to use ", op.ast
       #echo "for ", op.name.s, " "
@@ -428,8 +428,8 @@ proc addDestructorCall(c: var TLiftCtx; orig: PType; body, x: PNode) =
   if op != nil and sfOverridden in op.flags:
     if op.ast.isGenericRoutine:
       # patch generic destructor:
-      op = instantiateGeneric(c, op, t, t.typeInst)
-      setAttachedOp(c.g, c.idgen.module, t, attachedDestructor, op)
+      op = instantiateGeneric(c, op, t, orig)
+      setAttachedOp(c.g, c.idgen.module, orig, attachedDestructor, op)
 
   if op == nil and (useNoGc(c, t) or requiresDestructor(c, t)):
     op = produceSym(c.g, c.c, t, attachedDestructor, c.info, c.idgen)
@@ -444,16 +444,16 @@ proc addDestructorCall(c: var TLiftCtx; orig: PType; body, x: PNode) =
     internalError(c.g.config, c.info,
       "type-bound operator could not be resolved")
 
-proc considerUserDefinedOp(c: var TLiftCtx; t: PType; body, x, y: PNode): bool =
+proc considerUserDefinedOp(c: var TLiftCtx; orig, t: PType; body, x, y: PNode): bool =
   case c.kind
   of attachedDestructor:
     var op = t.destructor
     if op != nil and sfOverridden in op.flags:
 
       if op.ast.isGenericRoutine:
         # patch generic destructor:
-        op = instantiateGeneric(c, op, t, t.typeInst)
-        setAttachedOp(c.g, c.idgen.module, t, attachedDestructor, op)
+        op = instantiateGeneric(c, op, t, orig)
+        setAttachedOp(c.g, c.idgen.module, orig, attachedDestructor, op)
 
       #markUsed(c.g.config, c.info, op, c.g.usageSym)
       onUse(c.info, op)
@@ -467,10 +467,10 @@ proc considerUserDefinedOp(c: var TLiftCtx; t: PType; body, x, y: PNode): bool =
     if op != nil and sfOverridden in op.flags:
       if op.ast.isGenericRoutine:
         # patch generic =trace:
-        op = instantiateGeneric(c, op, t, t.typeInst)
+        op = instantiateGeneric(c, op, t, orig)
         setAttachedOp(c.g, c.idgen.module, t, c.kind, op)
 
-    result = considerAsgnOrSink(c, t, body, x, y, op)
+    result = considerAsgnOrSink(c, orig, t, body, x, y, op)
     if op != nil:
       setAttachedOp(c.g, c.idgen.module, t, c.kind, op)
 
@@ -490,7 +490,7 @@ proc considerUserDefinedOp(c: var TLiftCtx; t: PType; body, x, y: PNode): bool =
 
       if op.ast.isGenericRoutine:
         # patch generic destructor:
-        op = instantiateGeneric(c, op, t, t.typeInst)
+        op = instantiateGeneric(c, op, t, orig)
         setAttachedOp(c.g, c.idgen.module, t, attachedWasMoved, op)
 
       #markUsed(c.g.config, c.info, op, c.g.usageSym)
@@ -506,7 +506,7 @@ proc considerUserDefinedOp(c: var TLiftCtx; t: PType; body, x, y: PNode): bool =
 
       if op.ast.isGenericRoutine:
         # patch generic destructor:
-        op = instantiateGeneric(c, op, t, t.typeInst)
+        op = instantiateGeneric(c, op, t, orig)
         setAttachedOp(c.g, c.idgen.module, t, attachedDup, op)
 
       #markUsed(c.g.config, c.info, op, c.g.usageSym)
@@ -954,7 +954,8 @@ proc ownedClosureOp(c: var TLiftCtx; t: PType; body, x, y: PNode) =
   of attachedTrace: discard
   of attachedWasMoved: body.add genBuiltin(c, mWasMoved, "`=wasMoved`", x)
 
-proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode) =
+proc fillBody(c: var TLiftCtx; orig: PType; body, x, y: PNode) =
+  let t = orig.skipTypes({tyGenericInst, tyAlias})
   case t.kind
   of tyNone, tyEmpty, tyVoid: discard
   of tyPointer, tySet, tyBool, tyChar, tyEnum, tyInt..tyUInt64, tyCstring,
@@ -1000,7 +1001,7 @@ proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     elif optSeqDestructors in c.g.config.globalOptions:
       # note that tfHasAsgn is propagated so we need the check on
       # 'selectedGC' here to determine if we have the new runtime.
-      discard considerUserDefinedOp(c, t, body, x, y)
+      discard considerUserDefinedOp(c, orig, t, body, x, y)
     elif tfHasAsgn in t.flags:
       if c.kind in {attachedAsgn, attachedSink, attachedDeepCopy}:
         body.add newSeqCall(c, x, y)
@@ -1011,11 +1012,11 @@ proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode) =
     if useNoGc(c, t):
       useSeqOrStrOp(c, t, body, x, y)
     elif tfHasAsgn in t.flags:
-      discard considerUserDefinedOp(c, t, body, x, y)
+      discard considerUserDefinedOp(c, orig, t, body, x, y)
     else:
       defaultOp(c, t, body, x, y)
   of tyObject:
-    if not considerUserDefinedOp(c, t, body, x, y):
+    if not considerUserDefinedOp(c, orig, t, body, x, y):
       if t.sym != nil and sfImportc in t.sym.flags:
         case c.kind
         of {attachedAsgn, attachedSink, attachedDup}:
@@ -1036,7 +1037,7 @@ proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode) =
         else:
           fillBodyObjT(c, t, body, x, y)
   of tyDistinct:
-    if not considerUserDefinedOp(c, t, body, x, y):
+    if not considerUserDefinedOp(c, orig, t, body, x, y):
       fillBody(c, t.elementType, body, x, y)
   of tyTuple:
     fillBodyTup(c, t, body, x, y)
@@ -1052,10 +1053,9 @@ proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode) =
      tyTypeDesc, tyGenericInvocation, tyForward, tyStatic:
     #internalError(c.g.config, c.info, "assignment requested for type: " & typeToString(t))
     discard
-  of tyOrdinal, tyRange, tyInferred,
-     tyGenericInst, tyAlias, tySink:
+  of tyOrdinal, tyRange, tyInferred, tySink:
     fillBody(c, skipModifier(t), body, x, y)
-  of tyConcept, tyIterable: raiseAssert "unreachable"
+  of tyConcept, tyIterable, tyAlias, tyGenericInst: raiseAssert "unreachable"
 
 proc produceSymDistinctType(g: ModuleGraph; c: PContext; typ: PType;
                             kind: TTypeAttachedOp; info: TLineInfo;
@@ -1253,13 +1253,13 @@ proc patchBody(g: ModuleGraph; c: PContext; n: PNode; info: TLineInfo; idgen: Id
         n[0] = newSymNode(op)
   for x in n: patchBody(g, c, x, info, idgen)
 
-proc inst(g: ModuleGraph; c: PContext; t: PType; kind: TTypeAttachedOp; idgen: IdGenerator;
+proc inst(g: ModuleGraph; c: PContext; orig, t: PType; kind: TTypeAttachedOp; idgen: IdGenerator;
           info: TLineInfo) =
   let op = getAttachedOp(g, t, kind)
   if op != nil and op.ast != nil and op.ast.isGenericRoutine:
     if t.typeInst != nil:
       var a = TLiftCtx(info: info, g: g, kind: kind, c: c, idgen: idgen)
-      let opInst = instantiateGeneric(a, op, t, t.typeInst)
+      let opInst = instantiateGeneric(a, op, t, orig)
       if opInst.ast != nil:
         patchBody(g, c, opInst.ast, info, a.idgen)
       setAttachedOp(g, idgen.module, t, kind, opInst)
@@ -1277,7 +1277,7 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf
   if orig == nil or {tfCheckedForDestructor, tfHasMeta} * orig.flags != {}: return
   incl orig.flags, tfCheckedForDestructor
 
-  let skipped = orig.skipTypes({tyGenericInst, tyAlias, tySink})
+  let skipped = orig.skipStructuralGenerics({tyAlias, tySink})
   if isEmptyContainer(skipped) or skipped.kind == tyStatic: return
 
   let h = sighashes.hashType(skipped, g.config, {CoType, CoConsiderOwned, CoDistinct})
@@ -1301,7 +1301,7 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf
   # bug #15122: We need to produce all prototypes before entering the
   # mind boggling recursion. Hacks like these imply we should rewrite
   # this module.
-  var generics: array[attachedWasMoved..attachedTrace, bool] = default(array[attachedWasMoved..attachedTrace, bool])
+  var generics = default(array[attachedWasMoved..attachedTrace, bool])
   for k in attachedWasMoved..lastAttached:
     generics[k] = getAttachedOp(g, canon, k) != nil
     if not generics[k]:
@@ -1313,7 +1313,7 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf
     if not generics[k]:
       discard produceSym(g, c, canon, k, info, idgen)
     else:
-      inst(g, c, canon, k, idgen, info)
+      inst(g, c, canon, canon, k, idgen, info)
     if canon != orig:
       setAttachedOp(g, idgen.module, orig, k, getAttachedOp(g, canon, k))
 

compiler/sighashes.nim

@@ -120,14 +120,17 @@ proc hashType(c: var MD5Context, t: PType; flags: set[ConsiderFlag]; conf: Confi
     else:
       c.hashSym(t.sym)
   of tyGenericInst:
-    if sfInfixCall in t.base.sym.flags:
+    if sfInfixCall in t.base.sym.flags or CoConsiderOwned in flags:
       # This is an imported C++ generic type.
       # We cannot trust the `lastSon` to hold a properly populated and unique
       # value for each instantiation, so we hash the generic parameters here:
-      let normalizedType = t.skipGenericAlias
-      c.hashType normalizedType.genericHead, flags, conf
-      for _, a in normalizedType.genericInstParams:
-        c.hashType a, flags, conf
+      let normalizedType = t.skipStructuralGenerics
+      if normalizedType.kind == tyGenericInst:
+        c.hashType normalizedType.genericHead, flags, conf
+        for _, a in normalizedType.genericInstParams:
+          c.hashType a, flags, conf
+      else:
+        c.hashType normalizedType, flags, conf
     else:
       c.hashType t.skipModifier, flags, conf
   of tyAlias, tySink, tyUserTypeClasses, tyInferred:

compiler/types.nim

@@ -1242,25 +1242,25 @@ proc sameTypeAux(x, y: PType, c: var TSameTypeClosure): bool =
 
   if x == y: return true
   let aliasSkipSet = maybeSkipRange({tyAlias})
-  var a = skipTypes(x, aliasSkipSet)
+  var a = skipStructuralGenerics(x, aliasSkipSet)
   while a.kind == tyUserTypeClass and tfResolved in a.flags:
-    a = skipTypes(a.last, aliasSkipSet)
-  var b = skipTypes(y, aliasSkipSet)
+    a = skipStructuralGenerics(a.last, aliasSkipSet)
+  var b = skipStructuralGenerics(y, aliasSkipSet)
   while b.kind == tyUserTypeClass and tfResolved in b.flags:
-    b = skipTypes(b.last, aliasSkipSet)
+    b = skipStructuralGenerics(b.last, aliasSkipSet)
   assert(a != nil)
   assert(b != nil)
   case c.cmp
   of dcEq:
     if a.kind != b.kind: return false
   of dcEqIgnoreDistinct:
     let distinctSkipSet = maybeSkipRange({tyDistinct, tyGenericInst})
-    a = a.skipTypes(distinctSkipSet)
-    b = b.skipTypes(distinctSkipSet)
+    a = a.skipStructuralGenerics(distinctSkipSet)
+    b = b.skipStructuralGenerics(distinctSkipSet)
     if a.kind != b.kind: return false
   of dcEqOrDistinctOf:
     let distinctSkipSet = maybeSkipRange({tyDistinct, tyGenericInst})
-    a = a.skipTypes(distinctSkipSet)
+    a = a.skipStructuralGenerics(distinctSkipSet)
     if a.kind != b.kind: return false
 
   #[

tests/arc/t22218.nim

@@ -1,6 +1,6 @@
 discard """
   cmd: "nim c --mm:arc $file"
-  errormsg: "'=copy' is not available for type <Obj>; requires a copy because it's not the last read of 'chan[]'; routine: test"
+  errormsg: "'=copy' is not available for type <Obj[system.int]>; requires a copy because it's not the last read of 'chan[]'; routine: test"
 """
 
 # bug #22218
@@ -22,4 +22,4 @@ proc test() =
     echo chan.v
     echo v
 
-test()
+test()

tests/arc/tphantomgeneric1.nim

@@ -0,0 +1,19 @@
+discard """
+  output: '''
+int
+float
+'''
+"""
+
+# issue #22479
+
+type Obj[T] = object
+
+proc `=destroy`[T](self: var Obj[T]) =
+  echo T
+
+block:
+  let intObj = Obj[int]()
+
+block:
+  let floatObj = Obj[float]()

tests/arc/tphantomgeneric2.nim

@@ -0,0 +1,33 @@
+discard """
+  output: '''
+created Phantom[system.float] with value 1
+created Phantom[system.string] with value 2
+created Phantom[system.byte] with value 3
+destroyed Phantom[system.byte] with value 3
+destroyed Phantom[system.string] with value 2
+destroyed Phantom[system.float] with value 1
+'''
+"""
+
+# issue #24374
+
+type Phantom[T] = object
+  value: int
+  # markerField: T
+
+proc initPhantom[T](value: int): Phantom[T] =
+  doAssert value >= 0
+  echo "created " & $Phantom[T] & " with value " & $value
+  result = Phantom[T](value: value)
+
+proc `=wasMoved`[T](x: var Phantom[T]) =
+  x.value = -1
+
+proc `=destroy`[T](x: Phantom[T]) =
+  if x.value >= 0:
+    echo "destroyed " & $Phantom[T] & " with value " & $x.value
+
+let
+  x = initPhantom[float](1)
+  y = initPhantom[string](2)
+  z = initPhantom[byte](3)