From a2031ec6cfe9475fb38ebc204ebcf8c2b6d02dce Mon Sep 17 00:00:00 2001
From: metagn <metagngn@gmail.com>
Date: Mon, 18 Nov 2024 19:37:47 +0300
Subject: [PATCH] remove structural equality check for objects and distinct
 types (#24448)

follows up #24425, fixes #18861, fixes #22445

Since #24425 generic object and distinct types now accurately link back
to their generic instantiations. To work around this issue previously,
type matching checked if generic objects/distinct types were
*structurally* equal, which caused false positives with types that
didn't use generic parameters in their structures. This structural check
is now removed, in cases where generic objects/distinct types require a
nontrivial equality check, the generic parameters of the `typeInst`
fields are checked for equality instead.

The check is copied from `tyGenericInst`, but the check in
`tyGenericInst` is not always sufficient as this type can be skipped or
unreachable in the case of `ref object`s.
---
 compiler/types.nim                | 36 +++++++++++++++++++------------
 tests/objects/tgenericsubtype.nim | 34 +++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/compiler/types.nim b/compiler/types.nim
index 66df653a2facb..18f90b75ac799 100644
--- a/compiler/types.nim
+++ b/compiler/types.nim
@@ -1306,26 +1306,34 @@ proc sameTypeAux(x, y: PType, c: var TSameTypeClosure): bool =
       cycleCheck()
       result = sameTypeAux(a.skipModifier, b.skipModifier, c)
   of tyObject:
-    withoutShallowFlags:
+    result = sameFlags(a, b)
+    if result:
       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)
+        # should be generic, and belong to the same generic head type:
+        assert a.typeInst != nil, "generic object " & $a & " has no typeInst"
+        assert b.typeInst != nil, "generic object " & $b & " has no typeInst"
+        if result:
+          withoutShallowFlags:
+            # this is required because of generic `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
   of tyDistinct:
     cycleCheck()
     if c.cmp == dcEq:
-      if sameFlags(a, b):
+      result = sameFlags(a, b)
+      if result:
         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)
+          # should be generic, and belong to the same generic head type:
+          assert a.typeInst != nil, "generic distinct type " & $a & " has no typeInst"
+          assert b.typeInst != nil, "generic distinct type " & $b & " has no typeInst"
+          withoutShallowFlags:
+            # just in case `tyGenericInst` was skipped at some point,
+            # 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
     else:
       result = sameTypeAux(a.elementType, b.elementType, c) and sameFlags(a, b)
   of tyEnum, tyForward:
diff --git a/tests/objects/tgenericsubtype.nim b/tests/objects/tgenericsubtype.nim
index c2618654c695c..8c0596bb90814 100644
--- a/tests/objects/tgenericsubtype.nim
+++ b/tests/objects/tgenericsubtype.nim
@@ -16,3 +16,37 @@ block: # no generic field
   proc foo(x: typedesc[Foo]) = discard
 
   foo(Bar)
+
+block: # issue #22445
+  type
+    MyType = ref object of RootObj
+    MyChild[T] = ref object of MyType
+    
+  var curr = MyChild[int]()
+  doAssert not(curr of MyChild[float])
+  doAssert curr of MyChild[int]
+  doAssert curr of MyType
+
+block: # issue #18861, original case
+  type
+    Connection = ref ConnectionObj
+    ConnectionObj = object of RootObj
+    
+    ConnectionRequest = ref ConnectionRequestObj
+    ConnectionRequestObj = object of RootObj
+      redis: Connection
+    
+    ConnectionStrBool = distinct bool
+    
+    ConnectionRequestT[T] = ref object of ConnectionRequest
+    
+    ConnectionSetRequest = ref object of ConnectionRequestT[ConnectionStrBool]
+      keepttl: bool
+
+  proc execute(req: ConnectionRequest): bool = discard
+  proc execute(req: ConnectionRequestT[bool]): bool = discard
+  proc execute(req: ConnectionRequestT[ConnectionStrBool]): bool = discard
+
+  proc testExecute() =
+    var connection: ConnectionSetRequest
+    let repl = connection.execute()