Minimal changes to compile with OCaml 4.07

[hcarty]

This is a bit of a hack and won't really scale across lots of OCaml versions. It builds with OCaml 4.07.0+rc1 though, so it at least unblocks other testing against the soon-to-be latest OCaml release.

If you want to build rtop you'll also need ocaml-community/utop#238 (or a better equivalent allowing utop to build under 4.07).

[IwanKaramazow]

We should probably update the ci to test 4.07 😄

[hcarty]

Definitely! Is that something you'd like to have in this PR or a separate one? I'm not sure how to approach that since utop is not ready for 4.07 yet.

[hcarty]

And should we add 4.07.0+rc1 now or wait for the final 4.07.0 release?

[jordwalke]

They changed this again in 4.07?

[hcarty]

No change vs 4.06. This is just a copy of the 4.06 file as a quick hack to allow the build to complete with minimal changes. select.ml should eventually be updated to handle this rather than duplicating this code for every new version of OCaml. I unfortunately haven't had time to make such an update.

[jordwalke]

Thank you. Would you mind adding the relevant CI to this PR? After it's merged, I can revert that portion of it, but then as soon as utop is fixed, I'll just unrevert it.

[hcarty]

@jordwalke Thanks for reviewing! I've added separate commits for CircleCI and Travis. These checks could remain and be updated when the final 4.07.0 release lands once they're working properly.

Travis should have the utop pin removed once ocaml-community/utop#238 lands in a utop release. It should also be updated to the 4.07.0 switch once 4.07.0 is released.

CircleCI would only need to have the utop pin removed. The ocaml/opam2 docker image used in that config should be automatically updated to the latest 4.07.x point release as they come out.

[hcarty]

Is the comment formatting test failure on Travis expected?

I just pushed an update to pin to utop's master branch - ocaml-community/utop#238 has been merged so it has proper 4.07 support now.

I'm happy to squash these commits when the PR is ready if you'd prefer that.

[IwanKaramazow]

Hmm the comment failure is not expected. I can probably investigate, if you don't have the time, but it'll be next week.

If you rebase on master you'll have to add a new file mlVariants.re.4.07.0 and copy the contents of mlVariants.re.4.06.0 to it.

[hcarty]

@IwanKaramazow If you have time to look at this I'd appreciate it. It'll be a little while before I can come back to work on this more.

[IwanKaramazow]

I'll see what I can do

[hcarty]

@IwanKaramazow Now that 4.07.0 is official - do you want to finish this up or should I? Aside from rebasing and adding the file you mentioned (#2013 (comment)) do you know if there there is anything else left to do?

[hcarty]

Rebased on master with CI bumped to the actual 4.07.0 release and mlVariants.re.4.07.0 added to the source tree.

There is a still a formatting test failure in comments.mli that I can reproduce locally. Not sure why it happens:

Unit Test: ./comments.mli
diff --git a/Users/travis/build/facebook/reason/formatTest/typeCheckedTests/expected_output/comments.rei b/Users/travis/build/facebook/reason/formatTest/typeCheckedTests/actual_output/comments.rei
index c6f7d58..0d0f98b 100644
--- a/Users/travis/build/facebook/reason/formatTest/typeCheckedTests/expected_output/comments.rei
+++ b/Users/travis/build/facebook/reason/formatTest/typeCheckedTests/actual_output/comments.rei
@@ -1,8 +1,8 @@
 /* **** comment */
 /*** comment */
-/*** docstring */
+/** docstring */;
 /* comment */
-/*** docstring */
+/** docstring */;
 /*** comment */
 /**** comment */
 /***** comment */
  ⊘ FAILED
  /Users/travis/build/facebook/reason/formatTest/typeCheckedTests/actual_output/comments.rei
  doesn't match expected output
  /Users/travis/build/facebook/reason/formatTest/typeCheckedTests/expected_output/comments.rei
  To approve the changes run:
    cp /Users/travis/build/facebook/reason/formatTest/typeCheckedTests/actual_output/comments.rei /Users/travis/build/facebook/reason/formatTest/typeCheckedTests/expected_output/comments.rei

[anmonteiro]

I just rebased this on master and investigated the failure. Here's the difference (refmt is current master's refmt on 4.06.1)

$ echo '(** docstring *)' | refmt --parse ml -past
[]
$ echo '(** docstring *)' | ./_build/install/default/bin/refmt --parse ml -past
[
  structure_item ([1,0+0]..[1,0+16])
    Pstr_attribute "ocaml.text"
    [
      structure_item ([1,0+0]..[1,0+16])
        Pstr_eval
        expression ([1,0+0]..[1,0+16])
          Pexp_constant PConst_string(" docstring ",None)
    ]
]

[anmonteiro]

So the issue here is in 4.07 OCaml parses (** docstring *) as [@@@ocaml.text " docstring "] but in previous versions it just parses it as a comment...

[anmonteiro]

Maybe related to ocaml/ocaml#1693

[IwanKaramazow]

Yes, the parsing behaviour has changed on the ocaml side. A specific test with the behaviour for 4.07 makes sense here (I think)

[hcarty]

Thank you for picking this up @anmonteiro!

[anmonteiro]

I think this is ready now. Can I get an approval? @IwanKaramazow @jordwalke @chenglou

[jordwalke]

Thank you all very much for collaborating on this to get it ready! I look forward to using this.