-
Notifications
You must be signed in to change notification settings - Fork 88
Parse labeled tuple patterns and types #1523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parse labeled tuple patterns and types #1523
Conversation
4010c19
to
d7c82a9
Compare
4010c19
to
18e55a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good.
I have some minor comments, and I think this PR should probably also update core_type_desc
, since you have working examples with labeled tuple type expressions - see the comment in typetexp.ml
ocaml/testsuite/tests/typing-labeled-tuples/labeledtuples_dsource.ml
Outdated
Show resolved
Hide resolved
a1821b1
to
6317bb5
Compare
6317bb5
to
e848590
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good now, thanks! I pushed a commit that just regenerates the parser, since the last commit changed the mly, and I'll merge once the tests pass.
Summary
Parses labeled tuple patterns using the following temporary syntax. Note the use of punning, punning + annotations, and mixed labeled and non-labeled arguments. Labels are separated from their patterns with
=
, components are separated with;
(desired syntax::
and,
, respectively).Parses labeled tuple types using the following temporary syntax:
Testing
Added testcases to
ocaml/testsuite/tests/typing-labeled-tuples/labeledtuples.ml
.To test
-dsource
, added a testcase toocaml/testsuite/tests/typing-labeled-tuples/labeledtuples_dsource.ml
.Added
CR: labeled tuples
throughout areas that will eventually need more thorough testing.