Skip to content
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

Multiway match typechecking and translation for pattern guards #2

Merged
merged 15 commits into from
Aug 2, 2023

Conversation

rajgodse
Copy link
Owner

Implements typechecking and translation for pattern guards.

The changes are mostly the straightforward generalization of the single case pattern guard construct.

The main complexity comes from changes related to the restructuring of Typedtree.case, whose field c_rhs used to be of type expression, but is now of type case_rhs, in conjunction with the way that multiway pattern guards change the notion of the rhs of a case.

The changes in ocamldoc are type-directed, but it is not apparent that they exhibit the intended behavior. It is worth trying to figure out the intended behavior of the modified functions through review.

Copy link
Collaborator

@ncik-roberts ncik-roberts left a comment

Choose a reason for hiding this comment

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

The translcore changes are trickier than I thought, but they still seem fine. I left a few mostly-minor comments.

ocaml/ocamldoc/odoc_ast.ml Outdated Show resolved Hide resolved
ocaml/ocamldoc/odoc_ast.ml Outdated Show resolved Hide resolved
ocaml/testsuite/tests/pattern-guards/test.ml Outdated Show resolved Hide resolved
ocaml/typing/parmatch.ml Show resolved Hide resolved
ocaml/typing/parmatch.ml Outdated Show resolved Hide resolved
ocaml/lambda/translcore.ml Outdated Show resolved Hide resolved
ocaml/lambda/translcore.ml Outdated Show resolved Hide resolved
ocaml/lambda/translcore.ml Outdated Show resolved Hide resolved
ocaml/lambda/translcore.ml Show resolved Hide resolved
ocaml/testsuite/tests/pattern-guards/test.ml Outdated Show resolved Hide resolved
@rajgodse
Copy link
Owner Author

As discussed, rebased onto test changes in other PRs to consolidate tests before restructuring and splitting test cases into different test files.

@rajgodse
Copy link
Owner Author

Rebase onto new parsing

Copy link
Collaborator

@ncik-roberts ncik-roberts left a comment

Choose a reason for hiding this comment

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

A few fairly minor suggestions.

ocaml/ocamldoc/odoc_ast.ml Outdated Show resolved Hide resolved
ocaml/ocamldoc/odoc_ast.ml Outdated Show resolved Hide resolved
ocaml/ocamldoc/odoc_ast.ml Outdated Show resolved Hide resolved
ocaml/testsuite/tests/pattern-guards/nested_guards.ml Outdated Show resolved Hide resolved
Copy link
Collaborator

@ncik-roberts ncik-roberts left a comment

Choose a reason for hiding this comment

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

I think this will be good to merge after it's rebased and any conflicts are resolved. As discussed, we'll need to open a new PR from multiway-match-parsing into the flambda-backend repo even after this is merged.

@rajgodse rajgodse force-pushed the multiway-match-typechecking branch from bc3dd1b to 3f8864d Compare August 2, 2023 22:20
@rajgodse rajgodse merged commit 278f127 into multiway-match-parsing Aug 2, 2023
34 checks passed
rajgodse added a commit that referenced this pull request Aug 2, 2023
* multiway typechecking and translation

* update jane test output

* self-review: format and style in translcore

* more translcore/typedtree cleanup

* expose value `is_guarded_rhs`

* fix typedtree printer

* make discussed changes to ocamldoc

* format: remove unnecessary parens in pattern

Co-authored-by: Nick Roberts <[email protected]>

* improve parmatch variable naming

* explain [exp_attributes] and [exp_extra] weirdness

* improve translcore [event_function*] naming

* inlined transl_body in transl_rhs

* rename pats_exp... to use "rhs" naming

* added test for guarded value/exception or-patterns

* address ocamldoc CRs

---------

Co-authored-by: Nick Roberts <[email protected]>
rajgodse added a commit that referenced this pull request Aug 3, 2023
Multiway match typechecking and translation for pattern guards (#2)

* multiway typechecking and translation

* update jane test output

* self-review: format and style in translcore

* more translcore/typedtree cleanup

* expose value `is_guarded_rhs`

* fix typedtree printer

* make discussed changes to ocamldoc

* format: remove unnecessary parens in pattern



* improve parmatch variable naming

* explain [exp_attributes] and [exp_extra] weirdness

* improve translcore [event_function*] naming

* inlined transl_body in transl_rhs

* rename pats_exp... to use "rhs" naming

* added test for guarded value/exception or-patterns

* address ocamldoc CRs

---------

Co-authored-by: Nick Roberts <[email protected]>
rajgodse added a commit that referenced this pull request Aug 18, 2023
Multiway match typechecking and translation for pattern guards (#2)

* multiway typechecking and translation

* update jane test output

* self-review: format and style in translcore

* more translcore/typedtree cleanup

* expose value `is_guarded_rhs`

* fix typedtree printer

* make discussed changes to ocamldoc

* format: remove unnecessary parens in pattern

* improve parmatch variable naming

* explain [exp_attributes] and [exp_extra] weirdness

* improve translcore [event_function*] naming

* inlined transl_body in transl_rhs

* rename pats_exp... to use "rhs" naming

* added test for guarded value/exception or-patterns

* address ocamldoc CRs

---------

Co-authored-by: Nick Roberts <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants