Exceptions refactoring and try-finally transform #171
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
Nim has a "exception stack" system where inner except branches will push a new exception to the stack then pop it after it's done. To replicate this accurately in CPS, we use one local per cps `try` instead of a single field like how it is done currently. This is because we set the global exception to whatever the stored exception was in our continuation leg, so to prevent inner cps `try`s from overriding what is perceived to be the "current" exception of the outer branch, we simply give them different locals. Ideally we can tap right into Nim's exception `up` field, but it's not exported.
Not sure what I thought of when I wrote that code, but we can just run filter on the outer node and don't have to think about the bugs this `for` would cause us.
Fixes disruptek/cps#164
Instead of creating a continuation leg for each except branch, we merge all except branches into one, then turn that into a continuation. That way we only have one continuation for all handlers, allow for a better implementation of cps exceptions in the future.
Implements try-finally transformation for CPS by generating finally as a continuation leg with a static generic for where it will continues after. Untested because the compiler broke. Known issues: - Early returns are not handled, which can be fixed by improving isScopeExit and making early termination an annotation. Fixes #80.
This was
linked to
issues
Jun 13, 2021
Instead of waiting for nim-lang/Nim#18254 and any other templates bug to be fix. We take the initiative and write our own continuation templater. It appears to work well enough to use as an alternative until a better alternative become available. I've also added an extra test that verify the exception re-raise property.
We ended up not needing it for finally
We implement early returns as `cpsTerminate` then tie them up at the end via `cpsResolver`. This way try-finally can capture early termination and specialize to those. Need @disruptek to review this stuff.
disruptek
approved these changes
Jun 13, 2021
cps/transform.nim
Outdated
Comment on lines
422
to
424
| # The key is the symbol need updating and the value is what to update | ||
| # it to | ||
| var replacements: Table[NimNode, NimNode] |
There was a problem hiding this comment.
Rather than a comment, I'd just use a type declaration. 😁
cps/transform.nim
Outdated
| result = copyNimTree(replacements[n]) | ||
|
|
||
| replacements[replace] = replacement | ||
| templ.filter(generator) |
There was a problem hiding this comment.
eh do we ever use it like this? I think we prefer result = filter(templ, generator) right?
disruptek
approved these changes
Jun 14, 2021
disruptek
added
enhancement
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.
As usual, the compiler blocks our transform:
nim-lang/Nim#18254Worked around that bugBlocked by: nim-lang/Nim#18247
So main changes:
try-exceptis now implemented by merging allexceptbranches into one then turn that into a continuation. This should help with implementing an unwinding system for cps.try-finallyis implemented by turning thefinallyleg into a continuationtaking a generic parameter pointing to where to go next.then specialize it to each exit legs.Known issues:
try-exceptdoesn't work with except branches matching multiple exception types:except ValueError, IOError.try-finallydoesn't handle early returns, but this is fixable by improvingisScopeExitand making early return an annotation.