check constant conditions in generic when in objects
#24042
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
|
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
This was referenced Sep 2, 2024
Araq
pushed a commit
that referenced
this pull request
Sep 6, 2024
fixes #16700, fixes #20916, refs #24010 Fixes the instantiation issues for proc param default values encountered in #24010 by: 1. semchecking generic default param values with `inGenericContext` for #22029 and followups to apply (the bigger change in semtypes), 2. rejecting explicit generic instantiations with unresolved generic types inside `inGenericContext` (sigmatch change), 3. instantiating the default param values using `prepareNode` rather than an insufficient manual method (the bigger change in seminst). This had an important side effect of references to other parameters not working since they would be resolved as a symbol belonging to the uninstantiated original generic proc rather than the later instantiated proc. There is a more radical way to fix this which is generating ident nodes with `tyFromExpr` in specifically this context, but instead we just count them as belonging to the same proc in `hoistParamsUsedInDefault`. Other minor bugfixes: * To make the error message in t20883 make sense, we now give a "cannot instantiate" error when trying to instantiate a proc generic param with `tyFromExpr`. * Object constructors as default param values generated default values of private fields going through `evalConstExpr` more than once, but the VM doesn't mark the object fields as `nfSkipFieldChecking` which gives a "cannot access field" error. So the VM now marks object fields it generates as `nfSkipFieldChecking`. Not sure if this affects VM performance, don't see why it would. * The nkRecWhen changes in #24042 didn't cover the case where all conditions are constantly false correctly, this is fixed with a minor change. This isn't needed for this PR now but I encountered it after forgetting to `dec c.inGenericContext`.
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 #24041
whenstatements in generic object types normally just leave their conditions as expressions and still typecheck their branch bodies. Instead of this, when the condition can be evaluated as a constant as well as the ones before it and it resolves totrue, it now uses the body of that branch without typechecking the remaining ones.