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

[easy-ish] PPX: replace [%lwt ...] by [%lwt.catch ...] or remove it #527

Closed
aantron opened this issue Dec 26, 2017 · 0 comments
Closed

[easy-ish] PPX: replace [%lwt ...] by [%lwt.catch ...] or remove it #527

aantron opened this issue Dec 26, 2017 · 0 comments
Labels
Milestone

Comments

@aantron
Copy link
Collaborator

aantron commented Dec 26, 2017

The catch-all [%lwt ...] syntax has been a source of problems in #307 and #525, due to how PPX works.

In particular, let%lwt x = e in e' is represented as [%lwt let x = e in e'] in the AST, and all the other special ...%lwt keywords (try%lwt, etc.) are represented similarly, i.e. as ordinary, unadorned expressions nested inside an [%lwt ...] extension point.

Besides keywords, the Lwt PPX provides that if you have [%lwt e], then it will be expanded to Lwt.catch (fun () -> e) (fun exn -> Lwt.fail exn). However, because this conflicts with interpreting the keywords (above), this works for all expressions except let, try, etc., and only for them.

This has two problems:

  1. It is a bit difficult to predict and reason about. It's probably a bit scary to refactor, as an exception handler could suddenly disappear if the top level becomes a keyword, though it doesn't have many users.
  2. It slows us down adding interactions between %lwt and additional keywords, such as begin%lwt (Ppx: add support for begin%lwt e1; e2; end in ppx extension #525) and ;%lwt (Support ;%lwt as sequence syntax -- backwards incompatible in some cases #307), because giving those keywords a new special interpretation under %lwt can break the interpretation of already-written user code.

This issue proposes to delete the catch-all interpretation of [%lwt ...] from the PPX. If we need a replacement, I suggest [%lwt.catch ...].

I see [%lwt ...] being used in ocsigen and sqlexpr, perhaps as a result of conversion from Camlp4.

Given how few users there are, I suggest to delete this interpretation entirely.

I think we should do this regardless of whether we merge #525 or #307, because it will simplify Lwt, and give it more options for evolving in the future.

cc @Drup @hcarty @kandu @raphael-proust

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant