Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cps: add limited exception handling support #94

Closed
wants to merge 4 commits into from

Conversation

alaviss
Copy link
Contributor

@alaviss alaviss commented May 13, 2021

Experimental and not fully tested yet

Splitting inside any except or finally bodies are
not supported and will yield an error

/cc @disruptek I need you to break this

cps.nim Outdated
of nnkProcDef:
if not n.hasPragma("cpsContinuation"):
result = n
of nnkReturnStmt, nnkBreakStmt, nnkContinueStmt:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only have to handle nnkReturnStmt, any nnkBreakStmt or nnkContinueStmt should be within the same continuation, thus finally won't trigger.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, finally is rewritten into block, right?

Copy link
Contributor Author

@alaviss alaviss May 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, I just replicate it across all legs, with a tag letting us know whether the finally should be run. There might be a bug or two with that, but I haven't figured out any tests yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The finally clause should always be run; that's the point. When you rewrite it into block statements, you produce two (or more) control-flow paths and all receive a copy of the finally code -- perhaps via a template. Any subsequent rewrite deals with these different paths via our normal block-handling logic, so the finally effectively evaporates.

It's kinda like your solution to if expressions... You dig down into the logic and say, "okay, now what are we really trying to say here..." and that statement is "treat this as an expression on the RHS of an assignment".

In this case, what we are really trying to say is, "do this stuff regardless of how you exit the scope".

cps.nim Outdated
desym cont

# var ok {.inject.} = false
handlerWrapperBody.add newVarStmt(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need this stuff if there is a finally body, since it is an indicator for whether the finally branch should be executed

@alaviss alaviss linked an issue May 15, 2021 that may be closed by this pull request
Experimental and not fully tested yet

Splitting inside any `except` or `finally` bodies are
not supported and will yield an error
- use workaroundRewrites at the end
- make wrapper uses a typed body
- move var decl to outside of wrapper
- don't set ok for non-escaping control-flow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect rewrite for defer
2 participants