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

Update to OCaml 5.0 implementation of effects #7

Merged
merged 21 commits into from Nov 17, 2022

Conversation

tanelso2
Copy link
Contributor

@tanelso2 tanelso2 commented Nov 1, 2022

No description provided.

@tanelso2
Copy link
Contributor Author

tanelso2 commented Nov 1, 2022

Two things I'm worried about:

  1. I deleted a section on free monads and how they relate to effects handlers because my implementation of that section didn't really use any of those concept

  2. the section on gdb is mostly correct, but does not provide exactly the same input/output because the implementation has changed

@kayceesrk
Copy link
Contributor

Hi @tanelso2. Thanks for the work! It looks good to me overall.

I deleted a section on free monads and how they relate to effects handlers because my implementation of that section didn't really use any of those concept

That seems fine to me.

the section on gdb is mostly correct, but does not provide exactly the same input/output because the implementation has changed

We've simplified the implementation of effect handlers and the gdb support. Hence, the changes are expected.

@kayceesrk
Copy link
Contributor

kayceesrk commented Nov 1, 2022

Perhaps @Sudha247 may also have a look at this PR.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Comment on lines -276 to -277
An effect system for OCaml
[is in development](https://www.youtube.com/watch?t=2035&v=z8SI7WBtlcA&feature=youtu.be).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is still true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this is aimed at being a tutorial for the initial release of effects, does it still make sense to say it is "in development"?

It seems out of place and doesn't flow well

## 2. Effectful Computations in a Pure Setting
## 2. Shallow vs Deep Handlers

The OCaml standard library provides two different modules for handling effects: `Effect.Deep` and `Effect.Shallow`. When a deep handler returns a continuation, the continuation also includes the handler. This means that, when the continuation is resumed, the effect handler is automatically re-installed, and will handle the effect(s) that the computation may perform in the future.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wish we hadn't named the module Effect.Shallow as the handler therein are not shallow. Genuine shallow handlers are more expressive in a certain sense than the (sheep) handlers exposed by that module. I guess it is too late to make this change upstream @kayceesrk.

Copy link
Contributor

Choose a reason for hiding this comment

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

Naming things is hard. Would you call these Sheep handlers rather than Shallow handlers? In any case, it may be useful to create an issue on ocaml/ocaml with a justification for why Shallow handlers may not be the most appropriate name.

Copy link
Contributor Author

@tanelso2 tanelso2 Nov 12, 2022

Choose a reason for hiding this comment

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

Can someone link me something on the difference between shallow and sheep handlers? I had trouble finding any information on this with Google, not sure where I should be looking

sources/input_line_eff.ml Show resolved Hide resolved
Copy link
Collaborator

@Sudha247 Sudha247 left a comment

Choose a reason for hiding this comment

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

Thanks for the work @tanelso2! I've left some comments below. Thanks for also updating from travis to GitHub Actions. Unrelated to this PR, we could maybe update it to run the code on a OCaml 5 pipeline.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@Sudha247
Copy link
Collaborator

GitHub Actions job passed: https://github.com/tanelso2/ocaml-effects-tutorial/actions/runs/3476866298. It doesn't show up on the PR, I think it will be activated if the PR is merged. The outstanding comments have been addressed.

@kayceesrk
Copy link
Contributor

If there are no further outstanding comments, feel free to merge @Sudha247

@Sudha247 Sudha247 merged commit 49dc407 into ocaml-multicore:master Nov 17, 2022
@Sudha247
Copy link
Collaborator

Thanks again @tanelso2!

@tanelso2
Copy link
Contributor Author

@Sudha247 Happy to help!

Been really excited about algebraic effects as a concept and wanted to learn how to use them

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.

None yet

4 participants