make sigmatch use prepareNode for tyFromExpr #24095
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
metagn
changed the title
|
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 11, 2024
Araq
pushed a commit
that referenced
this pull request
Sep 11, 2024
follows up #24095 In #24095 a check was added that used `iterOverType` to check if a type contained unresolved types, with the aim of always treating `tyGenericBody` as resolved. But the body of the `tyGenericBody` is also iterated over in `iterOverType`, so if the body of the type actually used generic parameters (which isn't the case in the test added in #24095, but is now), the check would still count the type as unresolved. This is handled by not iterating over the children of `tyGenericBody`, the only users of `iterOverType` are `containsGenericType` and `containsUnresolvedType`, the first one always returns true for `tyGenericBody` and the second one aims to always return false. Unfortunately this means `iterOverType` isn't as generic of an API anymore but maybe it shouldn't be used anymore for these procs.
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 regression remaining after #24092
In #24092
prepareNodewas updated so it wouldn't try to instantiate generic type symbols (likeGenericwhentype Generic[T] = object, andprepareNodeis whattyFromExpruses in most of the compiler. An exception is in sigmatch, which is now changed to useprepareNodeto make generic type symbols work in the same way as usual. However this requires another change to work:Dot fields and matches to
typedescon generic types generatetyFromExprin generic contexts since #24005, including generic type symbols. But this means when we try to instantiate thetyFromExprin sigmatch, which increasesc.inGenericContextfor potentially remaining unresolved expressions, dotcalls stay astyFromExprand so never match. To fix this, we change the "generic type" check in dot fields andtypedescmatching to an "unresolved type" check which excludes generic body types; and for generic body types, we only generatetyFromExprif the dot field is a generic parameter of the generic type (so that it gets resolved only at instantiation).Notes for the future:
inc c.inGenericContext, if atyFromExprcan't instantiate it's fine if we just fail the match (i.e. redirect the instantiation errors fromsemtypinstto a match failure). Then again maybe this is the best way to check for inability to instantiate.elif c.inGenericContext > 0 and t.containsUnresolvedTypecheck in dotfields could maybe be simplified to just checking fortyFromExprandtyGenericParam, but I don't know if this is an exhaustive list.