always reinstantiate nominal values of generic instantiations

compiler/ast.nim

@@ -447,6 +447,8 @@ const
   tfReturnsNew* = tfInheritable
   tfNonConstExpr* = tfExplicitCallConv
     ## tyFromExpr where the expression shouldn't be evaluated as a static value
+  tfGenericHasDestructor* = tfExplicitCallConv
+    ## tyGenericBody where an instance has a generated destructor
   skError* = skUnknown
 
 var

compiler/liftdestructors.nim

@@ -1276,6 +1276,10 @@ proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInf
   ## The later 'injectdestructors' pass depends on it.
   if orig == nil or {tfCheckedForDestructor, tfHasMeta} * orig.flags != {}: return
   incl orig.flags, tfCheckedForDestructor
+  # for user defined generic destructors:
+  let origRoot = genericRoot(orig)
+  if origRoot != nil:
+    incl origRoot.flags, tfGenericHasDestructor
 
   let skipped = orig.skipTypes({tyGenericInst, tyAlias, tySink})
   if isEmptyContainer(skipped) or skipped.kind == tyStatic: return

compiler/semstmts.nim

@@ -2035,14 +2035,27 @@ proc canonType(c: PContext, t: PType): PType =
   else:
     result = t
 
-proc prevDestructor(c: PContext; prevOp: PSym; obj: PType; info: TLineInfo) =
-  var msg = "cannot bind another '" & prevOp.name.s & "' to: " & typeToString(obj)
-  if sfOverridden notin prevOp.flags:
+proc prevDestructor(c: PContext; op: TTypeAttachedOp; prevOp: PSym; obj: PType; info: TLineInfo) =
+  var msg = "cannot bind another '" & AttachedOpToStr[op] & "' to: " & typeToString(obj)
+  if prevOp == nil:
+    # happens if the destructor was implicitly constructed for a specific instance,
+    # not the entire generic type
+    msg.add "; previous declaration was constructed implicitly"
+  elif sfOverridden notin prevOp.flags:
     msg.add "; previous declaration was constructed here implicitly: " & (c.config $ prevOp.info)
   else:
     msg.add "; previous declaration was here: " & (c.config $ prevOp.info)
   localError(c.config, info, errGenerated, msg)
 
+proc checkedForDestructor(t: PType): bool =
+  if tfCheckedForDestructor in t.flags:
+    return true
+  # maybe another instance was instantiated, marking the generic root:
+  let root = genericRoot(t)
+  if root != nil and tfGenericHasDestructor in root.flags:
+    return true
+  result = false
+
 proc whereToBindTypeHook(c: PContext; t: PType): PType =
   result = t
   while true:
@@ -2076,10 +2089,10 @@ proc bindDupHook(c: PContext; s: PSym; n: PNode; op: TTypeAttachedOp) =
       let ao = getAttachedOp(c.graph, obj, op)
       if ao == s:
         discard "forward declared destructor"
-      elif ao.isNil and tfCheckedForDestructor notin obj.flags:
+      elif ao.isNil and not checkedForDestructor(obj):
         setAttachedOp(c.graph, c.module.position, obj, op, s)
       else:
-        prevDestructor(c, ao, obj, n.info)
+        prevDestructor(c, op, ao, obj, n.info)
       noError = true
       if obj.owner.getModule != s.getModule:
         localError(c.config, n.info, errGenerated,
@@ -2123,10 +2136,10 @@ proc bindTypeHook(c: PContext; s: PSym; n: PNode; op: TTypeAttachedOp) =
       let ao = getAttachedOp(c.graph, obj, op)
       if ao == s:
         discard "forward declared destructor"
-      elif ao.isNil and tfCheckedForDestructor notin obj.flags:
+      elif ao.isNil and not checkedForDestructor(obj):
         setAttachedOp(c.graph, c.module.position, obj, op, s)
       else:
-        prevDestructor(c, ao, obj, n.info)
+        prevDestructor(c, op, ao, obj, n.info)
       noError = true
       if obj.owner.getModule != s.getModule:
         localError(c.config, n.info, errGenerated,
@@ -2217,10 +2230,10 @@ proc semOverride(c: PContext, s: PSym, n: PNode) =
         let ao = getAttachedOp(c.graph, obj, k)
         if ao == s:
           discard "forward declared op"
-        elif ao.isNil and tfCheckedForDestructor notin obj.flags:
+        elif ao.isNil and not checkedForDestructor(obj):
           setAttachedOp(c.graph, c.module.position, obj, k, s)
         else:
-          prevDestructor(c, ao, obj, n.info)
+          prevDestructor(c, k, ao, obj, n.info)
         if obj.owner.getModule != s.getModule:
           localError(c.config, n.info, errGenerated,
             "type bound operation `" & name & "` can be defined only in the same module with its type (" & obj.typeToString() & ")")

compiler/semtypinst.nim

@@ -80,7 +80,7 @@ type
     owner*: PSym              # where this instantiation comes from
     recursionLimit: int
 
-proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType): PType
+proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType, isInstValue = false): PType
 proc replaceTypeVarsS(cl: var TReplTypeVars, s: PSym, t: PType): PSym
 proc replaceTypeVarsN*(cl: var TReplTypeVars, n: PNode; start=0; expectedType: PType = nil): PNode
 
@@ -95,8 +95,8 @@ template checkMetaInvariants(cl: TReplTypeVars, t: PType) = # noop code
       debug t
       writeStackTrace()
 
-proc replaceTypeVarsT*(cl: var TReplTypeVars, t: PType): PType =
-  result = replaceTypeVarsTAux(cl, t)
+proc replaceTypeVarsT*(cl: var TReplTypeVars, t: PType, isInstValue = false): PType =
+  result = replaceTypeVarsTAux(cl, t, isInstValue)
   checkMetaInvariants(cl, result)
 
 proc prepareNode*(cl: var TReplTypeVars, n: PNode): PNode =
@@ -481,7 +481,7 @@ proc handleGenericInvocation(cl: var TReplTypeVars, t: PType): PType =
     return
 
   let bbody = last body
-  var newbody = replaceTypeVarsT(cl, bbody)
+  var newbody = replaceTypeVarsT(cl, bbody, isInstValue = true)
   cl.skipTypedesc = oldSkipTypedesc
   newbody.flags = newbody.flags + (t.flags + body.flags - tfInstClearedFlags)
   result.flags = result.flags + newbody.flags - tfInstClearedFlags
@@ -578,7 +578,7 @@ proc propagateFieldFlags(t: PType, n: PNode) =
       propagateFieldFlags(t, son)
   else: discard
 
-proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType): PType =
+proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType, isInstValue = false): PType =
   template bailout =
     if (t.sym == nil) or (t.sym != nil and sfGeneratedType in t.sym.flags):
       # In the first case 't.sym' can be 'nil' if the type is a ref/ptr, see
@@ -712,13 +712,17 @@ proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType): PType =
     propagateToOwner(result, result.last)
 
   else:
-    if containsGenericType(t):
+    if containsGenericType(t) or
+        # nominal types as direct generic instantiation values
+        # are re-instantiated even if they don't contain generic fields
+        (isInstValue and (t.kind in {tyDistinct, tyObject} or isRefPtrObject(t))):
       #if not cl.allowMetaTypes:
       bailout()
       result = instCopyType(cl, t)
       result.size = -1 # needs to be recomputed
       #if not cl.allowMetaTypes:
       cl.localCache[t.itemId] = result
+      let propagateInstValue = isInstValue and isRefPtrObject(t)
 
       for i, resulti in result.ikids:
         if resulti != nil:
@@ -728,7 +732,7 @@ proc replaceTypeVarsTAux(cl: var TReplTypeVars, t: PType): PType =
               typeToString(result[i], preferDesc) &
               "' inside of type definition: '" &
               t.owner.name.s & "'; Maybe generic arguments are missing?")
-          var r = replaceTypeVarsT(cl, resulti)
+          var r = replaceTypeVarsT(cl, resulti, isInstValue = propagateInstValue)
           if result.kind == tyObject:
             # carefully coded to not skip the precious tyGenericInst:
             let r2 = r.skipTypes({tyAlias, tySink, tyOwned})

compiler/types.nim

@@ -1309,12 +1309,22 @@ proc sameTypeAux(x, y: PType, c: var TSameTypeClosure): bool =
     withoutShallowFlags:
       ifFastObjectTypeCheckFailed(a, b):
         cycleCheck()
+        if a.typeInst != nil and b.typeInst != nil:
+          # this is required because of `ref object`s,
+          # the value of their dereferences are not wrapped in `tyGenericInst`,
+          # so we need to check the generic parameters here
+          for ff, aa in underspecifiedPairs(a.typeInst, b.typeInst, 1, -1):
+            if not sameTypeAux(ff, aa, c): return false
+        # XXX should be removed in favor of above lines,
+        # structural equality is wrong in general:
         result = sameObjectStructures(a, b, c) and sameFlags(a, b)
   of tyDistinct:
     cycleCheck()
     if c.cmp == dcEq:
       if sameFlags(a, b):
         ifFastObjectTypeCheckFailed(a, b):
+          # XXX should be removed in favor of checking generic params,
+          # structural equality is wrong in general:
           result = sameTypeAux(a.elementType, b.elementType, c)
     else:
       result = sameTypeAux(a.elementType, b.elementType, c) and sameFlags(a, b)

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)

tests/destructor/tinvalid_rebind.nim

@@ -1,7 +1,7 @@
 discard """
 joinable: false
 cmd: "nim check $file"
-errormsg: "cannot bind another '=destroy' to: Foo; previous declaration was constructed here implicitly: tinvalid_rebind.nim(12, 7)"
+errormsg: "cannot bind another '=destroy' to: Foo; previous declaration was constructed implicitly"
 line: 14
 """
 

tests/destructor/tinvalid_rebind_nonempty.nim

@@ -0,0 +1,16 @@
+discard """
+joinable: false
+cmd: "nim check $file"
+errormsg: "cannot bind another '=destroy' to: Foo; previous declaration was constructed implicitly"
+line: 15
+"""
+
+type
+  Foo[T] = object
+    x: T
+
+proc main =
+  var f: Foo[int]
+
+proc `=destroy`[T](f: var Foo[T]) =
+  discard