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

Deprecate let%lwt at the module item level #733

Merged
merged 5 commits into from
Oct 6, 2019
Merged

Conversation

aantron
Copy link
Collaborator

@aantron aantron commented Oct 6, 2019

Using let%lwt as a module item now produces a warning like this:

File "test/ppx/main.ml", line 162, characters 9-41:
162 |          let%lwt result = Lwt.return_true
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning 22: let%lwt should not be used at the module item level.
Replace let%lwt x = e by let x = Lwt_main.run (e)

Resolves #728.


I completely removed support for let%lwt ... and ..., because it was broken. This line

[%expr Lwt.bind [%e pvb_expr] (fun [%p pvar (gen_name i)] -> gen_exp rest (i + 1))]

was accidentally inserting the identifier gen_exp into the output AST, because it is missing an [%e ...] antiquotation, resulting in errors like

File "run_2.ml", line 1, characters 0-49:
Error: Unbound value gen_exp

This means that nobody has ever used let%lwt ... and ... successfully.

I removed the warning that we were trying to print on let%lwt rec ... and ..., because, in this case, the warning seems to be ignored, and the output is simply

File "run_2.ml", line 1, characters 4-7:
Error: Uninterpreted extension 'lwt'.

whether we try to add the warning or not. Given that we will already be discouraging let%lwt at structure item level without and, I think it's fine to have just this uglier message.

The result of these two changes is that we never try to interpret let%lwt ... and ... at all, simplifying the PPX a bit.

@aantron aantron added this to the 4.3.2 milestone Oct 6, 2019
@aantron
Copy link
Collaborator Author

aantron commented Oct 6, 2019

(And I checked the blame about the gen_exp issue; the bug has been in the PPX since the beginning.)

@aantron aantron merged commit 403df4a into master Oct 6, 2019
@aantron aantron deleted the structure-item-ppx branch October 6, 2019 22:46
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.

Deprecate let%lwt structure items (they expand to Lwt_main.run)
1 participant