Skip to content

Improve: switch to ast 4.08#831

Merged
gpetiot merged 71 commits intomasterfrom
ast-408
Jun 17, 2019
Merged

Improve: switch to ast 4.08#831
gpetiot merged 71 commits intomasterfrom
ast-408

Conversation

@hhugo
Copy link
Copy Markdown
Collaborator

@hhugo hhugo commented May 14, 2019 •
edited by gpetiot
Loading

fix #630

  • plug the 4.08 ast in the code
  • remove hack: sort object fields
  • remove hack: dispatch ([@ vs [@@) attributes on exception.
  • process locations to toplevel directives and their arguments (pdir_loc, pdira_loc)
  • process locations in attributes (attr_loc)
  • process locations in row_field (prf_loc)
  • process locations in object_field (pof_loc)
  • process locations in type_extension (ptyext_loc)
  • handle letop
  • handle extended open
  • handle typesubst
  • handle modsubst
  • Experiment with {pexp,ptyp,ppat}_loc_stack (in another PR Ast 408, update with [*_loc_stack]s #867 )
  • Allow module M(_:S) = struct end syntax (will break formatting with 4.07)
  • update CI with OCaml 4.08

@hhugo
Copy link
Copy Markdown
Collaborator Author

hhugo commented May 14, 2019

Some more changes where needed in ocaml-migrate-parsetree. see
ocaml-ppx/ocaml-migrate-parsetree#71

@hhugo hhugo mentioned this pull request May 14, 2019
Closed
@avsm avsm added this to the 0.10 milestone May 14, 2019
@gpetiot gpetiot changed the title [WIP] switch to ast 4.08 [WIP] Improve: switch to ast 4.08 May 16, 2019
hhugo
hhugo commented May 17, 2019
View reviewed changes
@hhugo
Copy link
Copy Markdown
Collaborator Author

hhugo commented May 17, 2019

@Julow, I've updated the description with a maybe non exhaustive todo list.

Feel free to to push to this branch directly.

@Julow
Copy link
Copy Markdown
Collaborator

Julow commented May 17, 2019

I updated your branch. I implemented:

  • Underscore in sugared functor argument
  • Module substitution in signatures
  • Type substitution in signatures
  • Monadic let operators
  • Arbitrary module expression after open in structures and expressions
  • Fix some comments dropped
  • Exceptions in or-patterns

Some requires a lot more testing (eg. let open, comments around new syntaxes).
This is very WIP, the current code must be cleaned before merging.

hhugo
hhugo commented May 20, 2019
View reviewed changes
hhugo
hhugo commented May 22, 2019
View reviewed changes
@hhugo
Copy link
Copy Markdown
Collaborator Author

hhugo commented May 27, 2019

I've implemented some of the missing bullets without much testing.

hhugo
hhugo commented May 27, 2019
View reviewed changes
hhugo
hhugo commented May 27, 2019
View reviewed changes
hhugo
hhugo commented May 27, 2019
View reviewed changes
hhugo
hhugo commented May 27, 2019
View reviewed changes
hhugo
hhugo commented May 29, 2019
View reviewed changes
@gpetiot gpetiot force-pushed the ast-408 branch 2 times, most recently from adf9e62 to b2b1599 Compare May 31, 2019 11:01
This was referenced Jun 2, 2019
Merged
Closed
hhugo
hhugo commented Jun 3, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator Author

@hhugo hhugo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just done another round of review

@hhugo
Copy link
Copy Markdown
Collaborator Author

hhugo commented Jun 5, 2019

The new examples in the testsuite that use the 4.08 syntax makes it harder to check regressions with the 4.07 parser. Is this an issue in practice ? Is there anything we can do apart from splitting examples out ?

@Julow
Copy link
Copy Markdown
Collaborator

Julow commented Jun 6, 2019

I merged hugo's PR, added some tests and rebased on master.

@Julow Julow requested a review from gpetiot June 6, 2019 16:58
@gpetiot
Copy link
Copy Markdown
Collaborator

gpetiot commented Jun 7, 2019

I rebased on the last master, it looks good to me and we can merge as soon as the CI is alright.

hhugo
hhugo commented Jun 7, 2019
View reviewed changes
@gpetiot
Copy link
Copy Markdown
Collaborator

gpetiot commented Jun 12, 2019

Can the CI failure be due to uuseg not being dunified? Looks like uuseg.string is not visible from uuseg.

@Julow
Copy link
Copy Markdown
Collaborator

Julow commented Jun 12, 2019

I think travis tried to merge automatically and got in a weird state. The second build is doing the same commands and is passing.

Julow
Julow reviewed Jun 12, 2019
View reviewed changes
@gpetiot gpetiot changed the title [WIP] Improve: switch to ast 4.08 Improve: switch to ast 4.08 Jun 13, 2019
gpetiot
gpetiot approved these changes Jun 13, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@gpetiot gpetiot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI is finally fixed, and the diff test_branch looks good.
I think it can be pushed now (unless we want to make a release of ocamlformat with 4.07 and another with 4.08 if it makes sense? @jberdine @hhugo @Julow )

@hhugo
Copy link
Copy Markdown
Collaborator Author

hhugo commented Jun 13, 2019

Should we wait for ocaml 4.08 to be officially released ? The current PR makes it impossible to run the testsuite with an older version of the compiler.

@hhugo
Copy link
Copy Markdown
Collaborator Author

hhugo commented Jun 13, 2019

In term of release, when do you expect the next one ? We probably need a release soon after OCaml 4.08 is release (with support for the 4.08 syntax).

@hhugo
Copy link
Copy Markdown
Collaborator Author

hhugo commented Jun 14, 2019

I've tried this branch on js_of_ocaml with both 4.07 and 4.08.
Here is the diff https://github.com/ocsigen/js_of_ocaml/pull/819/files
I found a regression with doc comments on exception https://github.com/ocsigen/js_of_ocaml/pull/819/files#diff-cbc2fa1c5c44a83f4d10d62fc20cb780R692

@hhugo
Copy link
Copy Markdown
Collaborator Author

hhugo commented Jun 14, 2019

4.08 is now officially release. Should we update the CI again :)

@gpetiot gpetiot merged commit 607782b into master Jun 17, 2019
@gpetiot gpetiot deleted the ast-408 branch June 17, 2019 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpetiot gpetiot gpetiot approved these changes

@Julow Julow Julow approved these changes

Assignees

No one assigned

Labels

JaneStreet

Projects

None yet

Milestone

0.10

Development

Successfully merging this pull request may close these issues.

Switch to OCaml 4.08 Ast ..

5 participants

@hhugo @Julow @gpetiot @avsm @facebook-github-bot