Skip to content

Commit 5cc318b

Browse files
authored
generate better asts for function bindings (#2651)
Currently: ```ocaml let foo1 : _ = function () -> () let foo2 x : _ = function () -> () ``` are parsed into these value_bindings: ```ocaml (* foo1 *) { pvb_args = [] ; pvb_constraint = Some "_" ; pvb_body = Pfunction_cases ... } (* foo2 *) { pvb_args = ["x"] ; pvb_constraint = Some "_" ; pvb_body = Pfunction_cases ... } ``` I expect instead: ```ocaml (* foo1 *) { pvb_args = [] ; pvb_constraint = Some "_" ; pvb_body = Pfunction_body (Pexp_function ([], None, Pfunction_cases ...)) } (* foo2 (no changes here) *) { pvb_args = ["x"] ; pvb_constraint = Some "_" ; pvb_body = Pfunction_cases ... } ``` I think the ast for foo1: - is confusing - creates a needless distinction between `let f : _ = function () -> ()` vs `let f : _ = (function () -> ())`, unlike, say, `1 + function () -> ()` vs `1 + (function () -> ())`. - is essentially an invariant violation. The type of value_bindings in ocamlformat should be understood to be the union of a non-function let-binding + an inline pexp_function node. But the node for foo1 corresponds to the syntax of neither a non-function let-binding (because of body = Pfunction_cases _), nor an inline pexp_function (because pexp_function can't have a type_constraint with an empty list of params).
1 parent 2a071f8 commit 5cc318b

File tree

7 files changed

+26
-21
lines changed

7 files changed

+26
-21
lines changed

Diff for: CHANGES.md

+3
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ profile. This started with version 0.26.0.
1414

1515
- Fixed `nested-match=align` not working with `match%ext` (#2648, @EmileTrotignon)
1616

17+
- Fixed the AST generated for bindings of the form `let pattern : type = function ...`
18+
(#2651, @v-gb)
19+
1720
## 0.27.0
1821

1922
### Highlight

Diff for: lib/Fmt_ast.ml

+9-3
Original file line numberDiff line numberDiff line change
@@ -4586,9 +4586,15 @@ and fmt_value_binding c ~ctx0 ~rec_flag ?in_ ?epi
45864586
in
45874587
let fmt_newtypes, fmt_cstr = fmt_value_constraint c lb_typ in
45884588
let indent, intro_as_pro =
4589-
match lb_body.ast with
4590-
| Pfunction_cases _ -> (c.conf.fmt_opts.function_indent.v, true)
4591-
| Pfunction_body {pexp_desc= Pexp_function (_, _, _); _}
4589+
match (lb_args, lb_body.ast) with
4590+
| _, Pfunction_cases _
4591+
|( []
4592+
, Pfunction_body
4593+
{ pexp_attributes= []
4594+
; pexp_desc= Pexp_function ([], None, Pfunction_cases _)
4595+
; _ } ) ->
4596+
(c.conf.fmt_opts.function_indent.v, true)
4597+
| _, Pfunction_body {pexp_desc= Pexp_function (_, _, _); _}
45924598
when c.conf.fmt_opts.let_binding_deindent_fun.v ->
45934599
(max (c.conf.fmt_opts.let_binding_indent.v - 1) 0, false)
45944600
| _ -> (c.conf.fmt_opts.let_binding_indent.v, false)

Diff for: test/passing/refs.janestreet/let_binding-deindent-fun.ml.ref

+1-2
Original file line numberDiff line numberDiff line change
@@ -306,8 +306,7 @@ fun xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx : _ ->
306306
match () with
307307
| _ -> ()
308308

309-
let _
310-
=
309+
let _ =
311310
(*
312311
An alternative would be to track 'mutability of the field'
313312
directly.

Diff for: test/passing/refs.janestreet/let_binding-in_indent.ml.ref

+1-2
Original file line numberDiff line numberDiff line change
@@ -306,8 +306,7 @@ fun xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx : _ ->
306306
match () with
307307
| _ -> ()
308308

309-
let _
310-
=
309+
let _ =
311310
(*
312311
An alternative would be to track 'mutability of the field'
313312
directly.

Diff for: test/passing/refs.janestreet/let_binding-indent.ml.ref

+1-2
Original file line numberDiff line numberDiff line change
@@ -306,8 +306,7 @@ fun xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx : _ ->
306306
match () with
307307
| _ -> ()
308308

309-
let _
310-
=
309+
let _ =
311310
(*
312311
An alternative would be to track 'mutability of the field'
313312
directly.

Diff for: test/passing/refs.janestreet/let_binding.ml.ref

+1-2
Original file line numberDiff line numberDiff line change
@@ -306,8 +306,7 @@ fun xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx : _ ->
306306
match () with
307307
| _ -> ()
308308

309-
let _
310-
=
309+
let _ =
311310
(*
312311
An alternative would be to track 'mutability of the field'
313312
directly.

Diff for: vendor/parser-extended/parser.mly

+10-10
Original file line numberDiff line numberDiff line change
@@ -2619,30 +2619,30 @@ let_binding_body_no_punning:
26192619
let_ident strict_binding(fun_body)
26202620
{ let args, tc, exp = $2 in
26212621
($1, args, tc, exp) }
2622-
| let_ident type_constraint EQUAL fun_body
2622+
| let_ident type_constraint EQUAL seq_expr
26232623
{ let v = $1 in (* PR#7344 *)
26242624
let t =
26252625
match $2 with
26262626
Pconstraint t ->
26272627
Pvc_constraint { locally_abstract_univars = []; typ=t }
26282628
| Pcoerce (ground, coercion) -> Pvc_coercion { ground; coercion}
26292629
in
2630-
(v, [], Some t, $4)
2630+
(v, [], Some t, Pfunction_body $4)
26312631
}
2632-
| let_ident COLON poly(core_type) EQUAL fun_body
2632+
| let_ident COLON poly(core_type) EQUAL seq_expr
26332633
{
26342634
let t = ghtyp ~loc:($loc($3)) $3 in
2635-
($1, [], Some (Pvc_constraint { locally_abstract_univars = []; typ=t }), $5)
2635+
($1, [], Some (Pvc_constraint { locally_abstract_univars = []; typ=t }), Pfunction_body $5)
26362636
}
2637-
| let_ident COLON TYPE lident_list DOT core_type EQUAL fun_body
2637+
| let_ident COLON TYPE lident_list DOT core_type EQUAL seq_expr
26382638
{ let constraint' =
26392639
Pvc_constraint { locally_abstract_univars=$4; typ = $6}
26402640
in
2641-
($1, [], Some constraint', $8) }
2642-
| pattern_no_exn EQUAL fun_body
2643-
{ ($1, [], None, $3) }
2644-
| simple_pattern_not_ident COLON core_type EQUAL fun_body
2645-
{ ($1, [], Some(Pvc_constraint { locally_abstract_univars=[]; typ=$3 }), $5) }
2641+
($1, [], Some constraint', Pfunction_body $8) }
2642+
| pattern_no_exn EQUAL seq_expr
2643+
{ ($1, [], None, Pfunction_body $3) }
2644+
| simple_pattern_not_ident COLON core_type EQUAL seq_expr
2645+
{ ($1, [], Some(Pvc_constraint { locally_abstract_univars=[]; typ=$3 }), Pfunction_body $5) }
26462646
;
26472647
let_binding_body:
26482648
| let_binding_body_no_punning

0 commit comments

Comments
 (0)