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

WIP: Emit array spreads using the native es array spread syntax #7034

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

- Improve bigint literal comparison. https://github.com/rescript-lang/rescript-compiler/pull/7029
- Improve output of `@variadic` bindings. https://github.com/rescript-lang/rescript-compiler/pull/7030
- Emit array spreads as the native ES `...` spread syntax, instead of `Belt.Array.concatMany`. https://github.com/rescript-lang/rescript-compiler/pull/7034

# 12.0.0-alpha.3

Expand Down
4 changes: 4 additions & 0 deletions jscomp/core/lam_dispatch_primitive.ml
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,10 @@ let translate loc (prim_name : string) (args : J.expression list) : J.expression
match args with
| [e] -> {e with expression_desc = Await e}
| _ -> assert false)
| "?array_spread" -> (
match args with
| [e] -> {e with expression_desc = Spread e}
| _ -> assert false)
| "?create_dict" -> (
match args with
| [{expression_desc = Array (items, _)}] ->
Expand Down
17 changes: 17 additions & 0 deletions jscomp/frontend/bs_builtin_ppx.ml
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,23 @@ let expr_mapper ~async_context ~in_function_def (self : mapper)
me,
self.expr self expr );
}
| Pexp_array exprs
when exprs
|> List.exists (fun e ->
Res_parsetree_viewer.has_array_spread_attribute
e.Parsetree.pexp_attributes) ->
{
e with
pexp_desc =
Pexp_array
(exprs
|> List.map (fun e ->
if
Res_parsetree_viewer.has_array_spread_attribute
e.Parsetree.pexp_attributes
then Ast_array_spread.create_array_spread_expression e
else e));
}
| _ -> default_expr_mapper self e

let expr_mapper ~async_context ~in_function_def (self : mapper)
Expand Down
9 changes: 9 additions & 0 deletions jscomp/ml/ast_array_spread.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
let create_array_spread_expression (e : Parsetree.expression) =
let loc = e.pexp_loc in
let unsafe_array_spread =
Ast_helper.Exp.ident ~loc
{txt = Ldot (Lident Js_runtime_modules.array, "unsafe_spread"); loc}
in
Ast_helper.Exp.apply ~loc unsafe_array_spread [(Nolabel, e)]


2 changes: 2 additions & 0 deletions jscomp/runtime/caml_array.res
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,5 @@ let blit = (a1, i1, a2, i2, len) =>
a2->unsafe_set(j + i2, a1->unsafe_get(j + i1))
}
}

external unsafe_spread: array<'a> => 'a = "?array_spread"
2 changes: 2 additions & 0 deletions jscomp/runtime/caml_array.resi
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,5 @@ let blit: (array<'a>, int, array<'a>, int, int) => unit
let get: (array<'a>, int) => 'a

let set: (array<'a>, int, 'a) => unit

external unsafe_spread: array<'a> => 'a = "?array_spread"
62 changes: 15 additions & 47 deletions jscomp/syntax/src/res_core.ml
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ let tagged_template_literal_attr =

let spread_attr = (Location.mknoloc "res.spread", Parsetree.PStr [])

let array_spread_attr = (Location.mknoloc "res.arraySpread", Parsetree.PStr [])

type argument = {label: Asttypes.arg_label; expr: Parsetree.expression}

type type_parameter = {
Expand Down Expand Up @@ -3954,57 +3956,23 @@ and parse_dict_expr ~start_pos p =
and parse_array_exp p =
let start_pos = p.Parser.start_pos in
Parser.expect Lbracket p;
let split_by_spread exprs =
List.fold_left
(fun acc curr ->
match (curr, acc) with
| (true, expr, start_pos, end_pos), _ ->
(* find a spread expression, prepend a new sublist *)
([], Some expr, start_pos, end_pos) :: acc
| ( (false, expr, start_pos, _endPos),
(no_spreads, spread, _accStartPos, acc_end_pos) :: acc ) ->
(* find a non-spread expression, and the accumulated is not empty,
* prepend to the first sublist, and update the loc of the first sublist *)
(expr :: no_spreads, spread, start_pos, acc_end_pos) :: acc
| (false, expr, start_pos, end_pos), [] ->
(* find a non-spread expression, and the accumulated is empty *)
[([expr], None, start_pos, end_pos)])
[] exprs
in
let list_exprs_rev =
let exprs =
parse_comma_delimited_reversed_list p ~grammar:Grammar.ExprList
~closing:Rbracket ~f:parse_spread_expr_region_with_loc
|> List.rev
in
Parser.expect Rbracket p;
let loc = mk_loc start_pos p.prev_end_pos in
let collect_exprs = function
| [], Some spread, _startPos, _endPos -> [spread]
| exprs, Some spread, _startPos, _endPos ->
let els = Ast_helper.Exp.array ~loc exprs in
[els; spread]
| exprs, None, _startPos, _endPos ->
let els = Ast_helper.Exp.array ~loc exprs in
[els]
in
match split_by_spread list_exprs_rev with
| [] -> Ast_helper.Exp.array ~loc:(mk_loc start_pos p.prev_end_pos) []
| [(exprs, None, _, _)] ->
Ast_helper.Exp.array ~loc:(mk_loc start_pos p.prev_end_pos) exprs
| exprs ->
let xs = List.map collect_exprs exprs in
let list_exprs =
List.fold_right
(fun exprs1 acc ->
List.fold_right (fun expr1 acc1 -> expr1 :: acc1) exprs1 acc)
xs []
in
Ast_helper.Exp.apply ~loc
(Ast_helper.Exp.ident ~loc ~attrs:[spread_attr]
(Location.mkloc
(Longident.Ldot
(Longident.Ldot (Longident.Lident "Belt", "Array"), "concatMany"))
loc))
[(Asttypes.Nolabel, Ast_helper.Exp.array ~loc list_exprs)]
Ast_helper.Exp.array
~loc:(mk_loc start_pos p.prev_end_pos)
(exprs
|> List.map (fun (is_spread, expr, _, _) ->
if is_spread then
{
expr with
Parsetree.pexp_attributes =
array_spread_attr :: expr.Parsetree.pexp_attributes;
}
else expr))

(* TODO: check attributes in the case of poly type vars,
* might be context dependend: parseFieldDeclaration (see ocaml) *)
Expand Down
24 changes: 8 additions & 16 deletions jscomp/syntax/src/res_parsetree_viewer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,12 @@ let has_await_attribute attrs =
| _ -> false)
attrs

let collect_array_expressions expr =
match expr.pexp_desc with
| Pexp_array exprs -> (exprs, None)
| _ -> ([], Some expr)
let has_array_spread_attribute attrs =
List.exists
(function
| {Location.txt = "res.arraySpread"}, _ -> true
| _ -> false)
attrs

let collect_list_expressions expr =
let rec collect acc expr =
Expand Down Expand Up @@ -219,7 +221,8 @@ let filter_parsing_attrs attrs =
Location.txt =
( "res.arity" | "res.braces" | "ns.braces" | "res.iflet"
| "res.namedArgLoc" | "res.optional" | "res.ternary" | "res.async"
| "res.await" | "res.template" | "res.taggedTemplate" );
| "res.await" | "res.template" | "res.taggedTemplate"
| "res.arraySpread" );
},
_ ) ->
false
Expand Down Expand Up @@ -680,17 +683,6 @@ let is_spread_belt_list_concat expr =
has_spread_attr expr.pexp_attributes
| _ -> false

let is_spread_belt_array_concat expr =
match expr.pexp_desc with
| Pexp_ident
{
txt =
Longident.Ldot
(Longident.Ldot (Longident.Lident "Belt", "Array"), "concatMany");
} ->
has_spread_attr expr.pexp_attributes
| _ -> false

(* Blue | Red | Green -> [Blue; Red; Green] *)
let collect_or_pattern_chain pat =
let rec loop pattern chain =
Expand Down
8 changes: 2 additions & 6 deletions jscomp/syntax/src/res_parsetree_viewer.mli
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ val process_function_attributes :

val has_await_attribute : Parsetree.attributes -> bool

val has_array_spread_attribute : Parsetree.attributes -> bool

type if_condition_kind =
| If of Parsetree.expression
| IfLet of Parsetree.pattern * Parsetree.expression
Expand All @@ -39,10 +41,6 @@ val collect_if_expressions :
(Location.t * if_condition_kind * Parsetree.expression) list
* Parsetree.expression option

val collect_array_expressions :
Parsetree.expression ->
Parsetree.expression list * Parsetree.expression option

val collect_list_expressions :
Parsetree.expression ->
Parsetree.expression list * Parsetree.expression option
Expand Down Expand Up @@ -140,8 +138,6 @@ val has_template_literal_attr : Parsetree.attributes -> bool

val is_spread_belt_list_concat : Parsetree.expression -> bool

val is_spread_belt_array_concat : Parsetree.expression -> bool

val collect_or_pattern_chain : Parsetree.pattern -> Parsetree.pattern list

val process_braces_attr :
Expand Down
76 changes: 13 additions & 63 deletions jscomp/syntax/src/res_printer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ let add_parens doc =
Doc.rparen;
])

let add_spread doc = Doc.concat [Doc.dotdotdot; doc]

let add_braces doc =
Doc.group
(Doc.concat
Expand Down Expand Up @@ -2923,10 +2925,17 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl =
let doc =
print_expression_with_comments ~state expr cmt_tbl
in
match Parens.expr expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc)
let doc =
match Parens.expr expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc
in
if
ParsetreeViewer.has_array_spread_attribute
expr.pexp_attributes
then add_spread doc
else doc)
exprs);
]);
Doc.trailing_comma;
Expand Down Expand Up @@ -3117,9 +3126,6 @@ and print_expression ~state (e : Parsetree.expression) cmt_tbl =
Doc.text expr
| extension ->
print_extension ~state ~at_module_lvl:false extension cmt_tbl)
| Pexp_apply (e, [(Nolabel, {pexp_desc = Pexp_array sub_lists})])
when ParsetreeViewer.is_spread_belt_array_concat e ->
print_belt_array_concat_apply ~state sub_lists cmt_tbl
| Pexp_apply (e, [(Nolabel, {pexp_desc = Pexp_array sub_lists})])
when ParsetreeViewer.is_spread_belt_list_concat e ->
print_belt_list_concat_apply ~state sub_lists cmt_tbl
Expand Down Expand Up @@ -3899,62 +3905,6 @@ and print_binary_expression ~state (expr : Parsetree.expression) cmt_tbl =
])
| _ -> Doc.nil

and print_belt_array_concat_apply ~state sub_lists cmt_tbl =
let make_spread_doc comma_before_spread = function
| Some expr ->
Doc.concat
[
comma_before_spread;
Doc.dotdotdot;
(let doc = print_expression_with_comments ~state expr cmt_tbl in
match Parens.expr expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc);
]
| None -> Doc.nil
in
let make_sub_list_doc (expressions, spread) =
let comma_before_spread =
match expressions with
| [] -> Doc.nil
| _ -> Doc.concat [Doc.text ","; Doc.line]
in
let spread_doc = make_spread_doc comma_before_spread spread in
Doc.concat
[
Doc.join
~sep:(Doc.concat [Doc.text ","; Doc.line])
(List.map
(fun expr ->
let doc = print_expression_with_comments ~state expr cmt_tbl in
match Parens.expr expr with
| Parens.Parenthesized -> add_parens doc
| Braced braces -> print_braces doc expr braces
| Nothing -> doc)
expressions);
spread_doc;
]
in
Doc.group
(Doc.concat
[
Doc.lbracket;
Doc.indent
(Doc.concat
[
Doc.soft_line;
Doc.join
~sep:(Doc.concat [Doc.text ","; Doc.line])
(List.map make_sub_list_doc
(List.map ParsetreeViewer.collect_array_expressions
sub_lists));
]);
Doc.trailing_comma;
Doc.soft_line;
Doc.rbracket;
])

and print_belt_list_concat_apply ~state sub_lists cmt_tbl =
let make_spread_doc comma_before_spread = function
| Some expr ->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
let x = [|1;2;3|]
let x = [|1;2;3|]
let x = [|(1 : int);(2 : int);(3 : int)|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|[|4;5|];y|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|[|4;5|];y;[|7|];y|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|[|4;5|];(y : int array)|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|[|4;5;k|];y|]
let x = ((Belt.Array.concatMany)[@res.spread ]) [|y|]
let x = [|4;5;((y)[@res.arraySpread ])|]
let x = [|4;5;((y)[@res.arraySpread ]);7;((y)[@res.arraySpread ])|]
let x = [|4;5;(((y : int array))[@res.arraySpread ])|]
let x = [|4;5;k;((y)[@res.arraySpread ])|]
let x = [|((y)[@res.arraySpread ])|]
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ let foo = ((Function$ (fun x -> ((x : t) :> int)))[@res.arity 1])
let _ = (x : int)
let foo = ((x : int), (y :> float))
let foo = ((x : int), (y :> float), (z :> int))
let foo = ((x : int), y, (z :> int))
let foo = ((x : int), y, (z :> int))
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ let flags =
(fun items ->
((match items with
| [|{js|-pp|js};_ppFlag;rest|] -> loop rest
| [|x;rest|] ->
((Belt.Array.concatMany)[@res.spread ])
[|[|x|];(loop rest)|]
| [|x;rest|] -> [|x;((loop rest)[@res.arraySpread ])|]
| [||] -> [||])
[@res.braces ])))
[@res.arity 1]) in
Expand Down
21 changes: 21 additions & 0 deletions jscomp/test/array_spreads.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions jscomp/test/array_spreads.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
let xx = [3]
let x = [1, 2, 3, ...Js.Array2.map(xx, v => v * 2)]
let y = [...x, 4, 5, ...x, 6, ...x->Js.Array2.map(v => v * 2)]
Comment on lines +2 to +3
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Top Js.Array2.map works (no pipe), but bottom does not (using pipes). @cristianoc got any spontaneous ideas about this?

3 changes: 2 additions & 1 deletion jscomp/test/build.ninja

Large diffs are not rendered by default.

Loading