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

JS runtime: Extract RESUME function #708

Merged
merged 2 commits into from
Nov 22, 2024
Merged

JS runtime: Extract RESUME function #708

merged 2 commits into from
Nov 22, 2024

Conversation

dvdvgt
Copy link
Collaborator

@dvdvgt dvdvgt commented Nov 22, 2024

No description provided.

meta.stack = null
return body(resumeComp, meta, k1)
const resume = (c, ks, k) => RESUME(c, cont, ks, k)
return body(resume, meta, k1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you plan to change this to:

body(c, meta, k1)

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, I think it would not work if I did so.

Copy link
Collaborator

@b-studios b-studios Nov 22, 2024

Choose a reason for hiding this comment

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

Here I am doing the same (modulo direct style)

return [cont, meta, k1]

This requires changes to the transformation, like in the other PR #679 .

case core.Stmt.Resume(cont, body) =>
val ks2 = Id("ks")
val k2 = Id("k")
Resume(cont.id, Block.BlockLit(Nil, Nil, ks2, k2, transform(body, ks2, Continuation.Dynamic(k2))),
MetaCont(ks), k.reify)

@b-studios
Copy link
Collaborator

I took the liberty to make the above mentioned changes.

@dvdvgt
Copy link
Collaborator Author

dvdvgt commented Nov 22, 2024

Was just about to push the cherry-picked commits myself. Thanks for the help, though :)

@b-studios
Copy link
Collaborator

Sorry if this duplicated work that you wanted to do :(

@b-studios
Copy link
Collaborator

From my side, we can merge this now. Thanks for your help -- finally I can put #679 to rest :)

@b-studios b-studios mentioned this pull request Nov 22, 2024
@dvdvgt
Copy link
Collaborator Author

dvdvgt commented Nov 22, 2024

Not sure whether I was of much help here, but sure :) LGTM

@b-studios b-studios merged commit adf76cf into master Nov 22, 2024
2 checks passed
@b-studios b-studios deleted the feature/js-resume branch November 22, 2024 21:15
dvdvgt added a commit that referenced this pull request Nov 22, 2024
b-studios added a commit that referenced this pull request Dec 24, 2024
Co-authored-by: Jonathan Brachthäuser <jonathan@b-studios.de>
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.

2 participants