Upgrade Parser to 4.06 AST by anmonteiro · Pull Request #2036 · reasonml/reason

anmonteiro · 2018-06-30T23:45:55Z

This PR upgrades the parser to the 4.06 AST version.

This paves the way to solving a number of issues that are currently blocked because we use the 4.04 version of the AST. For example, this PR also fixes:

Comments at end of line for polymorphic variants are duplicated on refmt. #1828
refmt: "Encountered impossible type specification" error in valid private extensible variant type definition #2302

In addition, this PR introduces parsing / printing of some new syntactic constructs made possible by OCaml 4.06:

open statements in class expressions (e.g. class x = { open M; as self; }; (Support 'let open' in class and class type expressions ocaml/ocaml#1249)
open statements in class types (e.g. class type x = { open M; as 'a; }; (also Support 'let open' in class and class type expressions ocaml/ocaml#1249)
Inherited field in object type (Support inherited field in object type expression ocaml/ocaml#1118). This is the current syntax for inherited fields in object types:

type x = {. a: int};
type y = {
  .
  b: string,
  ...x, /* <-- NEW */
};

anmonteiro · 2018-06-30T23:56:13Z

src/reason-parser/reason_parser.mly

 %on_error_reduce structure_item let_binding_body
                 as_loc(attribute)+
                 type_longident
-                 attribute+


Menhir complains these are unreachable from a toplevel expression. Probably because there's as_loc(attribute)+ above.

anmonteiro · 2018-07-01T03:17:09Z

Tests are now failing on versions of OCaml < 4.06 because I implemented parsing of Pcl_open (e.g. class x = { open M; as self; };.

Not sure what's the proper way to parse these on older versions of OCaml; or is it even supported?

hcarty · 2018-07-01T03:41:58Z

It was only added in 4.06.0. See https://github.com/ocaml/ocaml/blob/trunk/Changes#L801

anmonteiro · 2018-07-01T03:42:43Z

@hcarty I'm aware. I'm trying to see if there's a way to run those tests only against 4.06.0.

anmonteiro · 2018-07-01T03:49:28Z

I can't find a way to do it with the current setup. In any case, after thinking about it, I don't see the value of exercising the OCaml parser, since it's something we don't control. I'll test Pcl_open and the new AST types in .re files.

hcarty · 2018-07-01T04:07:35Z

@anmonteiro I apologize - I misunderstood the question

anmonteiro · 2018-07-01T17:01:26Z

I think this PR is now ready. Code reviews very much appreciated!

IwanKaramazow

Is it possible to add "type checked" tests covering the new ast constructs?

IwanKaramazow · 2018-07-01T18:09:25Z

src/reason-parser/reason_oprint.ml

-        | Ovar_name (id, tyl) ->
-            fprintf ppf "@[%a%a@]" print_typargs tyl print_ident id
+        | Ovar_typ o -> print_out_type ppf o
+            (* fprintf ppf "@[%a%a@]" print_typargs tyl print_ident id *)


Commented out line can probably be omitted?

IwanKaramazow · 2018-07-01T18:14:26Z

src/reason-parser/reason_parser.mly

  | let_bindings SEMI class_expr_lets_and_rest
    { class_of_let_bindings $1 $3 }
  | object_body { mkclass (Pcl_structure $1) }
+  | LET? OPEN override_flag as_loc(mod_longident) SEMI class_expr_lets_and_rest


Is there a reason why we still need to support let open ... vs just open ... in newly added syntax? I tend to favour just open in favour of less grammar complexity. Thoughts?

I added these for consistency with the seq_expr_no_seq production.

IwanKaramazow · 2018-07-01T18:15:56Z

src/reason-parser/reason_parser.mly


+class_sig_body_cty:
+  | class_sig_body { Pcty_signature $1 }
+  | LET? OPEN override_flag as_loc(mod_longident) SEMI as_loc(class_sig_body_cty)


Same remark about let open as above.

IwanKaramazow · 2018-07-01T18:18:53Z

src/reason-parser/reason_pprint_ast.ml

                let pcd_name = {
-                  txt = label;
+                  txt = label.txt;
                  loc = pcd_loc;


This should probably become label.loc? (And finally fixes the comment duplication?)

nice catch.

anmonteiro · 2018-07-01T19:09:11Z

Went through the code review items and:

added typechecked tests for the new AST constructs
fixed Comments at end of line for polymorphic variants are duplicated on refmt. #1828 now that we have locations of each poly variant in the AST (thanks @IwanKaramazow!)

jordwalke · 2018-08-10T07:42:09Z

VERY cool.

anmonteiro · 2020-02-21T19:12:07Z

superseded by #2487

facebook-github-bot added CLA Signed labels Jun 30, 2018

anmonteiro force-pushed the upgrade-parser-to-406 branch 2 times, most recently from fde4aa0 to 7d8b1e9 Compare June 30, 2018 23:54

anmonteiro commented Jun 30, 2018

View reviewed changes

anmonteiro force-pushed the upgrade-parser-to-406 branch from b262010 to f2f01cc Compare July 1, 2018 18:20

IwanKaramazow reviewed Jul 1, 2018

View reviewed changes

anmonteiro force-pushed the upgrade-parser-to-406 branch from 5694900 to 3b34a1e Compare July 1, 2018 18:52

anmonteiro force-pushed the upgrade-parser-to-406 branch from 038161b to 3470c12 Compare July 2, 2018 01:18

anmonteiro force-pushed the upgrade-parser-to-406 branch from 3470c12 to 12aba0b Compare October 11, 2018 19:01

anmonteiro mentioned this pull request Dec 26, 2018

refmt: "Encountered impossible type specification" error in valid private extensible variant type definition #2302

Closed

anmonteiro force-pushed the upgrade-parser-to-406 branch from 12aba0b to c9dabb1 Compare April 2, 2019 11:02

cem2ran mentioned this pull request Sep 13, 2019

fix(WebView): event types rescript-react-native/rescript-react-native#608

Merged

anmonteiro added 9 commits November 3, 2019 10:47

Upgrade Parser to 4.06 AST

366d49e

fix some todos

b06bb1a

parse and print Pcl_open

c45fb97

fix tests for Pcl_open

a242677

parse and print Ptcy_open

f070a22

parse and print Oinherit

64a720a

code review items

f9ddd02

fix reasonml#1828

5dcc60c

punning needs LIDENT, not poly_type

05a9299

anmonteiro added 5 commits November 3, 2019 10:50

fix after rebase

2fd0069

fix building after rebase

3590bee

Also fix reasonml#2302

2a5b970

move tests to 4.06 specific features

b0e7e6b

simpler oprint

2b43907

anmonteiro force-pushed the upgrade-parser-to-406 branch from 6a875d7 to 2b43907 Compare November 3, 2019 19:04

fix tests after rebase

fa71b54

anmonteiro force-pushed the upgrade-parser-to-406 branch from 8cb794f to fa71b54 Compare November 3, 2019 19:15

anmonteiro mentioned this pull request Nov 4, 2019

Upgrade Parser to 4.08 AST and add 4.08 features #2487

Merged

anmonteiro mentioned this pull request Nov 21, 2019

Raise minimum OCaml AST support #2496

Closed

anmonteiro closed this Feb 21, 2020

Conversation

anmonteiro commented Jun 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anmonteiro commented Jul 1, 2018

Uh oh!

hcarty commented Jul 1, 2018

Uh oh!

anmonteiro commented Jul 1, 2018

Uh oh!

anmonteiro commented Jul 1, 2018

Uh oh!

hcarty commented Jul 1, 2018

Uh oh!

anmonteiro commented Jul 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IwanKaramazow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anmonteiro commented Jul 1, 2018

Uh oh!

jordwalke commented Aug 10, 2018

Uh oh!

anmonteiro commented Feb 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

anmonteiro commented Jun 30, 2018 •

edited

Loading

anmonteiro commented Jul 1, 2018 •

edited

Loading