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

-dsource prints uninterpreted syntax extensions incorrectly #6464

Closed
vicuna opened this issue Jun 19, 2014 · 8 comments
Closed

-dsource prints uninterpreted syntax extensions incorrectly #6464

vicuna opened this issue Jun 19, 2014 · 8 comments
Assignees
Labels
Milestone

Comments

@vicuna
Copy link

vicuna commented Jun 19, 2014

Original bug ID: 6464
Reporter: @whitequark
Assigned to: @alainfrisch
Status: closed (set by @xavierleroy on 2017-02-16T14:16:16Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.0+dev
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: ~DO NOT USE (was: OCaml general)
Related to: #6865
Monitored by: @Drup

Bug description

I.e. it prints

&lwt let _ = raise Not_found

instead of

let%lwt _ = raise Not_found

Seems to be a leftover from earlier syntax.

File attachments

@vicuna
Copy link
Author

vicuna commented Jun 19, 2014

Comment author: @lpw25

This appears to have been fixed by #6377. It now prints:

let _ = [%foo raise Not_found]

However, this raises an interesting issue about the automatic conversion of:

let _ = ...

into a Pstr_eval. It means the extension node is in an unexpected place. For example, by naming the result x we instead get.

[%%foo let x = raise Not_found]

More generally, this conversion is a bit unexpected and likely to cause mistakes in peoples AST transformers (-ppx).

Does anybody know why this conversion is done?

@vicuna
Copy link
Author

vicuna commented Sep 29, 2014

Comment author: @whitequark

I think let _ = .. is just syntax for Pstr_eval? As in, there is no automatic conversion.

Either way, I wrote a lot of ppx extensions and this was never an issue to me.

@vicuna
Copy link
Author

vicuna commented Sep 30, 2014

Comment author: @lpw25

I think let _ = .. is just syntax for Pstr_eval?

The more natural representation would be:

Pstr_value(_, [{pvb_pat = {ppat_desc = Ppat_any; _}; _}])

Either way, I wrote a lot of ppx extensions and this was never an issue to me.

If you wanted to write a ppx that provided a "let%foo" syntax for top-level phrases, and naturally wrote it to detect Pstr_value, then it would not work for:

let%foo _ = ...

If you then added support for Pstr_eval to handle this case, then it would also apply to:

[%%foo 5];;

which may well not be what you want.

In general, any syntactic sugar carried out by the parser makes it more difficult for ppx transformers to apply to the syntax that they are intended for.

@vicuna
Copy link
Author

vicuna commented Sep 30, 2014

Comment author: @Drup

I agree with lpw25. lwt's ppx is affected by this bug (we replace top level let%lwt by a call to "Lwt_main.run".

@vicuna
Copy link
Author

vicuna commented Sep 30, 2014

Comment author: @whitequark

Hm, and I wrote that part of lwt's ppx. Guess I was wrong.

@vicuna
Copy link
Author

vicuna commented Jan 16, 2015

Comment author: @damiendoligez

To summarize, the original bug is fixed by #6377 and now we have a new one:

the parser should not turn let _ = ... into a Pstr_eval, but into a Pstr_value as it does for let x = ...

@vicuna
Copy link
Author

vicuna commented Mar 18, 2015

Comment author: @lpw25

the parser should not turn let _ = ... into a Pstr_eval, but into a Pstr_value as it does for let x = ...

I've attached a patch against 4.02 to fix this

@vicuna
Copy link
Author

vicuna commented Dec 2, 2015

Comment author: @alainfrisch

The special case is gone ( 01bf671 ).

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

2 participants