fix heap-cell cleanup calling the wrong destructor

Summary
=======

Fix `ref`s to objects calling the wrong destructor upon the refcount
reaching zero in some narrow circumstances involving generic types and
`sink` type modifiers.

Details
=======

Due to the sighash-based canonicalization used prior to type-bound op
lifting ignoring `sink` modifiers, the same hook was lifted for both
`ref Obj[proc(x: sink T)]` and `ref Obj[proc(x: T)]`, which led to the
wrong destructor being called on cleanup when the object type is final.

In a very narrow situation, where all of the following apply:
* the `ref` type itself is generic
* the object type is never used outside the `ref` type
* the object type has no user-defined destructor, only an implicit one
* the object type is non-final
* the types reach the MIR phase in the wrong order
the canonicalization led to the object not having its destructor called
at all, due to the object's destructor not being lifted, thus
potentially *leaking memory* if the object type requires destruction
for some of its fields.

Two types for which `sameType(a, b)` results in `false` should produce
different sighashes, and so hashing is changed to not ignore `sink`
types, fixing both issues.

In addition, the scanning for types to lift the ops for in `sempass2`
is fixed to handle generic ref types used as the constructor in object
construction expressions, which makes the code work as intended.

Author: zerbina

compiler/sem/sempass2.nim

@@ -1345,9 +1345,9 @@ proc track(tracked: PEffects, n: PNode) =
         checkForSink(tracked.config, tracked.c.idgen, tracked.owner, x)
     setLen(tracked.guards.s, oldFacts)
     if tracked.owner.kind != skMacro:
-      # XXX n.typ can be nil in runnableExamples, we need to do something about it.
-      if n.typ != nil and n.typ.skipTypes(abstractInst).kind == tyRef:
-        createTypeBoundOps(tracked, n.typ.lastSon, n.info)
+      let skipped = n.typ.skipTypes(abstractInst)
+      if skipped.kind == tyRef:
+        createTypeBoundOps(tracked, skipped.lastSon, n.info)
       createTypeBoundOps(tracked, n.typ, n.info)
   of nkTupleConstr:
     for i in 0..<n.len:

compiler/sem/sighashes.nim

@@ -132,7 +132,7 @@ proc hashType(c: var MD5Context, t: PType; flags: set[ConsiderFlag]) =
         c.hashType t[i], flags
     else:
       c.hashType t.lastSon, flags
-  of tyAlias, tySink, tyUserTypeClasses, tyInferred:
+  of tyAlias, tyUserTypeClasses, tyInferred:
     c.hashType t.lastSon, flags
   of tyBool, tyChar, tyInt..tyUInt64:
     # no canonicalization for integral types, so that e.g. ``pid_t`` is
@@ -179,7 +179,7 @@ proc hashType(c: var MD5Context, t: PType; flags: set[ConsiderFlag]) =
       c &= t.id
     if t.len > 0 and t[0] != nil:
       hashType c, t[0], flags
-  of tyRef, tyPtr, tyGenericBody, tyVar:
+  of tyRef, tyPtr, tyGenericBody, tyVar, tySink:
     c &= char(t.kind)
     c.hashType t.lastSon, flags
   of tyFromExpr:

tests/lang_objects/destructor/twrong_destructor_called_on_ref_cleanup.nim

@@ -0,0 +1,17 @@
+discard """
+  description: '''
+    Regression test for certain ref types calling the wrong destructor
+  '''
+  output: '''proc (x: int){.closure.}
+proc (x: sink int){.closure.}
+'''
+"""
+
+type Object[T] = object
+  x: T
+
+proc `=destroy`[T](x: var Object[T]) =
+  echo T
+
+discard (ref Object[proc(x: sink int)])()
+discard (ref Object[proc(x: int)])()