diff --git a/jscomp/common/external_ffi_types.ml b/jscomp/common/external_ffi_types.ml index 4cf156032..90914edb1 100644 --- a/jscomp/common/external_ffi_types.ml +++ b/jscomp/common/external_ffi_types.ml @@ -46,7 +46,7 @@ type arg_type = External_arg_spec.attr hand *) type arg_label = External_arg_spec.Obj_label.t -type js_send_kind = Send | Invoke | Pipe +type js_send_kind = Pipe | Send of int type external_spec = | Js_var of { diff --git a/jscomp/common/external_ffi_types.mli b/jscomp/common/external_ffi_types.mli index e0d7ffa40..7fd5e5249 100644 --- a/jscomp/common/external_ffi_types.mli +++ b/jscomp/common/external_ffi_types.mli @@ -39,7 +39,7 @@ type external_module_name = { type arg_type = External_arg_spec.attr type arg_label = External_arg_spec.Obj_label.t -type js_send_kind = Send | Invoke | Pipe +type js_send_kind = Pipe | Send of int type external_spec = | Js_var of { diff --git a/jscomp/core/lam_compile_external_call.ml b/jscomp/core/lam_compile_external_call.ml index 91fdf80da..6e0d5d2e3 100644 --- a/jscomp/core/lam_compile_external_call.ml +++ b/jscomp/core/lam_compile_external_call.ml @@ -266,44 +266,17 @@ let translate_scoped_module_val let start = E.js_global x in List.fold_left ~f:E.dot ~init:start (rest @ [ fn ])) -let js_send_self_and_args = - let rec inner args arg_types (acc_args, acc_arg_types) = - match (args, arg_types) with - | [], [] -> assert false - | ( self :: args, - { - (* The "self" arg in `[@mel.send]` is the first non-labelled argument - that we find, e.g. `t` in `external foo: ?a:int -> t -> unit`. - This allows to write expressive bindings to JS that are applied - via dot-notation and still model the "self" arg after those in OCaml - such that those labels can be applied optionally. *) - External_arg_spec.arg_label = External_arg_spec.Arg_label.Arg_empty; - _; - } - :: arg_types ) -> - ( self, - List.rev_append acc_args args, - List.rev_append acc_arg_types arg_types ) - | ( arg :: rest, - ({ arg_label = Arg_label | Arg_optional; _ } as arg_type) :: types_rest - ) -> - inner rest types_rest (arg :: acc_args, arg_type :: acc_arg_types) - | [], _ :: _ | _ :: _, [] -> assert false - in - fun args arg_types ~kind -> - match kind with - | External_ffi_types.Pipe -> assert false - | Send -> ( - match args with - | self :: args -> - (* PR2162 [self_type] more checks in syntax: +let js_send_self_and_args args arg_types ~self_idx = + (* PR2162 [self_type] more checks in syntax: - should not be [@as] *) - let[@ocaml.warning "-partial-match"] (_self_type :: arg_types) = - arg_types - in - (self, args, arg_types) - | _ -> assert false) - | Invoke -> inner args arg_types ([], []) + let[@ocaml.warning "-partial-match"] args_pre, self :: args_post = + List.split_at args self_idx + in + let[@ocaml.warning "-partial-match"] ( arg_types_pre, + _self_types :: arg_types_post ) = + List.split_at arg_types self_idx + in + (self, args_pre @ args_post, arg_types_pre @ arg_types_post) let translate_ffi = let translate_scoped_access scopes obj = @@ -412,9 +385,9 @@ let translate_ffi = add_eff eff (let self = translate_scoped_access scopes self in process_send ~new_ self name args) - | (Send | Invoke), true -> + | Send self_idx, true -> let self, args, arg_types = - js_send_self_and_args args arg_types ~kind + js_send_self_and_args args arg_types ~self_idx in let args, eff, dynamic = assemble_args_has_splice arg_types args in add_eff eff @@ -424,9 +397,9 @@ let translate_ffi = | true -> splice_fn_new_apply (E.dot self name) args | false -> splice_obj_fn_apply self name args else process_send ~new_ self name args) - | (Send | Invoke), false -> + | Send self_idx, false -> let self, args, arg_types = - js_send_self_and_args args arg_types ~kind + js_send_self_and_args args arg_types ~self_idx in (* PR2162 [self_type] more checks in syntax: - should not be [@mel.as] *) diff --git a/ppx/ast_external_process.ml b/ppx/ast_external_process.ml index 83ea246ad..eb808310a 100644 --- a/ppx/ast_external_process.ml +++ b/ppx/ast_external_process.ml @@ -207,7 +207,6 @@ type external_desc = { external_module_name : External_ffi_types.external_module_name option; module_as_val : External_ffi_types.external_module_name option; val_send : name_source; - val_invoke : name_source; val_send_pipe : core_type option; variadic : bool; (* mutable *) scopes : string list; @@ -226,7 +225,6 @@ let init_st = external_module_name = None; module_as_val = None; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; variadic = false; scopes = []; @@ -340,8 +338,6 @@ let parse_external_attributes (prim_name_check : string) | "mel.send" | "bs.send" | "send" -> Ast_attributes.error_if_bs_or_non_namespaced ~loc txt; { st with val_send = name_from_payload_or_prim ~loc payload } - | "mel.invoke" -> - { st with val_invoke = name_from_payload_or_prim ~loc payload } | "mel.send.pipe" | "bs.send.pipe" | "send.pipe" -> Ast_attributes.error_if_bs_or_non_namespaced ~loc txt; { @@ -436,19 +432,19 @@ type response = { type param_type = { label : Asttypes.arg_label; ty : core_type; - attr : attributes; + attrs : attributes; loc : location; } let mk_fn_type (new_arg_types_ty : param_type list) (result : core_type) : core_type = List.fold_right - ~f:(fun { label; ty; attr; loc } acc -> + ~f:(fun { label; ty; attrs; loc } acc -> { ptyp_desc = Ptyp_arrow (label, ty, acc); ptyp_loc = loc; ptyp_loc_stack = [ loc ]; - ptyp_attributes = attr; + ptyp_attributes = attrs; }) new_arg_types_ty ~init:result @@ -460,7 +456,6 @@ let process_obj (loc : Location.t) (st : external_desc) (prim_name : string) external_module_name = None; module_as_val = None; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; variadic = false; new_name = `Nm_na; @@ -665,6 +660,27 @@ let process_obj (loc : Location.t) (st : external_desc) (prim_name : string) Location.raise_errorf ~loc "Found an attribute that conflicts with `[%@mel.obj]'" +let mel_send_this_index arg_types = + let find_index ~f:p = + let rec aux i = function + | [] -> None + | a :: l -> if p a then Some i else aux (i + 1) l + in + aux 0 + in + find_index + ~f:(fun { attrs; _ } -> + List.exists + ~f:(fun ({ attr_name = { txt; _ }; _ } as attr) -> + match txt with + | "mel.this" -> + Mel_ast_invariant.mark_used_mel_attribute attr; + true + | _ -> false) + attrs) + arg_types + |> Option.value ~default:0 + let external_desc_of_non_obj (loc : Location.t) (st : external_desc) (prim_name_or_pval_prim : bundle_source) (arg_type_specs_length : int) arg_types_ty @@ -677,7 +693,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) external_module_name = None; module_as_val = None; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; variadic = false; scopes; @@ -703,7 +718,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) external_module_name = None; module_as_val = None; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; variadic = false; scopes; @@ -730,7 +744,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) new_name; external_module_name = None; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; scopes = []; (* module as var does not need scopes *) @@ -768,7 +781,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) set_index = false; get_index = false; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; new_name = `Nm_na; set_name = `Nm_na; @@ -796,7 +808,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) external_module_name; module_as_val = None; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; set_index = false; get_index = false; @@ -822,7 +833,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) call_name = `Nm_na; module_as_val = None; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; set_index = false; get_index = false; @@ -843,40 +853,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) else Js_call { variadic; name; external_module_name; scopes } | { val_send = `Nm_external (lazy name) | `Nm_payload name; - val_invoke = `Nm_na; - variadic; - scopes; - val_send_pipe = None; - call_name = `Nm_na; - module_as_val = None; - set_index = false; - get_index = false; - new_name; - set_name = `Nm_na; - get_name = `Nm_na; - external_module_name = None; - mk_obj = _; - return_wrapper = _; - } -> ( - (* PR #2162 - since when we assemble arguments the first argument in - [@@send] is ignored *) - match (arg_type_specs, new_name) with - | [], _ -> - Location.raise_errorf ~loc - "`[%@mel.send]` requires a function with at least one argument" - | { arg_type = Arg_cst _; arg_label = _ } :: _, _ -> - Location.raise_errorf ~loc - "`[%@mel.send]`'s first argument must not be a constant" - | _, `Nm_payload _ -> - Location.raise_errorf ~loc - "`[%@mel.send]' doesn't expect an attribute payload" - | _ :: _, `Nm_na -> - Js_send { variadic; name; scopes; kind = Send; new_ = false } - | _ :: _, `Nm_external _ -> - Js_send { variadic; name; scopes; kind = Send; new_ = true }) - | { - val_send = `Nm_na; - val_invoke = `Nm_external (lazy name) | `Nm_payload name; variadic; scopes; val_send_pipe = None; @@ -903,21 +879,22 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) | _, `Nm_payload _ -> Location.raise_errorf ~loc "`[%@mel.send]' doesn't expect an attribute payload" - | _ :: _, `Nm_na -> - Js_send { variadic; name; scopes; kind = Invoke; new_ = false } - | _ :: _, `Nm_external _ -> - Js_send { variadic; name; scopes; kind = Invoke; new_ = true }) + | _ :: _, (`Nm_na | `Nm_external _) -> + Js_send + { + variadic; + name; + scopes; + kind = Send (mel_send_this_index arg_types_ty); + new_ = not (new_name = `Nm_na); + }) | { val_send = #bundle_source; _ } -> Location.raise_errorf ~loc "Found an attribute that can't be used with `[%@mel.send]'" - | { val_invoke = #bundle_source; _ } -> - Location.raise_errorf ~loc - "Found an attribute that can't be used with `[%@mel.invoke]'" | { val_send_pipe = Some _; (* variadic = (false as variadic); *) val_send = `Nm_na; - val_invoke = `Nm_na; call_name = `Nm_na; module_as_val = None; set_index = false; @@ -965,7 +942,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) set_index = false; get_index = false; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; set_name = `Nm_na; get_name = `Nm_na; @@ -986,7 +962,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) set_index = false; get_index = false; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; new_name = `Nm_na; get_name = `Nm_na; @@ -1010,7 +985,6 @@ let external_desc_of_non_obj (loc : Location.t) (st : external_desc) set_index = false; get_index = false; val_send = `Nm_na; - val_invoke = `Nm_na; val_send_pipe = None; new_name = `Nm_na; set_name = `Nm_na; @@ -1033,7 +1007,12 @@ let list_of_arrow (ty : core_type) : core_type * param_type list = match ty.ptyp_desc with | Ptyp_arrow (label, t1, t2) -> aux t2 - (({ label; ty = t1; attr = ty.ptyp_attributes; loc = ty.ptyp_loc } + (({ + label; + ty = t1; + attrs = ty.ptyp_attributes @ t1.ptyp_attributes; + loc = ty.ptyp_loc; + } : param_type) :: acc) | Ptyp_poly (_, ty) -> @@ -1190,12 +1169,11 @@ module From_attributes = struct dont_inline_cross_module = false; } else - let arg_type_specs, new_arg_types_ty, arg_type_specs_length = - let variadic = external_desc.variadic in + let arg_type_specs, new_arg_types_ty, (arg_type_specs_length, _) = let (init : External_arg_spec.Arg_label.t External_arg_spec.param list * param_type list - * int) = + * (int * bool)) = match external_desc.val_send_pipe with | Some obj -> ( match refine_arg_type ~nolabel:true obj with @@ -1210,43 +1188,58 @@ module From_attributes = struct { label = Nolabel; ty = obj; - attr = []; + attrs = []; loc = obj.ptyp_loc; }; ], - 0 )) - | None -> ([], [], 0) + (0, false) )) + | None -> ([], [], (0, false)) in List.fold_right - ~f:(fun param_type (arg_type_specs, arg_types, i) -> + ~f:(fun + param_type + (arg_type_specs, arg_types, (i, last_was_mel_this)) + -> let arg_label = param_type.label in let ty = param_type.ty in - (if i = 0 && variadic then + let is_variadic = + (i = 0 || (i = 1 && last_was_mel_this)) + && external_desc.variadic + in + let is_mel_this_and_send = + external_desc.val_send <> `Nm_na + && List.exists + ~f:(fun { attr_name = { txt; _ }; _ } -> + txt = "mel.this") + param_type.attrs + in + (if is_variadic && not is_mel_this_and_send then match arg_label with | Optional _ -> Location.raise_errorf ~loc "`[%@mel.variadic]' cannot be applied to an \ optionally labelled argument" | Labelled _ | Nolabel -> ( - if ty.ptyp_desc = Ptyp_any then - Location.raise_errorf - "`[%@mel.variadic]' expects its last argument to be \ - an array" - else - match spec_of_ptyp ~nolabel:true ty with - | Nothing -> ( - match ty.ptyp_desc with - | Ptyp_constr ({ txt = Lident "array"; _ }, [ _ ]) - -> - () - | _ -> - Location.raise_errorf ~loc - "`[%@mel.variadic]' expects its last \ - argument to be an array") - | _ -> - Location.raise_errorf ~loc - "`[%@mel.variadic]' expects its last argument \ - to be an array")); + match ty.ptyp_desc with + | Ptyp_any -> + Location.raise_errorf + "`[%@mel.variadic]' expects its last argument to \ + be an array" + | _ -> ( + match spec_of_ptyp ~nolabel:true ty with + | Nothing -> ( + match ty.ptyp_desc with + | Ptyp_constr ({ txt = Lident "array"; _ }, [ _ ]) + -> + () + | _ -> + Location.raise_errorf ~loc + "`[%@mel.variadic]' expects its last \ + argument to be an array") + | _ -> + Location.raise_errorf ~loc + "`[%@mel.variadic]' expects its last argument \ + to be an array"))); let ( (arg_label : External_arg_spec.Arg_label.t), arg_type, new_arg_types ) = @@ -1277,7 +1270,8 @@ module From_attributes = struct in ( { External_arg_spec.arg_label; arg_type } :: arg_type_specs, new_arg_types, - if arg_type = Ignore then i else i + 1 )) + if arg_type = Ignore then (i, last_was_mel_this) + else (i + 1, is_mel_this_and_send) )) arg_types_ty ~init in let ffi = diff --git a/test/blackbox-tests/mel-send-labeled-args.t b/test/blackbox-tests/mel-send-labeled-args.t deleted file mode 100644 index 7a455afe2..000000000 --- a/test/blackbox-tests/mel-send-labeled-args.t +++ /dev/null @@ -1,28 +0,0 @@ -Test `@mel.invoke + labeled / optional arguments - - $ . ./setup.sh - - $ cat > x.ml < type 'a t = 'a array - > external push : ?value:'a -> 'a t -> int = "push" [@@mel.invoke] - > let x = - > let arr = [| 0; 1; 2 |] in - > push arr ~value:3 - > EOF - - $ melc -ppx melppx x.ml - // Generated by Melange - 'use strict'; - - - const arr = [ - 0, - 1, - 2 - ]; - - const x = arr.push(3); - - exports.x = x; - /* x Not a pure module */ - diff --git a/test/blackbox-tests/mel-send-mel-this.t b/test/blackbox-tests/mel-send-mel-this.t new file mode 100644 index 000000000..f8723b68a --- /dev/null +++ b/test/blackbox-tests/mel-send-mel-this.t @@ -0,0 +1,56 @@ +Test `@mel.invoke + labeled / optional arguments + + $ . ./setup.sh + + $ cat > x.ml < type 'a t = 'a array + > external push : ?value:'a -> ('a t[@mel.this]) -> unit = "push" [@@mel.send] + > external pushfirst : 'a t ->'a -> unit = "push" [@@mel.send] + > let () = + > let arr = [| 0; 1; 2 |] in + > push arr ~value:3; + > pushfirst arr 3 + > EOF + + $ melc -ppx melppx x.ml + // Generated by Melange + 'use strict'; + + + const arr = [ + 0, + 1, + 2 + ]; + + arr.push(3); + + arr.push(3); + + /* Not a pure module */ + +Test: relax of `pushMany` to skip over [@mel.this] + + $ cat > x.ml < type 'a t = 'a array + > external pushMany : values:'a array -> ('a t[@mel.this]) -> unit = "push" + > [@@mel.send] [@@mel.variadic] + > let () = + > let arr = [| 0; 1; 2 |] in + > pushMany ~values:[|1;2|] arr + > EOF + + $ melc -ppx melppx x.ml + // Generated by Melange + 'use strict'; + + + const arr = [ + 0, + 1, + 2 + ]; + + arr.push(1, 2); + + /* Not a pure module */