Express pattern guards using Jane_syntax #1672
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ncik-roberts
reviewed
Aug 8, 2023
ncik-roberts
reviewed
Aug 8, 2023
ncik-roberts
reviewed
Aug 8, 2023
ncik-roberts
reviewed
Aug 9, 2023
ncik-roberts
reviewed
Aug 9, 2023
rajgodse
force-pushed
the
jane-syntax
branch
2 times, most recently
from
9166657
to
1c51765
Compare
|
Builds will fail until #1747 is merged. |
Co-authored-by: Nick Roberts <nroberts02@gmail.com>
Co-authored-by: Nick Roberts <nroberts02@gmail.com>
Co-authored-by: Nick Roberts <nroberts02@gmail.com>
ncik-roberts
approved these changes
Aug 15, 2023
rajgodse
added a commit
to rajgodse/flambda-backend
that referenced
this pull request
Aug 18, 2023
* express pattern guards in jane syntax * fix ocamlprof and flags * update ast_mapper and ast_iterator * add flag for failure on wrong syntactic category * fix parsetree printer * promote seq_bad output * clean up types * fix parsing test * ignore attrs in pretty printer * update field names in ocamlprof * whitespace in seq_bad reference * fix typechecking order of boolean guards * add test for disambiguation from guard * test: move nested match to test functionality * remove attributes from jane_syntax cases * rewrite case jane_syntax to use extension nodes * update jane_syntax_parsing documentation * remove debug print * inline pattern guard generation in parser * fix ocamlprof type * remove bad advice in parsetree comment * comment: give better advice about parsed cases Co-authored-by: Nick Roberts <nroberts02@gmail.com> * reintroduce pc_rhs variants within typecore * fix `Ast_helper.Exp.case` API * remove unnecessary let Co-authored-by: Nick Roberts <nroberts02@gmail.com> * remove unnecessary optional arguments * unabbreviate parsed case rhs in helper * type_approx_function bug * check for jane syntax for uncurried function * delete test that didn't exhibit previous badness * move [fail_if_wrong_syntactic_category] to functor * add test flags * minimize diff from base * fix syntax error post-rebase * add parsetree tests back * update [fail_if_wrong_syntactic_category] comment Co-authored-by: Nick Roberts <nroberts02@gmail.com> * minor wording change to comment * fail if pattern guarded case has pc_guard set * documented Case0 fail_if_wrong_syntactic_category --------- Co-authored-by: Nick Roberts <nroberts02@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changing
Parsetreerequires painful updates to ppx's, so this PR rewrites the pattern guard feature usingJane_syntax.In doing so, it creates a new syntactic category:
Jane_syntax.Case.The attribute marking the extension and the location of a
Parsetree.casewhich represents aJane_syntax.Case.tare stored in its fieldpc_rhs: Parsetree.expression. This rhs should never be interpreted as an expression by itself, which is why everywhere in the code that accesses the rhs of aParsetree.caseshould first applyJane_syntax.Case.of_ast(as documented inparsetree.mli). Ensuring that this invariant is introduced everywhere is one of the main difficulties of this PR.However, the field
pc_rhsis rarely accessed: ripgrepping yields the following files:Of these, only
printast.mldoesn't applyof_astfor cases, but this is fine as it doesn't apply it for any other syntactic category (it just prints the raw Parsetree instead).Currently, there is only one constructor for the variant
Jane_syntax.Case.t, namelyJcase_pattern_guarded of Jane_syntax.Pattern_guarded.case. The payload of this constructor stores the same data asPpattern_guarded_rhsdid previously, together with the case's lhs.It is also worth noting the exact ast representation of
Jane_syntax.Pattern_guarded.Pg_case { scrutinee; cases }iscase : Parsetree.casewithcase.pc_rhs.pexp_desc = Pexp_match (scrutinee, cases), with extension attribute and location stored as described above.