Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0007196OCaml~DO NOT USE (was: OCaml general)public2016-03-26 23:322017-09-24 17:32
Reportercraff 
Assigned Togasche 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformlinuxOSDebianOS Versionstable
Product Version4.02.3 
Target VersionFixed in Version4.03.0+dev / +beta1 
Summary0007196: does not print "let open" correctly in "if"
DescriptionConsider the following file
-------------------
let _ =
  if true then (
    let open List in
    ();
    ());
  ()
-------------------

The pretty printing of ast is wrong:

$ ocamlc -dsource bug.ml
let _ = if true then let open List in ((); ()); ()

it should be

let _ = if true then (let open List in ((); ())); ()

Otherwise, the last unit is parser inside the then branch.
Steps To ReproduceGiven above
Additional Information4.03.0 is affected too, I did not check 4.01.0
TagsNo tags attached.
Attached Files

- Relationships

-  Notes
(0015618)
craff (reporter)
2016-03-27 12:06
edited on: 2016-03-27 17:02

I wrote major, because this can break bootstraping of -pp or -ppx extensions
that use ascii pretty printing as a simple way to boostrap (by producing
.ml/.mli file that do not use the extension)

Actually this broke decap bootstrap at some point and forced me not to use the "let open" syntax inside conditions

Moreover, this should be an easy fix ...

(0015627)
gasche (administrator)
2016-03-27 17:09

Indeed, this was very simple to fix:
  https://github.com/ocaml/ocaml/commit/c676334278bcc5295b5df6c97deb842aeeb7966e [^]
Thanks for the report!

Despite a few rewrites, the pretty-printing machinery is still baroque and I wouldn't trust it. It would be nice to overhaul it to exactly match the precedence levels and nonterminals used in the parser, to get verifiably correct minimal parentheses insertion. I'd rather get the parser converted to Menhir first to work from a nicer grammar.
(0015628)
craff (reporter)
2016-03-27 17:26

I use extensively the pretty printing to bootstrap decap and this is the first time I have a problem ...

What would be a real gain is to change the grammar itself, the priority of
if then else should not change by adding a let / fun / etc ...

This is a nightmare if you do not use the yacc way of giving precedence,
which is the case when pretty printing.

The nice things, is that changing the grammar at this place actually breaks little code (I tested), and can be easily detected by parsing both with the old and the new parser.

I would suggest a few deep revision of the syntax (in places that hurt students):

- if then else priority
- optional endif, endfun, endmatch endfunction
- "in" replaced by ";" (YES)
- let replaced by val in structure (syntax error sooner in the code)

If this change are not mandatory, a smooth transition is possible
(0015629)
craff (reporter)
2016-03-27 17:26

And thanks for the fix !!!

- Issue History
Date Modified Username Field Change
2016-03-26 23:32 craff New Issue
2016-03-27 12:06 craff Note Added: 0015618
2016-03-27 12:13 craff Note Edited: 0015618 View Revisions
2016-03-27 12:13 craff Note Edited: 0015618 View Revisions
2016-03-27 17:02 craff Note Edited: 0015618 View Revisions
2016-03-27 17:09 gasche Note Added: 0015627
2016-03-27 17:09 gasche Status new => resolved
2016-03-27 17:09 gasche Fixed in Version => 4.03.0+dev / +beta1
2016-03-27 17:09 gasche Resolution open => fixed
2016-03-27 17:09 gasche Assigned To => gasche
2016-03-27 17:26 craff Note Added: 0015628
2016-03-27 17:26 craff Note Added: 0015629
2017-02-23 16:36 doligez Category OCaml general => -OCaml general
2017-03-03 17:55 doligez Category -OCaml general => -(deprecated) general
2017-03-03 18:01 doligez Category -(deprecated) general => ~deprecated (was: OCaml general)
2017-03-06 17:04 doligez Category ~deprecated (was: OCaml general) => ~DO NOT USE (was: OCaml general)
2017-09-24 17:32 xleroy Status resolved => closed


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker