-
Notifications
You must be signed in to change notification settings - Fork 35
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
Fix dynamic_state to use new Effect syntax #40
Conversation
5acf2d9
to
d611e99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have left a few comments.
let x = perform S1.Get in | ||
perform (S2.Put (string_of_int x ^ "xx")); | ||
perform S2.Get | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should include the runner:
let main3 () =
run3 (module S1) 0 (fun () -> run3 (module S2) "" test3)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be worth noting that layering the integer state inside the string state works as well, i.e.
let main3' () =
run3 (module S2) "" (fun () -> run3 (module S1) 0 test3)
multishot/dynamic_state.ml
Outdated
|
||
This example also includes an unneccessary extra 'Choice' effect to | ||
demonstrate the combination of other effects with state in the same | ||
handler. This uses the experimental Obj.clone_continuation function to clone |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handler. This uses the experimental Obj.clone_continuation function to clone | |
handler. This uses the external `Multicont.Deep.clone_continuation` function to clone |
a := String.length !b; | ||
b := string_of_int !a; | ||
print_endline !b) | ||
else print_endline !b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose for consistency it would be good to include the runner here as well, e.g. main4 () = run4 test4
.
d611e99
to
ffa2111
Compare
ffa2111
to
5175692
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
Fix dynamic_state to use new Effect syntax, could possibly be simplified further. The type checking inside the effect handlers is a bit fragile, if you reorder the matches in
run
orrun2
they no longer type check.version 1 fails with
Exception: Stdlib.Effect.Unhandled(LocalState(R).New(1))
version 2 passed with
int = 5
version 3 I couldn't work out how to run it properly.
version 4 passed producing
3 3