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

Add support for new local syntax #73

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

tdelvecchio-jsc
Copy link

@tdelvecchio-jsc tdelvecchio-jsc commented Jun 5, 2024

This adds support for modes and modalities in the following positions:

  • Let bindings:
    let x @ mode = y
    let x : typ @@ mode = y
  • Expressions:
    let _ = (x : typ @@ mode)
  • Arrow params:
    type t = typ @ mode -> lbl:typ @ mode -> typ @ mode
  • Modalities on record fields:
    type t = { x : typ @@ modality }
  • Modalities on constructor arguments:
    type t = A of typ @@ modality * typ @@ modality
  • Value descriptions:
    val x : typ @@ mode
  • Patterns:
    let ((x @ mode), (y @ mode)) = z
  • Let-bound functions:
    let (f @ mode) (x @ mode) (y @ mode) = z

@tdelvecchio-jsc tdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch 3 times, most recently from b8c3f29 to 5ecc704 Compare June 6, 2024 16:49
$ fmt_pattern c lb_pat )
$ fmt_if_k
(not (List.is_empty lb_args))
$ wrap_if (has_args && has_modes) "(" ")"
Copy link
Author

Choose a reason for hiding this comment

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

Consider refactoring.

match (lb_pat.ast.ppat_desc, lb_exp.ast.pexp_desc) with
(* recognize and undo the pattern of code introduced by
ocaml/ocaml@fd0dc6a0fbf73323c37a73ea7e8ffc150059d6ff to fix
https://caml.inria.fr/mantis/view.php?id=7344 *)
Copy link
Author

Choose a reason for hiding this comment

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

This desugaring no longer happens; the constraint is now encoded in pvb_constraint.

@@ -15,10 +15,8 @@

(* A generic Parsetree mapping class *)

(*
Copy link
Author

Choose a reason for hiding this comment

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

This warning being commented out was the pain source of a bug for a couple hours. I don't wee why it's commented out when there's only like 2 places below that needed to be changed to make the warning pass.

@tdelvecchio-jsc tdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch 2 times, most recently from 4bbb7ec to ae48b5e Compare June 20, 2024 20:46
@tdelvecchio-jsc tdelvecchio-jsc marked this pull request as ready for review June 20, 2024 20:48
@tdelvecchio-jsc tdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch 3 times, most recently from b81a8f3 to edb9181 Compare June 21, 2024 21:18
@tdelvecchio-jsc
Copy link
Author

Tested by smashing tree to add mode annotations everywhere possible in jane, and running ocamlformat on entire tree. Obviously could have missed adding annotations in some places, and the annotations are added in a relatively uniform way, but this did successfully reveal a few bugs.

@tdelvecchio-jsc
Copy link
Author

tdelvecchio-jsc commented Jun 24, 2024

Currently, CI is failing on some tests unrelated to this change. I'm looking into this, but I don't think it should block review.

Update: Resolved.

@tdelvecchio-jsc tdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch 2 times, most recently from 7aed70f to 107d95c Compare June 24, 2024 16:29
@tdelvecchio-jsc
Copy link
Author

Consider whether PR 2706 to flambda affects this PR.

@tdelvecchio-jsc tdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch 2 times, most recently from 52d931d to b20bc81 Compare June 24, 2024 17:18
@freemagma
Copy link

Consider whether PR 2706 to flambda affects this PR.

It shouldn't. The parser-extended changes I made here were based on a compiler branch that was a superset of that PR

lib/Fmt_ast.ml Outdated Show resolved Hide resolved
Signed-off-by: Charlie Gunn <cgunn@janestreet.com>
Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Remove code to recognize ast pattern that no longer exists.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Support modes on arrow types.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Update normalize mapper to prevent bad sugaring.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix ast mapper to not drop modes and modalities in various places.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Support modes on value bindings.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add support for modes in pattern constraints and expression constraints.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Support modalities on record declarations.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Support modalities on value declarations.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix test; in [let (pat @ mode) = exp], the mode is actually attached to the value binding, not the pattern.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests for comments.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix moving comment in record fields.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests and make minor changes for formatting with line breaks.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Test in conjunction with old syntax.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix formatting of tuple patterns where els have modes.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add labeled tuple pattern tests.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix labeled tuple pattern punning with modes.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix invalid punning of labeled tuple expressions with modes.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests for attributes.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix issue with comments after [@] and [@@].

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix modes on let-bound tuple patterns.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix parens around aliased patterns with modes

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix tuple patterns with local exprs.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix let class expressions.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Move tests of modes on patterns to failing.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix incorrect dependencies in test causing CI failure.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Rework logic for handling comments after types in label declarations.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Extend tests of label declarations.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests for --break-separators=after.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>
@tdelvecchio-jsc tdelvecchio-jsc merged commit 8a2110a into jane Jun 27, 2024
8 checks passed
tdelvecchio-jsc pushed a commit that referenced this pull request Jun 27, 2024
Parse new local syntax, but ignore when formatting.

Signed-off-by: Charlie Gunn <cgunn@janestreet.com>
Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Remove code to recognize ast pattern that no longer exists.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Support modes on arrow types.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Update normalize mapper to prevent bad sugaring.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix ast mapper to not drop modes and modalities in various places.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Support modes on value bindings.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add support for modes in pattern constraints and expression constraints.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Support modalities on record declarations.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Support modalities on value declarations.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix test; in [let (pat @ mode) = exp], the mode is actually attached to the value binding, not the pattern.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests for comments.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix moving comment in record fields.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests and make minor changes for formatting with line breaks.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Test in conjunction with old syntax.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix formatting of tuple patterns where els have modes.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add labeled tuple pattern tests.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix labeled tuple pattern punning with modes.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix invalid punning of labeled tuple expressions with modes.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests for attributes.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix issue with comments after [@] and [@@].

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix modes on let-bound tuple patterns.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix parens around aliased patterns with modes

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix tuple patterns with local exprs.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix let class expressions.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Move tests of modes on patterns to failing.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Fix incorrect dependencies in test causing CI failure.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Rework logic for handling comments after types in label declarations.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Extend tests of label declarations.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>

Add tests for --break-separators=after.

Signed-off-by: Thomas Del Vecchio <tdelvecchio@janestreet.com>
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