don't instantiate generic body type symbols in generic expressions #24092
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
|
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 10, 2024
Araq
pushed a commit
that referenced
this pull request
Sep 11, 2024
fixes regression remaining after #24092 In #24092 `prepareNode` was updated so it wouldn't try to instantiate generic type symbols (like `Generic` when `type Generic[T] = object`, and `prepareNode` is what `tyFromExpr` uses in most of the compiler. An exception is in sigmatch, which is now changed to use `prepareNode` to make generic type symbols work in the same way as usual. However this requires another change to work: Dot fields and matches to `typedesc` on generic types generate `tyFromExpr` in generic contexts since #24005, including generic type symbols. But this means when we try to instantiate the `tyFromExpr` in sigmatch, which increases `c.inGenericContext` for potentially remaining unresolved expressions, dotcalls stay as `tyFromExpr` and so never match. To fix this, we change the "generic type" check in dot fields and `typedesc` matching to an "unresolved type" check which excludes generic body types; and for generic body types, we only generate `tyFromExpr` if the dot field is a generic parameter of the generic type (so that it gets resolved only at instantiation). Notes for the future: * Sigmatch shouldn't have to `inc c.inGenericContext`, if a `tyFromExpr` can't instantiate it's fine if we just fail the match (i.e. redirect the instantiation errors from `semtypinst` to a match failure). Then again maybe this is the best way to check for inability to instantiate. * The `elif c.inGenericContext > 0 and t.containsUnresolvedType` check in dotfields could maybe be simplified to just checking for `tyFromExpr` and `tyGenericParam`, but I don't know if this is an exhaustive list.
metagn
added a commit
to metagn/Nim
that referenced
this pull request
Oct 10, 2024
Araq
pushed a commit
that referenced
this pull request
Oct 10, 2024
fixes #24091, refs #24092 Any instantiations resolving to a generic body type now gives an error. Due to #24092, this does not error in cases like matching against `type M` in generics because generic body type symbols are just not instantiated. But this prevents parameters with type `type M` from being used, although there doesn't seem to be any code which does this. Just in case such code exists, we still allow `typedesc` types resolving to generic body types.
narimiran
pushed a commit
that referenced
this pull request
Jan 14, 2025
fixes #24091, refs #24092 Any instantiations resolving to a generic body type now gives an error. Due to #24092, this does not error in cases like matching against `type M` in generics because generic body type symbols are just not instantiated. But this prevents parameters with type `type M` from being used, although there doesn't seem to be any code which does this. Just in case such code exists, we still allow `typedesc` types resolving to generic body types. (cherry picked from commit 2f90453)
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 #24090
Generic body types are normally a sign of an uninstantiated type, and so give errors when trying to instantiate them. However when instantiating free user expressions like the nodes of
tyFromExpr, generic default params, static values etc, they can be used as arguments to macros or templates etc (as in the issue). So, we don't try to instantiate generic body type symbols at all in free expressions such as these (but not in for example type nodes), and avoid the error.In the future there should be a "concrete type" check for generic body types different from the check in type instantiation to deal with things like #24091, if we do want to allow this use of them.