-
Notifications
You must be signed in to change notification settings - Fork 172
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
Comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 toLwt.catch (fun () -> e) (fun exn -> Lwt.fail exn)
. However, because this conflicts with interpreting the keywords (above), this works for all expressions exceptlet
,try
, etc., and only for them.This has two problems:
%lwt
and additional keywords, such asbegin%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 inocsigen
andsqlexpr
, 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
The text was updated successfully, but these errors were encountered: