Skip to content

Commit 81a6608

Browse files
committed
Preserve constructor compatibility without parser modes
Apply the normalization approach proposed by @cristianoc in PR rescript-lang#8610. Keep source argument lists for printing and resolve semantic grouping after constructor disambiguation, without legacy PPX marker handling in the type checker or printer. Signed-off-by: Christoph Knittel <ck@cca.io>
1 parent 3557f92 commit 81a6608

19 files changed

Lines changed: 250 additions & 152 deletions

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
#### :boom: Breaking Change
1616

17-
- Distinguish multiple constructor arguments from a tuple passed as a single argument. Constructors with one tuple payload must now use nested parentheses, for example `Some((x, y))`; `Some(x, y)` now reports an arity mismatch. This makes constructor arity explicit in the parsetree and removes the separate parser modes for printing and type checking. https://github.com/rescript-lang/rescript/pull/8610
1817
- Reject malformed UTF-8 in documentation comments and invalid string or template literal escapes that were previously accepted, including empty or out-of-range braced Unicode escapes (`\u{}`, `\u{110000}`) and legacy decimal or octal escapes in templates (`\1`, `\01`, `\8`). These inputs now produce syntax diagnostics instead of compiling to invalid or inconsistent JavaScript. https://github.com/rescript-lang/rescript/pull/8606
1918
- Reject tagged template literals in patterns. Patterns cannot invoke their tag; previously their raw payload was compiled as a plain string comparison. https://github.com/rescript-lang/rescript/pull/8606
2019
- Remove runtime APIs that were deprecated for removal in ReScript 13, including the `Char` module, unsafe `Obj` operations, legacy `Pervasives` helpers, and `Array.unsafe_get`. https://github.com/rescript-lang/rescript/pull/8564
@@ -69,6 +68,7 @@
6968

7069
#### :house: Internal
7170

71+
- Remove separate parser modes for printing and type checking by preserving syntactic constructor arguments in the parsetree and resolving their semantic grouping during type checking. Existing constructor spellings and legacy PPX output remain supported. https://github.com/rescript-lang/rescript/pull/8610
7272
- Merge the duplicate Lam intermediate representation into Lambda, removing the conversion layer and obsolete supporting infrastructure. Lambda is now a single private, normalized representation, with generated JavaScript remaining semantically unchanged. https://github.com/rescript-lang/rescript/pull/8608
7373
- Add genType and source map controls and output to the developer playground. https://github.com/rescript-lang/rescript/pull/8448
7474
- Rework the object-type representation end to end: object rows are plain field chains carrying a per-field mutability state (no phantom setter members), object literals are typed directly and property access and assignment are first-class AST and Lambda nodes shared between the Lambda and JS pipelines, and dead class-system remnants (the field-presence lattice, the class-abbreviation memo on object types, method-send typing) are removed. https://github.com/rescript-lang/rescript/pull/8597

compiler/ml/ast_mapper_from0.ml

Lines changed: 18 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@ let map_loc sub {loc; txt} = {loc = sub.location sub loc; txt}
166166
let record_rest_attr_name = "_res.record_rest"
167167
let constructor_args_attr_name = "_res.constructor_args"
168168
let constructor_tuple_arg_attr_name = "_res.constructor_tuple_arg"
169-
let legacy_constructor_payload_attr_name = "_res.legacy_constructor_payload"
170169

171170
let has_explicit_arity_attr (attrs : Pt.attributes) =
172171
List.exists
@@ -190,17 +189,14 @@ let remove_constructor_args_attr attrs =
190189
let remove_constructor_tuple_arg_attr attrs =
191190
remove_internal_marker_attr ~name:constructor_tuple_arg_attr_name attrs
192191

193-
let add_legacy_constructor_payload_attr attrs =
194-
(Location.mknoloc legacy_constructor_payload_attr_name, Pt.PStr []) :: attrs
195-
196-
let decode_args ~map ~tuple_args ~split_tuple ~known_tuple_arg = function
197-
| None -> ([], false)
192+
(* Unmarked v0 tuples remain a single syntactic payload. Typecore resolves
193+
semantic argument grouping after it knows the constructor declaration. *)
194+
let decode_args ~map ~tuple_args ~split_tuple = function
195+
| None -> []
198196
| Some arg -> (
199197
match tuple_args arg with
200-
| Some args when split_tuple -> (List.map map args, false)
201-
| Some _ when known_tuple_arg -> ([map arg], false)
202-
| Some _ -> ([map arg], true)
203-
| None -> ([map arg], false))
198+
| Some args when split_tuple -> List.map map args
199+
| _ -> [map arg])
204200

205201
let record_rest_of_pattern (rest : Pt.pattern) =
206202
match rest.Pt.ppat_desc with
@@ -887,10 +883,8 @@ module E = struct
887883
| Pexp_construct (lid, arg) -> (
888884
let lid1 = map_loc sub lid in
889885
let has_constructor_args, attrs = remove_constructor_args_attr attrs in
890-
let has_constructor_tuple_arg, attrs =
891-
remove_constructor_tuple_arg_attr attrs
892-
in
893-
let args, has_legacy_constructor_payload =
886+
let _, attrs = remove_constructor_tuple_arg_attr attrs in
887+
let args =
894888
decode_args ~map:(sub.expr sub)
895889
~tuple_args:(fun arg ->
896890
match arg.pexp_desc with
@@ -900,12 +894,7 @@ module E = struct
900894
(has_constructor_args
901895
|| has_explicit_arity_attr attrs
902896
|| lid.txt = Longident.Lident "::")
903-
~known_tuple_arg:has_constructor_tuple_arg arg
904-
in
905-
let attrs =
906-
if has_legacy_constructor_payload then
907-
add_legacy_constructor_payload_attr attrs
908-
else attrs
897+
arg
909898
in
910899
let exp1 = construct ~loc ~attrs lid1 args in
911900
match lid.txt with
@@ -974,17 +963,14 @@ module E = struct
974963
:: attrs
975964
| _ -> attrs
976965
in
977-
let has_constructor_tuple_arg, attrs =
978-
remove_constructor_tuple_arg_attr attrs
979-
in
980-
let args, _ =
966+
let _, attrs = remove_constructor_tuple_arg_attr attrs in
967+
let args =
981968
decode_args ~map:(sub.expr sub)
982969
~tuple_args:(fun arg ->
983970
match arg.pexp_desc with
984971
| Pexp_tuple args -> Some args
985972
| _ -> None)
986-
~split_tuple:has_constructor_args
987-
~known_tuple_arg:has_constructor_tuple_arg arg
973+
~split_tuple:has_constructor_args arg
988974
in
989975
variant ~loc ~attrs lab args
990976
| Pexp_record (l, eo) ->
@@ -1155,10 +1141,8 @@ module P = struct
11551141
| Ppat_tuple pl -> tuple ~loc ~attrs (List.map (sub.pat sub) pl)
11561142
| Ppat_construct (l, arg) ->
11571143
let has_constructor_args, attrs = remove_constructor_args_attr attrs in
1158-
let has_constructor_tuple_arg, attrs =
1159-
remove_constructor_tuple_arg_attr attrs
1160-
in
1161-
let args, has_legacy_constructor_payload =
1144+
let _, attrs = remove_constructor_tuple_arg_attr attrs in
1145+
let args =
11621146
decode_args ~map:(sub.pat sub)
11631147
~tuple_args:(fun arg ->
11641148
match arg.ppat_desc with
@@ -1168,12 +1152,7 @@ module P = struct
11681152
(has_constructor_args
11691153
|| has_explicit_arity_attr attrs
11701154
|| l.txt = Longident.Lident "::")
1171-
~known_tuple_arg:has_constructor_tuple_arg arg
1172-
in
1173-
let attrs =
1174-
if has_legacy_constructor_payload then
1175-
add_legacy_constructor_payload_attr attrs
1176-
else attrs
1155+
arg
11771156
in
11781157
construct ~loc ~attrs (map_loc sub l) args
11791158
| Ppat_variant (l, arg) ->
@@ -1186,17 +1165,14 @@ module P = struct
11861165
:: attrs
11871166
| _ -> attrs
11881167
in
1189-
let has_constructor_tuple_arg, attrs =
1190-
remove_constructor_tuple_arg_attr attrs
1191-
in
1192-
let args, _ =
1168+
let _, attrs = remove_constructor_tuple_arg_attr attrs in
1169+
let args =
11931170
decode_args ~map:(sub.pat sub)
11941171
~tuple_args:(fun arg ->
11951172
match arg.ppat_desc with
11961173
| Ppat_tuple args -> Some args
11971174
| _ -> None)
1198-
~split_tuple:has_constructor_args
1199-
~known_tuple_arg:has_constructor_tuple_arg arg
1175+
~split_tuple:has_constructor_args arg
12001176
in
12011177
variant ~loc ~attrs l args
12021178
| Ppat_record (lpl, cf) ->

compiler/ml/parsetree.ml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,9 @@ and pattern_desc =
220220
C(P) [P]
221221
C(P1, ..., Pn) [P1; ...; Pn]
222222
C((P1, ..., Pn)) [Ppat_tuple [P1; ...; Pn]]
223+
224+
This list preserves syntax, not the declared constructor arity.
225+
Type checking normalizes tuple grouping using the resolved constructor.
223226
*)
224227
| Ppat_variant of label * pattern list
225228
(* #A []
@@ -312,6 +315,9 @@ and expression_desc =
312315
C(E) [E]
313316
C(E1, ..., En) [E1; ...; En]
314317
C((E1, ..., En)) [Pexp_tuple [E1; ...; En]]
318+
319+
This list preserves syntax, not the declared constructor arity.
320+
Type checking normalizes tuple grouping using the resolved constructor.
315321
*)
316322
| Pexp_variant of label * expression list
317323
(* #A []

compiler/ml/typecore.ml

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,23 +1212,32 @@ type type_pat_mode =
12121212
12131213
exception Need_backtrack
12141214
1215+
(* The parser preserves syntactic arguments for printing. Resolve their semantic
1216+
grouping only after constructor disambiguation, retaining the historical
1217+
equivalence of C(a, b) and C((a, b)), including for legacy PPX output. *)
1218+
let constructor_args_of_exp_payload ~arity sargs =
1219+
match sargs with
1220+
| [{pexp_desc = Pexp_tuple args}] when arity > 1 -> args
1221+
| {pexp_loc = first_loc} :: (_ :: _ as rest) when arity = 1 ->
1222+
let last = Ext_list.last rest in
1223+
let loc = Location.{first_loc with loc_end = last.pexp_loc.loc_end} in
1224+
[Ast_helper.Exp.tuple ~loc sargs]
1225+
| sargs -> sargs
1226+
1227+
let constructor_args_of_pat_payload ~arity sargs =
1228+
match sargs with
1229+
| [{ppat_desc = Ppat_tuple args}] when arity > 1 -> args
1230+
| {ppat_loc = first_loc} :: (_ :: _ as rest) when arity = 1 ->
1231+
let last = Ext_list.last rest in
1232+
let loc = Location.{first_loc with loc_end = last.ppat_loc.loc_end} in
1233+
[Ast_helper.Pat.tuple ~loc sargs]
1234+
| sargs -> sargs
1235+
12151236
(* type_pat propagates the expected type as well as maps for
12161237
constructors and labels.
12171238
Unification may update the typing environment. *)
12181239
(* constrs <> None => called from parmatch: backtrack on or-patterns
12191240
explode > 0 => explode Ppat_any for gadts *)
1220-
let legacy_constructor_payload_attr_name = "_res.legacy_constructor_payload"
1221-
1222-
let remove_legacy_constructor_payload_attr attrs =
1223-
let rec loop rev_attrs = function
1224-
| ({Location.txt; _}, PStr []) :: attrs
1225-
when txt = legacy_constructor_payload_attr_name ->
1226-
(true, List.rev_append rev_attrs attrs)
1227-
| attr :: attrs -> loop (attr :: rev_attrs) attrs
1228-
| [] -> (false, List.rev rev_attrs)
1229-
in
1230-
loop [] attrs
1231-
12321241
let rec type_pat ~constrs ~labels ~no_existentials ~mode ~explode ~env sp
12331242
expected_ty k =
12341243
Builtin_attributes.warning_scope sp.ppat_attributes (fun () ->
@@ -1402,10 +1411,6 @@ and type_pat_aux ~constrs ~labels ~no_existentials ~mode ~explode ~env sp
14021411
pat_env = !env;
14031412
})
14041413
| Ppat_construct (lid, sargs) ->
1405-
let has_legacy_constructor_payload, ppat_attributes =
1406-
remove_legacy_constructor_payload_attr sp.ppat_attributes
1407-
in
1408-
let sp = {sp with ppat_attributes} in
14091414
let opath =
14101415
try
14111416
let p0, p, _ = extract_concrete_variant !env expected_ty in
@@ -1440,10 +1445,7 @@ and type_pat_aux ~constrs ~labels ~no_existentials ~mode ~explode ~env sp
14401445
correct head *)
14411446
if constr.cstr_generalized then unify_head_only loc !env expected_ty constr;
14421447
let sargs =
1443-
match sargs with
1444-
| [{ppat_desc = Ppat_tuple sargs}]
1445-
when has_legacy_constructor_payload && constr.cstr_arity > 1 ->
1446-
sargs
1448+
match constructor_args_of_pat_payload ~arity:constr.cstr_arity sargs with
14471449
| [({ppat_desc = Ppat_any} as sp)] when constr.cstr_arity <> 1 ->
14481450
if constr.cstr_arity = 0 then
14491451
Location.prerr_warning sp.ppat_loc
@@ -4429,9 +4431,6 @@ and type_application ~context total_app env funct (sargs : sargs) :
44294431
Apply_non_function (expand_head env funct.exp_type) )))
44304432
44314433
and type_construct ~context env loc lid sargs ty_expected attrs =
4432-
let has_legacy_constructor_payload, attrs =
4433-
remove_legacy_constructor_payload_attr attrs
4434-
in
44354434
let opath =
44364435
try
44374436
let p0, p, _ = extract_concrete_variant env ty_expected in
@@ -4447,13 +4446,7 @@ and type_construct ~context env loc lid sargs ty_expected attrs =
44474446
Env.mark_constructor Env.Positive env (Longident.last lid.txt) constr;
44484447
Builtin_attributes.check_deprecated loc constr.cstr_attributes
44494448
constr.cstr_name;
4450-
let sargs =
4451-
match sargs with
4452-
| [{pexp_desc = Pexp_tuple sargs}]
4453-
when has_legacy_constructor_payload && constr.cstr_arity > 1 ->
4454-
sargs
4455-
| sargs -> sargs
4456-
in
4449+
let sargs = constructor_args_of_exp_payload ~arity:constr.cstr_arity sargs in
44574450
if List.length sargs <> constr.cstr_arity then
44584451
raise
44594452
(Error

compiler/syntax/src/res_parsetree_viewer.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ let filter_parsing_attrs attrs =
228228
( "res.braces" | "ns.braces" | "res.iflet" | "res.ternary"
229229
| "res.await" | "res.patVariantSpread" | "res.dictPattern"
230230
| "res.dictSpread" | "res.inlineRecordDefinition"
231-
| "res.variantArgs" | "_res.legacy_constructor_payload" );
231+
| "res.variantArgs" );
232232
},
233233
_ ) ->
234234
false

compiler/syntax/src/res_printer.ml

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2617,16 +2617,6 @@ and print_extension ~state ~at_module_lvl (string_loc, payload) cmt_tbl =
26172617
in
26182618
Doc.group (Doc.concat [ext_name; print_payload ~state payload cmt_tbl])
26192619

2620-
and remove_legacy_constructor_payload_attr attrs =
2621-
let rec loop rev_attrs = function
2622-
| ({Location.txt = "_res.legacy_constructor_payload"}, Parsetree.PStr [])
2623-
:: attrs ->
2624-
(true, List.rev_append rev_attrs attrs)
2625-
| attr :: attrs -> loop (attr :: rev_attrs) attrs
2626-
| [] -> (false, List.rev rev_attrs)
2627-
in
2628-
loop [] attrs
2629-
26302620
and print_pattern_args ~state (patterns : Parsetree.pattern list) cmt_tbl =
26312621
match patterns with
26322622
| [] -> Doc.nil
@@ -2671,15 +2661,6 @@ and print_pattern_args ~state (patterns : Parsetree.pattern list) cmt_tbl =
26712661
]
26722662

26732663
and print_pattern ~state (p : Parsetree.pattern) cmt_tbl =
2674-
let has_legacy_constructor_payload, ppat_attributes =
2675-
remove_legacy_constructor_payload_attr p.ppat_attributes
2676-
in
2677-
let p =
2678-
match (has_legacy_constructor_payload, p.ppat_desc) with
2679-
| true, Ppat_construct (constr, [{ppat_desc = Ppat_tuple args}]) ->
2680-
{p with ppat_desc = Ppat_construct (constr, args); ppat_attributes}
2681-
| _ -> {p with ppat_attributes}
2682-
in
26832664
let pattern_without_attributes =
26842665
match p.ppat_desc with
26852666
| Ppat_any -> Doc.text "_"
@@ -3196,15 +3177,6 @@ and print_object_get_doc ~state parent_expr (label : string Location.loc)
31963177
Doc.group (Doc.concat [parent_doc; Doc.lbracket; member; Doc.rbracket])
31973178

31983179
and print_expression ~state (e : Parsetree.expression) cmt_tbl =
3199-
let has_legacy_constructor_payload, pexp_attributes =
3200-
remove_legacy_constructor_payload_attr e.pexp_attributes
3201-
in
3202-
let e =
3203-
match (has_legacy_constructor_payload, e.pexp_desc) with
3204-
| true, Pexp_construct (constr, [{pexp_desc = Pexp_tuple args}]) ->
3205-
{e with pexp_desc = Pexp_construct (constr, args); pexp_attributes}
3206-
| _ -> {e with pexp_attributes}
3207-
in
32083180
let printed_expression =
32093181
match e.pexp_desc with
32103182
| Pexp_fun

tests/ERROR_VARIANTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ Source: [typecore.ml:27](../compiler/ml/typecore.ml).
201201
| Variant | Status | Fixture | Notes |
202202
|---|---|---|---|
203203
| `Polymorphic_label` || `polymorphic_label.res` | Pattern that instantiates a polymorphic record field: `({f: (f: int => int)}: t) =>` constrains the universal `'a` of `f: 'a. 'a => 'a` to `int => int`. |
204-
| `Constructor_arity_mismatch` || `constructor_arity_mismatch.res`, `constructor_arity_mismatch_pattern.res`, `constructor_tuple_arity_mismatch.res`, `constructor_tuple_arity_mismatch_pattern.res`, `arity_mismatch*.res` | Triggers in both expression and pattern paths, including the distinction between multiple arguments and one tuple argument. |
204+
| `Constructor_arity_mismatch` || `constructor_arity_mismatch.res`, `constructor_arity_mismatch_pattern.res`, `constructor_tuple_arity_mismatch.res`, `constructor_tuple_arity_mismatch_pattern.res`, `arity_mismatch*.res` | Triggers in both expression and pattern paths, after semantic argument normalization. |
205205
| `Label_mismatch` || `label_mismatch_record_literal.res` | Record literal without expected type mixing fields from two different record types — disambiguation picks one type per label, and the cross-type unify fails inside `type_label_exp`. |
206206
| `Pattern_type_clash` || many `*_pattern_type_clash.res` etc. | Most-fired pattern error. Sub-case fixtures: `pattern_matching_on_option_but_value_not_option.res` and `pattern_matching_on_value_but_is_option.res` (option-vs-non-option trace), `pattern_type_clash_polyvariant.res` (polyvariant tag against concrete type), `pattern_type_clash_tuple_arity.res` (tuple arity mismatch). |
207207
| `Or_pattern_type_clash` || `or_pattern_type_clash.res` | |
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11

22
We've found a bug for you!
3-
/.../fixtures/constructor_tuple_arity_mismatch.res:3:15-25
3+
/.../fixtures/constructor_tuple_arity_mismatch.res:3:15-31
44

5-
1 │ type unary = Unary((int, int))
5+
1 │ type binary = Binary(int, int)
66
2 │
7-
3 │ let invalid = Unary(1, 2)
7+
3 │ let invalid = Binary((1, 2, 3))
88
4 │
99

10-
This variant constructor Unary expects 1 argument, but it's being passed 2.
10+
This variant constructor Binary expects 2 arguments, but it's being passed 3.
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11

22
We've found a bug for you!
3-
/.../fixtures/constructor_tuple_arity_mismatch_pattern.res:5:5-18
3+
/.../fixtures/constructor_tuple_arity_mismatch_pattern.res:5:5-21
44

55
3 │ let read = value =>
66
4 │ switch value {
7-
5 │ | Binary((x, y)) => x + y
7+
5 │ | Binary((x, y, z)) => x + y + z
88
6 │ }
99
7 │
1010

11-
This variant constructor Binary expects 2 arguments, but it's only being passed 1.
11+
This variant constructor Binary expects 2 arguments, but it's being passed 3.
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
type unary = Unary((int, int))
1+
type binary = Binary(int, int)
22

3-
let invalid = Unary(1, 2)
3+
let invalid = Binary((1, 2, 3))

0 commit comments

Comments
 (0)