Skip to content

Commit

Permalink
remove structural equality check for objects and distinct types (#24448)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
metagn authored Nov 18, 2024
1 parent 712f5be commit a2031ec
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 14 deletions.
36 changes: 22 additions & 14 deletions compiler/types.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
34 changes: 34 additions & 0 deletions tests/objects/tgenericsubtype.nim
Original file line number Diff line number Diff line change
Expand Up @@ -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()

0 comments on commit a2031ec

Please sign in to comment.