From 7309115d7729c62cc44db12c7c1a400cd7d4ac35 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Wed, 7 Aug 2024 23:06:00 +0200
Subject: [PATCH] fix wrong hooks being picked for tuple types (#1402)

## Summary

* fix tuples such as `((A, B),)` and `((A,), B)` being assigned the
  the same set of hooks
* fix the aforementioned tuples sharing the same RTTI object

## Details

* add the tuple length to the type representation during type hashing
  (`sighash`), so that `((A,), B)` and `((A, B),)` result in two
  different `SigHash`es
* `liftdestructors` uses `sighashes` for canonicalizing structural
  types, thus previously treating said tuples as the same type
* RTTI creation, which also uses `SigHash`, was affected in the same
  way
---
 compiler/sem/sighashes.nim                    |  3 ++
 .../ttuple_hook_synthesis_issue.nim           | 30 +++++++++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 tests/lang_objects/destructor/ttuple_hook_synthesis_issue.nim

diff --git a/compiler/sem/sighashes.nim b/compiler/sem/sighashes.nim
index e56fc615aed..152deff135f 100644
--- a/compiler/sem/sighashes.nim
+++ b/compiler/sem/sighashes.nim
@@ -188,6 +188,9 @@ proc hashType(c: var MD5Context, t: PType; flags: set[ConsiderFlag]) =
     c.hashTree(t.n, {})
   of tyTuple:
     c &= char(t.kind)
+    # add the length so that (int, int) and ((int,), int) have different
+    # representations
+    c &= t.len
     if t.n != nil and CoType notin flags:
       assert(t.n.len == t.len)
       for i in 0..<t.n.len:
diff --git a/tests/lang_objects/destructor/ttuple_hook_synthesis_issue.nim b/tests/lang_objects/destructor/ttuple_hook_synthesis_issue.nim
new file mode 100644
index 00000000000..1ccfb0d09c6
--- /dev/null
+++ b/tests/lang_objects/destructor/ttuple_hook_synthesis_issue.nim
@@ -0,0 +1,30 @@
+discard """
+  description: '''
+    Ensure that hook synthesis considers ``(A, (B, C))`` different from
+    ``(A, (B,), C)``.
+  '''
+  output: "3\n4\n1\n2\n"
+  knownIssue.js vm: "seq destructors don't work yet"
+"""
+
+type Object = object
+  val: int
+
+proc `=destroy`(x: var Object) =
+  echo x.val
+
+# a seq is only used for testing purposes, since it caused misbehaviour at
+# run-time. Using ``(Object, (Object,) Object)`` and
+# ``(Object, (Object, Object))`` also led to the same issue, but ran
+# "correctly" due to both types having the same in-memory layout
+type
+  Tup1 = (seq[(Object,)], Object)
+  Tup2 = (seq[(Object, Object)],)
+
+proc test() =
+  var x: Tup1 = (@[(Object(val: 1),)], Object(val: 2))
+  var y: Tup2 = (@[(Object(val: 3), Object(val: 4))],)
+  # both locations were treated as having type `Tup1` by the called
+  # destructor, resulting in either crashes or the wrong output being produced
+
+test()