From 44d6fd9091985b94a0a83edacd560a2a1bba4237 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Wed, 18 Jun 2025 00:17:29 -0700 Subject: [PATCH 1/3] test: show regression with `const` assignment to function variables --- .../function-assignment-const.t | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 test/blackbox-tests/function-assignment-const.t diff --git a/test/blackbox-tests/function-assignment-const.t b/test/blackbox-tests/function-assignment-const.t new file mode 100644 index 000000000..02309935d --- /dev/null +++ b/test/blackbox-tests/function-assignment-const.t @@ -0,0 +1,52 @@ + + $ . ./setup.sh + $ cat >dune-project < (lang dune 3.8) + > (using melange 0.1) + > EOF + + $ cat > main.ml < let f x = + > let rec aux x k = + > match x with + > | 0 -> k 0 + > | x -> aux (x - 1) (fun x -> x) + > in + > aux x (fun x -> x) + > let _: int = f 3 + > EOF + + $ melc main.ml | tee main.js + // Generated by Melange + 'use strict'; + + const Curry = require("melange.js/curry.js"); + + function f(x) { + let _x = x; + const _k = function (x) { + return x; + }; + while (true) { + const k = _k; + const x$1 = _x; + if (x$1 === 0) { + return Curry._1(k, 0); + } + _k = (function (x) { + return x; + }); + _x = x$1 - 1 | 0; + continue; + }; + } + + f(3); + + module.exports = { + f, + } + /* Not a pure module */ + $ node ./main.js 2>&1 | grep -i typeerror + TypeError: Assignment to constant variable. + From d7b64896787556e013ba3659d468b77f6e235e90 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Wed, 18 Jun 2025 00:26:28 -0700 Subject: [PATCH 2/3] fix: account for `Lam_group.let_kind` when assigning functions to variables --- jscomp/core/js_dump.ml | 21 ++++++++------- jscomp/test/dist/jscomp/test/bench.js | 6 ++--- .../dist/jscomp/test/earger_curry_test.js | 6 ++--- .../dist/jscomp/test/ocaml_parsetree_test.js | 6 ++--- jscomp/test/dist/jscomp/test/ocaml_re_test.js | 2 +- .../dist/jscomp/test/ocaml_typedtree_test.js | 26 +++++++++---------- jscomp/test/dist/jscomp/test/qcc.js | 2 +- jscomp/test/dist/jscomp/test/sexpm.js | 2 +- .../function-assignment-const.t | 4 +-- 9 files changed, 38 insertions(+), 37 deletions(-) diff --git a/jscomp/core/js_dump.ml b/jscomp/core/js_dump.ml index 5d5a9569e..b75170849 100644 --- a/jscomp/core/js_dump.ml +++ b/jscomp/core/js_dump.ml @@ -323,8 +323,8 @@ let is_var (b : J.expression) a = type fn_exp_state = | Is_return (* for sure no name *) - | Name_top of Ident.t - | Name_non_top of Ident.t + | Name_top of { name : Ident.t; property : Lam_group.let_kind } + | Name_non_top of { name : Ident.t; property : Lam_group.let_kind } | No_name of { single_arg : bool } (* true means for sure, false -- not sure *) @@ -401,8 +401,8 @@ and pp_function ~return_unit ~is_method cxt ~fn_state (l : Ident.t list) let len = List.length l in (* length *) match fn_state with - | Name_top i | Name_non_top i -> - let cxt = pp_const_assign cxt i in + | Name_top { name = i; property } | Name_non_top { name = i; property } -> + let cxt = pp_assign ~property cxt i in let cxt = optimize len ~p:(arity = NA && len <= 8) cxt v in semi cxt; cxt @@ -414,7 +414,8 @@ and pp_function ~return_unit ~is_method cxt ~fn_state (l : Ident.t list) (* identifiers will be printed following*) match fn_state with | Is_return | No_name _ -> Js_fun_env.get_unbounded env - | Name_top id | Name_non_top id -> + | Name_top { name = id; property = _ } + | Name_non_top { name = id; property = _ } -> Ident.Set.add (Js_fun_env.get_unbounded env) id in (* the context will be continued after this function *) @@ -462,13 +463,13 @@ and pp_function ~return_unit ~is_method cxt ~fn_state (l : Ident.t list) string cxt L.function_; space cxt; param_body ()) - | Name_non_top x -> - ignore (pp_const_assign inner_cxt x : cxt); + | Name_non_top { name = x; property } -> + ignore (pp_assign ~property inner_cxt x : cxt); string cxt L.function_; space cxt; param_body (); semi cxt - | Name_top x -> + | Name_top { name = x; property = _ } -> string cxt L.function_; space cxt; ignore (ident inner_cxt x : cxt); @@ -961,7 +962,9 @@ and variable_declaration top cxt (variable : J.variable_declaration) : cxt = match e.expression_desc with | Fun { method_ = is_method; params; body = b; env; return_unit } -> pp_function ~return_unit ~is_method cxt - ~fn_state:(if top then Name_top name else Name_non_top name) + ~fn_state: + (if top then Name_top { name; property } + else Name_non_top { name; property }) params b env | _ -> let cxt = pp_assign ~property cxt name in diff --git a/jscomp/test/dist/jscomp/test/bench.js b/jscomp/test/dist/jscomp/test/bench.js index d27128e50..de8d58f82 100644 --- a/jscomp/test/dist/jscomp/test/bench.js +++ b/jscomp/test/dist/jscomp/test/bench.js @@ -7,7 +7,7 @@ const Curry = require("melange.js/curry.js"); const Stdlib = require("melange/stdlib.js"); function map(f, a) { - const f$1 = Curry.__1(f); + let f$1 = Curry.__1(f); const l = a.length; if (l === 0) { return []; @@ -20,7 +20,7 @@ function map(f, a) { } function init(l, f) { - const f$1 = Curry.__1(f); + let f$1 = Curry.__1(f); if (l === 0) { return []; } @@ -38,7 +38,7 @@ function init(l, f) { } function fold_left(f, x, a) { - const f$1 = Curry.__2(f); + let f$1 = Curry.__2(f); let r = x; for (let i = 0, i_finish = a.length; i < i_finish; ++i) { r = f$1(r, a[i]); diff --git a/jscomp/test/dist/jscomp/test/earger_curry_test.js b/jscomp/test/dist/jscomp/test/earger_curry_test.js index 4fc5042f2..5dd4a87b7 100644 --- a/jscomp/test/dist/jscomp/test/earger_curry_test.js +++ b/jscomp/test/dist/jscomp/test/earger_curry_test.js @@ -8,7 +8,7 @@ const Mt = require("./mt.js"); const Stdlib = require("melange/stdlib.js"); function map(f, a) { - const f$1 = Curry.__1(f); + let f$1 = Curry.__1(f); const l = a.length; if (l === 0) { return []; @@ -21,7 +21,7 @@ function map(f, a) { } function init(l, f) { - const f$1 = Curry.__1(f); + let f$1 = Curry.__1(f); if (l === 0) { return []; } @@ -39,7 +39,7 @@ function init(l, f) { } function fold_left(f, x, a) { - const f$1 = Curry.__2(f); + let f$1 = Curry.__2(f); let r = x; for (let i = 0, i_finish = a.length; i < i_finish; ++i) { r = f$1(r, a[i]); diff --git a/jscomp/test/dist/jscomp/test/ocaml_parsetree_test.js b/jscomp/test/dist/jscomp/test/ocaml_parsetree_test.js index 0de06d12f..556a338f6 100644 --- a/jscomp/test/dist/jscomp/test/ocaml_parsetree_test.js +++ b/jscomp/test/dist/jscomp/test/ocaml_parsetree_test.js @@ -1897,7 +1897,7 @@ function errorf(locOpt, subOpt, if_highlightOpt, fmt) { const sub = subOpt !== undefined ? subOpt : /* [] */ 0; const if_highlight = if_highlightOpt !== undefined ? if_highlightOpt : ""; let before = print_phanton_error_prefix; - const k = function (msg) { + let k = function (msg) { return { loc: loc, msg: msg, @@ -13108,10 +13108,10 @@ function token$1(lexbuf) { switch (doc) { case /* SHARP */ 84 : if (at_bol(lexbuf)) { - const cont = function (lexbuf) { + let cont = function (lexbuf) { return loop(lines, docs, lexbuf); }; - const look_ahead = function (token) { + let look_ahead = function (token) { sharp_look_ahead.contents = token; return /* SHARP */ 84; }; diff --git a/jscomp/test/dist/jscomp/test/ocaml_re_test.js b/jscomp/test/dist/jscomp/test/ocaml_re_test.js index e459230cd..764173c09 100644 --- a/jscomp/test/dist/jscomp/test/ocaml_re_test.js +++ b/jscomp/test/dist/jscomp/test/ocaml_re_test.js @@ -1782,7 +1782,7 @@ function is_charset(_param) { function split(s, cm) { let _t = s; - const f = function (i, j) { + let f = function (i, j) { Caml_bytes.set(cm, i, /* '\001' */1); Caml_bytes.set(cm, j + 1 | 0, /* '\001' */1); }; diff --git a/jscomp/test/dist/jscomp/test/ocaml_typedtree_test.js b/jscomp/test/dist/jscomp/test/ocaml_typedtree_test.js index cb89f5408..cdde1de99 100644 --- a/jscomp/test/dist/jscomp/test/ocaml_typedtree_test.js +++ b/jscomp/test/dist/jscomp/test/ocaml_typedtree_test.js @@ -2176,7 +2176,7 @@ function errorf(locOpt, subOpt, if_highlightOpt, fmt) { const sub = subOpt !== undefined ? subOpt : /* [] */ 0; const if_highlight = if_highlightOpt !== undefined ? if_highlightOpt : ""; let before = print_phanton_error_prefix; - const k = function (msg) { + let k = function (msg) { return { loc: loc, msg: msg, @@ -11298,10 +11298,10 @@ function find_name$1(s, tbl) { function fold_name(f) { return function (param, param$1) { - const f$1 = function (k, param) { + let f$1 = function (k, param) { return Curry._2(f, k, param[0]); }; - const f$2 = function (k) { + let f$2 = function (k) { return Curry._2(f$1, k.ident, k.data); }; let _stack = /* [] */ 0; @@ -12879,10 +12879,10 @@ function run_iter_cont(l) { function iter_types(f) { return function (param, param$1) { - const proj1 = function (env) { + let proj1 = function (env) { return env.types; }; - const proj2 = function (sc) { + let proj2 = function (sc) { return sc.comp_types; }; iter((function (id, param) { @@ -25175,10 +25175,10 @@ function token$1(lexbuf) { switch (doc) { case /* SHARP */ 84 : if (at_bol(lexbuf)) { - const cont = function (lexbuf) { + let cont = function (lexbuf) { return loop(lines, docs, lexbuf); }; - const look_ahead = function (token) { + let look_ahead = function (token) { sharp_look_ahead.contents = token; return /* SHARP */ 84; }; @@ -42575,7 +42575,7 @@ function print_out_sig_item(ppf, param) { }, _1: " =%a {%a@;<1 -2>}" }), print_private, td.otype_private, (function (param, param$1) { - const sep = function (ppf) { + let sep = function (ppf) { Stdlib__Format.fprintf(ppf)({ TAG: /* Format */ 0, _0: { @@ -65328,7 +65328,7 @@ function partial_pred(lev, env, expected_ty, constrs, labels, p) { function check_partial$1(levOpt, env, expected_ty) { const lev = levOpt !== undefined ? levOpt : current_level.contents; return function (param, param$1) { - const pred = function (param, param$1, param$2) { + let pred = function (param, param$1, param$2) { return partial_pred(lev, env, expected_ty, param, param$1, param$2); }; const first_check = check_partial(param, param$1); @@ -70213,7 +70213,7 @@ function type_let(checkOpt, check_strictOpt, env, rec_flag, spat_sexp_list, scop }); } - const callback = function (param) { + let callback = function (param) { const slot$1 = current_slot.contents; if (slot$1 !== undefined) { slot$1.contents = { @@ -71350,7 +71350,7 @@ register_error_of_exn(function (err) { const name = kind$1 === "record" ? "field" : "constructor"; let param$3 = param$1._2; let tpl = param$1._3; - const txt1 = function (ppf) { + let txt1 = function (ppf) { Curry._4(Stdlib__Format.fprintf(ppf)({ TAG: /* Format */ 0, _0: { @@ -71393,7 +71393,7 @@ register_error_of_exn(function (err) { _1: "The %s %a@ belongs to the %s type" }), name, longident, lid$2, kind$1); }; - const txt2 = function (ppf) { + let txt2 = function (ppf) { Curry._4(Stdlib__Format.fprintf(ppf)({ TAG: /* Format */ 0, _0: { @@ -71436,7 +71436,7 @@ register_error_of_exn(function (err) { _1: "The %s %a@ belongs to one of the following %s types:" }), name, longident, lid$2, kind$1); }; - const txt3 = function (ppf) { + let txt3 = function (ppf) { Curry._2(Stdlib__Format.fprintf(ppf)({ TAG: /* Format */ 0, _0: { diff --git a/jscomp/test/dist/jscomp/test/qcc.js b/jscomp/test/dist/jscomp/test/qcc.js index 2ac96ad1b..2fc8dbaac 100644 --- a/jscomp/test/dist/jscomp/test/qcc.js +++ b/jscomp/test/dist/jscomp/test/qcc.js @@ -1965,7 +1965,7 @@ function main(param) { partial_arg_0, 0 ]; - const c = function (param) { + let c = function (param) { return block(partial_arg, param); }; let stk = /* [] */ 0; diff --git a/jscomp/test/dist/jscomp/test/sexpm.js b/jscomp/test/dist/jscomp/test/sexpm.js index 3a7749c9a..8e1b2b963 100644 --- a/jscomp/test/dist/jscomp/test/sexpm.js +++ b/jscomp/test/dist/jscomp/test/sexpm.js @@ -359,7 +359,7 @@ function to_chan(oc, t) { } function to_file_seq(filename, seq) { - const f = function (oc) { + let f = function (oc) { return Curry._1(seq, (function (t) { to_chan(oc, t); Caml_io.caml_ml_output_char(oc, /* '\n' */10); diff --git a/test/blackbox-tests/function-assignment-const.t b/test/blackbox-tests/function-assignment-const.t index 02309935d..1786ada5a 100644 --- a/test/blackbox-tests/function-assignment-const.t +++ b/test/blackbox-tests/function-assignment-const.t @@ -24,7 +24,7 @@ function f(x) { let _x = x; - const _k = function (x) { + let _k = function (x) { return x; }; while (true) { @@ -47,6 +47,4 @@ f, } /* Not a pure module */ - $ node ./main.js 2>&1 | grep -i typeerror - TypeError: Assignment to constant variable. From b80fd67da42e9a3ec1d752da57ad68c17af8ad78 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Thu, 19 Jun 2025 00:00:02 -0700 Subject: [PATCH 3/3] changelos --- Changes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changes.md b/Changes.md index d2b4d8c68..c95c2b81b 100644 --- a/Changes.md +++ b/Changes.md @@ -11,6 +11,8 @@ Unreleased - melange.ppx: improve the inference of `@mel.as` in polymorphic variant arguments to `external`s, allow to mix ints and strings in the different variant branches ([#1418](https://github.com/melange-re/melange/pull/1418)) +- Fix code generation bug when assigning functions to variables + ([#1429](https://github.com/melange-re/melange/pull/1429)) 5.1.0-53 2025-03-23 ---------------