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

fix object subtype-relation between generic instances #847

Merged

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Aug 18, 2023

Summary

Fix a bug with the subtype-of-generic-object logic that caused generic
instances of object-like types to be treated as unrelated when one of
their in-between types is a ref/ptr type while the formal type is
not (or vice versa). Refer to the added test for an example of where
this issue surfaced.

Details

The mistake was considering the skipped ptr/ref at each inheritance
step, while it must only be considered once for the formal and original
actual type. This properly considers ref and ptr types being used
as object base types.

In addition to the fix, the documentation of and inside
isSubtypeOfGenericInstance is slightly improved.

Summary
=======

Fix a bug with the subtype-of-generic-object logic that caused generic
instances of object-like types to be treated as unrelated when one of
their in-between types is a `ref`/`ptr` type while the formal type is
not (or vice versa). Refer to the added test for an example of where
this issue surfaced.

Details
=======

The mistake was considering the skipped ptr/ref at each inheritance
step, while it must only be considered once for the formal and original
actual type. This properly considers `ref` and `ptr` types being used
as object base types.

In addition to the fix, the documentation of and inside
`isSubtypeOfGeneric` is slightly improved.
@zerbina zerbina added bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler labels Aug 18, 2023
@zerbina zerbina requested a review from saem August 18, 2023 17:44

First = ref object of Root[int]
Second = object of First
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no change for this PR. musing outloud really, but I do wonder about this feature... maybe it's ok under the "what else could it be?" although those sorts of things preclude future extension of the language.

Copy link
Collaborator

@saem saem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a really good find, and fix. 😃

@saem
Copy link
Collaborator

saem commented Aug 18, 2023

/merge

@github-actions
Copy link

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

  • I noticed this while working on other sigmatch fixes/changes/improvements

@chore-runner chore-runner bot added this pull request to the merge queue Aug 18, 2023
Merged via the queue into nim-works:devel with commit 802e1aa Aug 18, 2023
18 checks passed
@zerbina zerbina deleted the fix-generic-object-subtype-rel branch August 21, 2023 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants