Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006464OCamlOCaml generalpublic2014-06-19 17:552015-03-18 18:18
Reporterwhitequark 
Assigned Tolpw25 
PrioritynormalSeverityminorReproducibilityalways
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version4.02.0+dev 
Target Versionafter-4.02.2Fixed in Version 
Summary0006464: -dsource prints uninterpreted syntax extensions incorrectly
DescriptionI.e. it prints

    &lwt let _ = raise Not_found

instead of

    let%lwt _ = raise Not_found

Seems to be a leftover from earlier syntax.
TagsNo tags attached.
Attached Filespatch file icon fix-6464.patch [^] (1,147 bytes) 2015-03-18 18:18 [Show Content]

- Relationships

-  Notes
(0011762)
lpw25 (developer)
2014-06-19 23:27

This appears to have been fixed by 0006377. 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?
(0012258)
whitequark (developer)
2014-09-30 00:25

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.
(0012260)
lpw25 (developer)
2014-09-30 11:37
edited on: 2014-09-30 11:38

> 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.

(0012263)
drup (reporter)
2014-09-30 17:47

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".
(0012265)
whitequark (developer)
2014-09-30 23:18

Hm, and I wrote that part of lwt's ppx. Guess I was wrong.
(0013121)
doligez (administrator)
2015-01-16 17:30

To summarize, the original bug is fixed by 0006377 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 = ...`
(0013519)
lpw25 (developer)
2015-03-18 18:18

> 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

- Issue History
Date Modified Username Field Change
2014-06-19 17:55 whitequark New Issue
2014-06-19 23:27 lpw25 Note Added: 0011762
2014-07-16 10:32 doligez Status new => acknowledged
2014-07-16 10:32 doligez Product Version => 4.02.0+dev
2014-07-16 10:32 doligez Target Version => 4.02.1+dev
2014-09-04 00:25 doligez Target Version 4.02.1+dev => undecided
2014-09-14 22:37 doligez Target Version undecided => 4.02.2+dev
2014-09-30 00:25 whitequark Note Added: 0012258
2014-09-30 11:37 lpw25 Note Added: 0012260
2014-09-30 11:38 lpw25 Note Edited: 0012260 View Revisions
2014-09-30 17:47 drup Note Added: 0012263
2014-09-30 23:18 whitequark Note Added: 0012265
2015-01-16 17:30 doligez Note Added: 0013121
2015-01-16 17:31 doligez Target Version 4.02.2+dev => after-4.02.2
2015-03-18 17:41 lpw25 Assigned To => lpw25
2015-03-18 17:41 lpw25 Status acknowledged => assigned
2015-03-18 18:18 lpw25 File Added: fix-6464.patch
2015-03-18 18:18 lpw25 Note Added: 0013519


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker