From 0856c97ae83a7544ce28bf2776a2df4ee5f78591 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Wed, 14 Dec 2022 10:04:19 +0100 Subject: [PATCH 1/4] Explore default arguments in uncurried functions without a final unit argument Functions such as this one ```res let foo1 = (~x=3, ~y) => x+y ``` normally require a final unit unlabeled argument to indicate that the optional argument `x` is not passed. In this example, writing ```res let r1 = foo1(~y=11) ``` makes `r1` a function that is still expecting the `x` argument. Requiring a final unit argument, and passing it as `foo1(~y=11, ())` is the only way to omit `x` in practice. With uncurried functions, there's the opportunity to treat `foo1(~y=11)` in a special way, meaning that argument `x` is omitted. TODO: - Figure out what to do with the warning ""This optional parameter in final position will, in practice, not be optional" - Figure out the case of all optional arguments. There's no way to pass zero arguments. One could interpret `foo()`, which actually passes `()` as single unlabelled argument, in a special way when `foo` does not accept unlabelled arguments, and use it to mean zero arguments passed. --- CHANGELOG.md | 1 + jscomp/ml/typecore.ml | 26 ++++++++++++++++++-------- jscomp/test/uncurried_default.args.js | 21 +++++++++++++++++++++ jscomp/test/uncurried_default.args.res | 6 ++++++ 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 870c5c1da4..2b32515f98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ subset of the arguments, and return a curried type with the remaining ones https - Add support for default arguments in uncurried functions https://github.com/rescript-lang/rescript-compiler/pull/5835 - Inline uncurried application when it is safe https://github.com/rescript-lang/rescript-compiler/pull/5847 - Add support for toplevel `await` https://github.com/rescript-lang/rescript-compiler/pull/5940 +- Allow default arguments in uncurried functions without a final unit argument https://github.com/rescript-lang/rescript-compiler/pull/5907 #### :boom: Breaking Change diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 56303c5eda..499dbda3a5 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -3024,13 +3024,23 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex in let rec type_unknown_args max_arity (args : lazy_args) omitted ty_fun (syntax_args : sargs) : targs * _ = - match syntax_args with - | [] -> - (List.map - (function l, None -> l, None - | l, Some f -> l, Some (f ())) - (List.rev args), - instance env (result_type omitted ty_fun)) + match syntax_args with + | [] -> + let collect_args () = + (List.map + (function l, None -> l, None + | l, Some f -> l, Some (f ())) + (List.rev args), + instance env (result_type omitted ty_fun)) in + if List.length args < max_arity && uncurried then + (match (expand_head env ty_fun).desc with + | Tarrow (Optional l,t1,t2,_) -> + ignored := (Optional l,t1,ty_fun.level) :: !ignored; + let arg = Optional l, Some (fun () -> option_none (instance env t1) Location.none) in + type_unknown_args max_arity (arg::args) omitted t2 [] + | _ -> collect_args ()) + else + collect_args () | (l1, sarg1) :: sargl -> let (ty1, ty2) = let ty_fun = expand_head env ty_fun in @@ -3082,7 +3092,7 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex let sargs, omitted, arg = match extract_label name sargs with | None -> - if optional && label_assoc Nolabel sargs + if optional && (uncurried || label_assoc Nolabel sargs) then begin ignored := (l,ty,lv) :: !ignored; sargs, omitted , Some (fun () -> option_none (instance env ty) Location.none) diff --git a/jscomp/test/uncurried_default.args.js b/jscomp/test/uncurried_default.args.js index 7aacac8315..4a2df83c77 100644 --- a/jscomp/test/uncurried_default.args.js +++ b/jscomp/test/uncurried_default.args.js @@ -45,9 +45,30 @@ var partial$1 = Curry._1((function (param) { var total$1 = withOpt$1(10, 3)(4, 11); +function foo1(xOpt, y) { + var x = xOpt !== undefined ? xOpt : 3; + return x + y | 0; +} + +var x = 3; + +var r1 = x + 11 | 0; + +function foo2(y, xOpt, zOpt) { + var x = xOpt !== undefined ? xOpt : 3; + var z = zOpt !== undefined ? zOpt : 4; + return (x + y | 0) + z | 0; +} + +var r2 = foo2(11, undefined, undefined); + exports.StandardNotation = StandardNotation; exports.withOpt = withOpt$1; exports.testWithOpt = testWithOpt$1; exports.partial = partial$1; exports.total = total$1; +exports.foo1 = foo1; +exports.r1 = r1; +exports.foo2 = foo2; +exports.r2 = r2; /* testWithOpt Not a pure module */ diff --git a/jscomp/test/uncurried_default.args.res b/jscomp/test/uncurried_default.args.res index 346f4458bc..cfb7a4b043 100644 --- a/jscomp/test/uncurried_default.args.res +++ b/jscomp/test/uncurried_default.args.res @@ -13,3 +13,9 @@ let withOpt = (~x=1, y) => (~z=1, w) => x+y+z+w let testWithOpt = withOpt(3)(4) let partial = withOpt(. ~x=10)(. 3)(. ~z=4)(. 11) let total = withOpt(~x=10, 3)(~z=4, 11) + +let foo1 = (~x=3, ~y) => x+y +let r1 = foo1(~y=11) + +let foo2 = @warning("-16") (~y, ~x=3, ~z=4) => x+y+z +let r2 = foo2(~y=11) From c3131e6ac62257c928fcaa80b0cd929b9b8528ed Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Wed, 14 Dec 2022 10:36:08 +0100 Subject: [PATCH 2/4] Disable Unerasable_optional_argument for uncurried functions via the type checker. In reality this disables also the check for the body, but this check is on the way out so this should be reasonable in practice without over-complicating the error logic. --- jscomp/ml/typecore.ml | 17 +++++++++++------ jscomp/test/uncurried_default.args.res | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 499dbda3a5..0211a0f56b 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -2103,13 +2103,18 @@ and type_expect_ ?in_function ?(recarg=Rejected) env sexp ty_expected = exp_type = newty (Ttuple (List.map (fun e -> e.exp_type) expl)); exp_attributes = sexp.pexp_attributes; exp_env = env } + | Pexp_construct({txt = Lident "Function$"} as lid, sarg) -> + let state = Warnings.backup () in + let arity = Ast_uncurried.attributes_to_arity sexp.pexp_attributes in + let uncurried_typ = Ast_uncurried.make_uncurried_type ~env ~arity (newvar()) in + unify_exp_types loc env ty_expected uncurried_typ; + (* Disable Unerasable_optional_argument for uncurried functions *) + let unerasable_optional_argument = Warnings.number Unerasable_optional_argument in + Warnings.parse_options false ("-" ^ string_of_int unerasable_optional_argument); + let exp = type_construct env loc lid sarg ty_expected sexp.pexp_attributes in + Warnings.restore state; + exp | Pexp_construct(lid, sarg) -> - (match lid.txt with - | Lident "Function$" -> - let arity = Ast_uncurried.attributes_to_arity sexp.pexp_attributes in - let uncurried_typ = Ast_uncurried.make_uncurried_type ~env ~arity (newvar()) in - unify_exp_types loc env ty_expected uncurried_typ - | _ -> ()); type_construct env loc lid sarg ty_expected sexp.pexp_attributes | Pexp_variant(l, sarg) -> (* Keep sharing *) diff --git a/jscomp/test/uncurried_default.args.res b/jscomp/test/uncurried_default.args.res index cfb7a4b043..7577f75538 100644 --- a/jscomp/test/uncurried_default.args.res +++ b/jscomp/test/uncurried_default.args.res @@ -17,5 +17,5 @@ let total = withOpt(~x=10, 3)(~z=4, 11) let foo1 = (~x=3, ~y) => x+y let r1 = foo1(~y=11) -let foo2 = @warning("-16") (~y, ~x=3, ~z=4) => x+y+z +let foo2 = (~y, ~x=3, ~z=4) => x+y+z let r2 = foo2(~y=11) From 7785b261d44b4bbccd0fb1e6ee391290d59cdf3b Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Wed, 14 Dec 2022 20:00:36 +0100 Subject: [PATCH 3/4] Treat `foo(. )` as empty application if all arguments are optional. When all the processed arguments to the function are ignored (hence optional), and no arguments are ignored (so no mandatory labelled), it means the uncurried function only has optional arguments. - If the uncurried type of the function had an unlabelled arg, then it would be caught by the unit application, so it would be a non-ignored argument. But this is not possible as all the argiments are ignored. - If it had a labelled mandatory argument, then either it would be non-ignored, or omitted, but both cases are excluded. - It follows that the type only has optional arguments, and that the unit argument was the only argument supplied. The new mechanism does not kick in when some legit argument is passed alongside the unit. And noes not interfere with cases where the function expects a legitimate unit argument. --- jscomp/ml/typecore.ml | 6 ++- jscomp/test/uncurried_default.args.js | 61 +++++++++++++++++++++----- jscomp/test/uncurried_default.args.res | 12 +++++ 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/jscomp/ml/typecore.ml b/jscomp/ml/typecore.ml index 0211a0f56b..3badfcd50e 100644 --- a/jscomp/ml/typecore.ml +++ b/jscomp/ml/typecore.ml @@ -3019,7 +3019,7 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex match has_uncurried_type t with | Some (arity, _) -> let newarity = arity - nargs in - let fully_applied = newarity = 0 in + let fully_applied = newarity <= 0 in if uncurried && not fully_applied then raise(Error(funct.exp_loc, env, Uncurried_arity_mismatch (t, arity, List.length sargs))); @@ -3046,6 +3046,10 @@ and type_application uncurried env funct (sargs : sargs) : targs * Types.type_ex | _ -> collect_args ()) else collect_args () + | [(Nolabel, {pexp_desc = Pexp_construct ({txt = Lident "()"}, None)})] + when uncurried && omitted = [] && List.length args = List.length !ignored -> + (* foo(. ) treated as empty application if all args are optional (hence ignored) *) + type_unknown_args max_arity args omitted ty_fun [] | (l1, sarg1) :: sargl -> let (ty1, ty2) = let ty_fun = expand_head env ty_fun in diff --git a/jscomp/test/uncurried_default.args.js b/jscomp/test/uncurried_default.args.js index 4a2df83c77..5445f7508e 100644 --- a/jscomp/test/uncurried_default.args.js +++ b/jscomp/test/uncurried_default.args.js @@ -20,11 +20,42 @@ var partial = Curry._1((function (param) { var total = withOpt(10, 3)(4, 11); +function foo1(xOpt, y) { + var x = xOpt !== undefined ? xOpt : 3; + return x + y | 0; +} + +var x = 3; + +var r1 = x + 11 | 0; + +function foo2(y, xOpt, zOpt) { + var x = xOpt !== undefined ? xOpt : 3; + var z = zOpt !== undefined ? zOpt : 4; + return (x + y | 0) + z | 0; +} + +var r2 = foo2(11, undefined, undefined); + +function foo3(xOpt, yOpt) { + var x = xOpt !== undefined ? xOpt : 3; + var y = yOpt !== undefined ? yOpt : 4; + return x + y | 0; +} + +var r3 = foo3(undefined, undefined); + var StandardNotation = { withOpt: withOpt, testWithOpt: testWithOpt, partial: partial, - total: total + total: total, + foo1: foo1, + r1: r1, + foo2: foo2, + r2: r2, + foo3: foo3, + r3: r3 }; function withOpt$1(xOpt, y) { @@ -45,30 +76,40 @@ var partial$1 = Curry._1((function (param) { var total$1 = withOpt$1(10, 3)(4, 11); -function foo1(xOpt, y) { +function foo1$1(xOpt, y) { var x = xOpt !== undefined ? xOpt : 3; return x + y | 0; } -var x = 3; +var x$1 = 3; -var r1 = x + 11 | 0; +var r1$1 = x$1 + 11 | 0; -function foo2(y, xOpt, zOpt) { +function foo2$1(y, xOpt, zOpt) { var x = xOpt !== undefined ? xOpt : 3; var z = zOpt !== undefined ? zOpt : 4; return (x + y | 0) + z | 0; } -var r2 = foo2(11, undefined, undefined); +var r2$1 = foo2$1(11, undefined, undefined); + +function foo3$1(xOpt, yOpt) { + var x = xOpt !== undefined ? xOpt : 3; + var y = yOpt !== undefined ? yOpt : 4; + return x + y | 0; +} + +var r3$1 = foo3$1(undefined, undefined); exports.StandardNotation = StandardNotation; exports.withOpt = withOpt$1; exports.testWithOpt = testWithOpt$1; exports.partial = partial$1; exports.total = total$1; -exports.foo1 = foo1; -exports.r1 = r1; -exports.foo2 = foo2; -exports.r2 = r2; +exports.foo1 = foo1$1; +exports.r1 = r1$1; +exports.foo2 = foo2$1; +exports.r2 = r2$1; +exports.foo3 = foo3$1; +exports.r3 = r3$1; /* testWithOpt Not a pure module */ diff --git a/jscomp/test/uncurried_default.args.res b/jscomp/test/uncurried_default.args.res index 7577f75538..afe81ee308 100644 --- a/jscomp/test/uncurried_default.args.res +++ b/jscomp/test/uncurried_default.args.res @@ -3,6 +3,15 @@ module StandardNotation = { let testWithOpt = withOpt(. 3)(. 4) let partial = withOpt(~x=10)(3)(~z=4)(11) let total = withOpt(. ~x=10, 3)(. ~z=4, 11) + + let foo1 = (. ~x=3, ~y) => x+y + let r1 = foo1(. ~y=11) + + let foo2 = (. ~y, ~x=3, ~z=4) => x+y+z + let r2 = foo2(. ~y=11) + + let foo3 = (. ~x=3, ~y=4) => x+y + let r3 = foo3(. ) } @@uncurried @@ -19,3 +28,6 @@ let r1 = foo1(~y=11) let foo2 = (~y, ~x=3, ~z=4) => x+y+z let r2 = foo2(~y=11) + +let foo3 = (~x=3, ~y=4) => x+y +let r3 = foo3() From b89160e795d03773e852eecf40baf2d7d7e64518 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 13 Jan 2023 12:26:55 +0100 Subject: [PATCH 4/4] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b32515f98..17181fdfe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ subset of the arguments, and return a curried type with the remaining ones https - Add support for default arguments in uncurried functions https://github.com/rescript-lang/rescript-compiler/pull/5835 - Inline uncurried application when it is safe https://github.com/rescript-lang/rescript-compiler/pull/5847 - Add support for toplevel `await` https://github.com/rescript-lang/rescript-compiler/pull/5940 -- Allow default arguments in uncurried functions without a final unit argument https://github.com/rescript-lang/rescript-compiler/pull/5907 +- Support optional named arguments without a final unit in uncurried functions https://github.com/rescript-lang/rescript-compiler/pull/5907 #### :boom: Breaking Change