Add support for new local syntax #73

mdelvecchio-jsc · 2024-06-05T18:58:36Z

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

lib/Fmt_ast.ml

mdelvecchio-jsc · 2024-06-06T18:54:26Z

lib/Fmt_ast.ml

-                                  $ fmt_pattern c lb_pat )
-                              $ fmt_if_k
-                                  (not (List.is_empty lb_args))
+                                  $ wrap_if (has_args && has_modes) "(" ")"


Consider refactoring.

mdelvecchio-jsc · 2024-06-06T18:58:39Z

lib/Sugar.ml

-      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 *)


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

mdelvecchio-jsc · 2024-06-06T19:47:24Z

vendor/parser-extended/ast_mapper.ml


 (* A generic Parsetree mapping class *)

-(*


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.

mdelvecchio-jsc · 2024-06-24T15:09:21Z

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.

mdelvecchio-jsc · 2024-06-24T15:10:14Z

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.

test/passing/tests/modes_cmts_move.ml

mdelvecchio-jsc · 2024-06-24T16:33:54Z

Consider whether PR 2706 to flambda affects this PR.

freemagma · 2024-06-24T18:04:42Z

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

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>

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>

mdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch 3 times, most recently from b8c3f29 to 5ecc704 Compare June 6, 2024 16:49

mdelvecchio-jsc commented Jun 6, 2024

View reviewed changes

lib/Fmt_ast.ml Show resolved Hide resolved

mdelvecchio-jsc commented Jun 6, 2024

View reviewed changes

lib/Fmt_ast.ml Show resolved Hide resolved

mdelvecchio-jsc commented Jun 6, 2024

View reviewed changes

mdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch from 4fae337 to ccbfecb Compare June 12, 2024 16:04

mdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch 2 times, most recently from 4bbb7ec to ae48b5e Compare June 20, 2024 20:46

mdelvecchio-jsc marked this pull request as ready for review June 20, 2024 20:48

mdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch 3 times, most recently from b81a8f3 to edb9181 Compare June 21, 2024 21:18

mdelvecchio-jsc requested a review from freemagma June 24, 2024 15:10

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

freemagma reviewed Jun 24, 2024

View reviewed changes

test/passing/tests/modes_cmts_move.ml Show resolved Hide resolved

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

freemagma reviewed Jun 24, 2024

View reviewed changes

lib/Fmt_ast.ml Outdated Show resolved Hide resolved

mdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch from 4e8abc5 to 2526cf1 Compare June 26, 2024 17:18

freemagma approved these changes Jun 26, 2024

View reviewed changes

mdelvecchio-jsc force-pushed the tdelvecchio-new-local-syntax branch from 10ef9f6 to f396402 Compare June 27, 2024 15:53

mdelvecchio-jsc merged commit 8a2110a into jane Jun 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for new local syntax #73

Add support for new local syntax #73

Uh oh!

mdelvecchio-jsc commented Jun 5, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

mdelvecchio-jsc Jun 6, 2024

Uh oh!

mdelvecchio-jsc Jun 6, 2024

Uh oh!

mdelvecchio-jsc Jun 6, 2024

Uh oh!

mdelvecchio-jsc commented Jun 24, 2024

Uh oh!

mdelvecchio-jsc commented Jun 24, 2024 •

edited

Loading

Uh oh!

Uh oh!

mdelvecchio-jsc commented Jun 24, 2024

Uh oh!

freemagma commented Jun 24, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add support for new local syntax #73

Add support for new local syntax #73

Uh oh!

Conversation

mdelvecchio-jsc commented Jun 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdelvecchio-jsc Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

mdelvecchio-jsc Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

mdelvecchio-jsc Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

mdelvecchio-jsc commented Jun 24, 2024

Uh oh!

mdelvecchio-jsc commented Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mdelvecchio-jsc commented Jun 24, 2024

Uh oh!

freemagma commented Jun 24, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mdelvecchio-jsc commented Jun 5, 2024 •

edited

Loading

mdelvecchio-jsc commented Jun 24, 2024 •

edited

Loading