Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove structural equality check for objects and distinct types #24448

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Nov 17, 2024

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 objects.

@metagn metagn changed the title test removing structural equality for objects and distinct types remove structural equality check for objects and distinct types Nov 17, 2024
@metagn metagn marked this pull request as ready for review November 17, 2024 16:38
@Araq Araq merged commit a2031ec into nim-lang:devel Nov 18, 2024
18 checks passed
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from a2031ec

Hint: mm: orc; opt: speed; options: -d:release
177825 lines; 8.643s; 653.555MiB peakmem

narimiran pushed a commit that referenced this pull request Jan 14, 2025
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.

(cherry picked from commit a2031ec)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants