fix sym of created generic instantiation type #22642
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Araq
reviewed
Sep 4, 2023
|
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
metagn
added a commit
to metagn/Nim
that referenced
this pull request
Sep 5, 2023
reverts nim-lang#22642 but keeps nim-lang#22639 fixed
metagn
added a commit
to metagn/Nim
that referenced
this pull request
Sep 5, 2023
reverts nim-lang#22642, reopens nim-lang#22639, closes nim-lang#22646, refs nim-lang#22640, refs alaviss/union#51
alaviss
added a commit
to alaviss/union
that referenced
this pull request
Sep 5, 2023
Previously we performed tests against Union symbol itself to check for inheritance. This relied on an implementation detail where an instance of a generic has the same type as the generic type itself. Recent changes in the compiler shows that we cannot rely on this. Thus the module now opt for inheriting from and testing against a pre-defined instance of Union. Ref nim-lang/Nim#22642 Fixes #51
alaviss
added a commit
to alaviss/union
that referenced
this pull request
Sep 5, 2023
Previously we performed tests against Union symbol itself to check for inheritance. This relied on an implementation detail where an instance of a generic in the AST uses the same symbol as the generic type. The PR nim-lang/Nim#22642 changed this implementation detail. So now, we have to account for both cases of new and old compilers by also testing against a pre-defined instance. Fixes #51
metagn
added a commit
that referenced
this pull request
Sep 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #22639
A
tyGenericInsthas its last son as the instantiated body of the original generic type. However this type keeps its originalsymfield from the original generic types, which means the sym's type is uninstantiated. This causes problems in the implementation ofgetType, where it uses thesymfields of types to represent them in AST, the relevant example for the issue being here called from here.To fix this, create a new symbol from the original symbol for the instantiated body during the creation of
tyGenericInsts with the appropriate type. NormallyreplaceTypeVarsSwould be used for this, but here it seems to cause some recursion issue (immediately gives an error like "cannot instantiate HSlice[T, U]"), so we directly set the symbol's type to the instantiated type.Avoiding recursion means we also cannot use
replaceTypeVarsNfor the symbol AST, and the symbol not having any AST crashes the implementation ofgetTypeagain here, so the symbol AST is set to the original generic type AST for now which is what it was before anyway.Not sure about this because not sure why the recursion issue is happening, putting it at the end of the proc doesn't help either. Also not sure if the
cl.owner != nil and s.owner != cl.ownercondition fromreplaceTypeVarsSis relevant here. This might also break some code if it depended on the original generic type symbol being given.