[WIP] Early evaluation of mIs #8723
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
The `evalIs` implementation was just a broken copy of `isOpImpl` so let's just avoid it alltogether: `mIs` nodes are either resolved during the semantic phase or bust.
zah
reviewed
Aug 22, 2018
compiler/semexprs.nim
Outdated
| @@ -409,8 +409,10 @@ proc semIs(c: PContext, n: PNode, flags: TExprFlags): PNode = | |||
| else: | |||
| if lhsType.base.kind == tyNone: | |||
| # this is a typedesc variable, leave for evals | |||
| debug lhsType | |||
There was a problem hiding this comment.
This was intentional, in case the CI run triggered this code path. I'll just remove this branch.
zah
reviewed
Aug 22, 2018
| result = evalIs(n, lhs.sym, g) | ||
| # The only kind of mIs node that comes here is one depending on some | ||
| # generic parameter and that's (hopefully) handled at instantiation time | ||
| discard |
There was a problem hiding this comment.
You can go further and add an internalAssert false here, but it might be wiser to do this after the 0.19 release.
There was a problem hiding this comment.
I'm thinking of something like this as a safety net:
internalAssert g.config, n[1].typ.base.containsGenericTypeEdit: scrap that, we can't do much here since the semFold may be invoked at different times (see tactors test)
zah
approved these changes
Aug 22, 2018
|
Looks good to me too but what are the risks it breaks something? I think it's safer to apply this after 0.19 is out. |
krux02
pushed a commit
to krux02/Nim
that referenced
this pull request
Oct 15, 2018
* Early evaluation of mIs The `evalIs` implementation was just a broken copy of `isOpImpl` so let's just avoid it alltogether: `mIs` nodes are either resolved during the semantic phase or bust. * Remove dead code and tidy it up
narimiran
pushed a commit
to narimiran/Nim
that referenced
this pull request
Oct 31, 2018
* Early evaluation of mIs The `evalIs` implementation was just a broken copy of `isOpImpl` so let's just avoid it alltogether: `mIs` nodes are either resolved during the semantic phase or bust. * Remove dead code and tidy it up
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.
CC @zah
evalIsis dead and nobody will shed a single tearinGenericContextmakes sure we either resolve this expression here if possible or we resolve it after the generic instantation phase.lhsType.base.kind == tyNonepath to triggertisopr