From 9deb39986ae7d432b96574d8df6440edb2101360 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 20 Aug 2026 12:11:24 +0200 Subject: [PATCH] Fix argument evaluation order under function inlining Lam_beta_reduce built the bindings for non-substitutable arguments by prepending during fold_left2 (reverse parameter order) and then wrapped the body with fold_right, making the first parameter's binding innermost - so the last argument was evaluated first. Fold left instead, so the first parameter's binding is outermost and arguments evaluate in call order. The reversed order was visible in checked-in output: bs_set_int_test.mjs evaluated setEqArray's second argument before its first. The new inline_arg_order_test fixture pins the order with recursive (hence non-substitutable) effectful arguments; its checked-in JS would show the regression directly. Co-Authored-By: Claude Fable 5 Signed-off-by: Cristiano Calcagno --- CHANGELOG.md | 1 + compiler/core/lam_beta_reduce.ml | 8 +++- tests/belt_tests/src/bs_set_int_test.mjs | 12 ++--- tests/tests/src/inline_arg_order_test.mjs | 55 +++++++++++++++++++++++ tests/tests/src/inline_arg_order_test.res | 30 +++++++++++++ 5 files changed, 98 insertions(+), 8 deletions(-) create mode 100644 tests/tests/src/inline_arg_order_test.mjs create mode 100644 tests/tests/src/inline_arg_order_test.res diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a21252bcff..afce8dd43a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ #### :bug: Bug fix +- Fix argument evaluation order when a function call is inlined: the beta reducer stacked argument bindings in reverse parameter order, so the last argument was evaluated first when arguments could not be substituted directly. https://github.com/rescript-lang/rescript/pull/8572 - Preserve parentheses around multiplication, division, and modulo expressions used as exponents. https://github.com/rescript-lang/rescript/pull/8550 - Enforce function arity in interface/module inclusion and type coercion. Previously a curried implementation (e.g. `int => int => int`) could satisfy an uncurried interface (`(int, int) => int`) or be coerced to it, which could miscompile calls made through the interface type. Such mismatches are now compile errors with an explanatory hint. https://github.com/rescript-lang/rescript/pull/8559 - Fix bare labeled arrow types (`~x: int => string`) getting no arity: they printed identically to their parenthesized form (`(~x: int) => string`) but did not unify with it. https://github.com/rescript-lang/rescript/pull/8563 diff --git a/compiler/core/lam_beta_reduce.ml b/compiler/core/lam_beta_reduce.ml index e7f9842bbc7..10ddae76b68 100644 --- a/compiler/core/lam_beta_reduce.ml +++ b/compiler/core/lam_beta_reduce.ml @@ -63,7 +63,10 @@ let propagate_beta_reduce (meta : Lam_stats.t) (params : Ident.t list) (Hash_ident.of_list2 (List.rev params) rev_new_params) body in - Ext_list.fold_right rest_bindings new_body (fun (param, arg) l -> + (* [rest_bindings] is in reverse parameter order; folding left makes the + first parameter's binding outermost, so arguments evaluate in call + order. *) + Ext_list.fold_left rest_bindings new_body (fun l (param, arg) -> (match arg with | Lprim {primitive = Pmakeblock (_, _, Immutable); args; _} -> Hash_ident.replace meta.ident_tbl param @@ -104,7 +107,8 @@ let propagate_beta_reduce_with_map (meta : Lam_stats.t) (Hash_ident.of_list2 (List.rev params) rev_new_params) body in - Ext_list.fold_right rest_bindings new_body (fun (param, (arg : Lam.t)) l -> + (* See above: fold left so arguments evaluate in call order. *) + Ext_list.fold_left rest_bindings new_body (fun l (param, (arg : Lam.t)) -> (match arg with | Lprim {primitive = Pmakeblock (_, _, Immutable); args} -> Hash_ident.replace meta.ident_tbl param diff --git a/tests/belt_tests/src/bs_set_int_test.mjs b/tests/belt_tests/src/bs_set_int_test.mjs index a14053aa637..b55367bcffb 100644 --- a/tests/belt_tests/src/bs_set_int_test.mjs +++ b/tests/belt_tests/src/bs_set_int_test.mjs @@ -61,23 +61,23 @@ Mocha.describe("Bs_set_int_test", () => { let nr = r; Test_utils.ok("File \"bs_set_int_test.res\", line 40, characters 7-14", Belt_SetInt.eq(match[0], nl)); Test_utils.ok("File \"bs_set_int_test.res\", line 41, characters 7-14", Belt_SetInt.eq(match[1], nr)); - let i$2 = range(50, 100); let s = Belt_SetInt.intersect(Belt_SetInt.fromArray(range(1, 100)), Belt_SetInt.fromArray(range(50, 200))); + let i$2 = range(50, 100); Test_utils.ok("File \"bs_set_int_test.res\", line 44, characters 6-13", Belt_SetInt.eq(Belt_SetInt.fromArray(i$2), s)); - let i$3 = range(1, 200); let s$1 = Belt_SetInt.union(Belt_SetInt.fromArray(range(1, 100)), Belt_SetInt.fromArray(range(50, 200))); + let i$3 = range(1, 200); Test_utils.ok("File \"bs_set_int_test.res\", line 55, characters 6-13", Belt_SetInt.eq(Belt_SetInt.fromArray(i$3), s$1)); - let i$4 = range(1, 49); let s$2 = Belt_SetInt.diff(Belt_SetInt.fromArray(range(1, 100)), Belt_SetInt.fromArray(range(50, 200))); + let i$4 = range(1, 49); Test_utils.ok("File \"bs_set_int_test.res\", line 66, characters 6-13", Belt_SetInt.eq(Belt_SetInt.fromArray(i$4), s$2)); - let i$5 = revRange(50, 100); let s$3 = Belt_SetInt.intersect(Belt_SetInt.fromArray(revRange(1, 100)), Belt_SetInt.fromArray(revRange(50, 200))); + let i$5 = revRange(50, 100); Test_utils.ok("File \"bs_set_int_test.res\", line 77, characters 6-13", Belt_SetInt.eq(Belt_SetInt.fromArray(i$5), s$3)); - let i$6 = revRange(1, 200); let s$4 = Belt_SetInt.union(Belt_SetInt.fromArray(revRange(1, 100)), Belt_SetInt.fromArray(revRange(50, 200))); + let i$6 = revRange(1, 200); Test_utils.ok("File \"bs_set_int_test.res\", line 88, characters 6-13", Belt_SetInt.eq(Belt_SetInt.fromArray(i$6), s$4)); - let i$7 = revRange(1, 49); let s$5 = Belt_SetInt.diff(Belt_SetInt.fromArray(revRange(1, 100)), Belt_SetInt.fromArray(revRange(50, 200))); + let i$7 = revRange(1, 49); Test_utils.ok("File \"bs_set_int_test.res\", line 99, characters 6-13", Belt_SetInt.eq(Belt_SetInt.fromArray(i$7), s$5)); let ss = [ 1, diff --git a/tests/tests/src/inline_arg_order_test.mjs b/tests/tests/src/inline_arg_order_test.mjs new file mode 100644 index 00000000000..400d34ce691 --- /dev/null +++ b/tests/tests/src/inline_arg_order_test.mjs @@ -0,0 +1,55 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as Primitive_object from "@rescript/runtime/lib/es6/Primitive_object.mjs"; + +let recorded = []; + +let equalish = Primitive_object.equal; + +function copy(a) { + return a.slice(); +} + +function helper(x, y) { + return Primitive_object.equal(x, y.slice()); +} + +function effA(_n) { + while (true) { + let n = _n; + recorded.push("a"); + if (n <= 0) { + return [n]; + } + _n = n - 1 | 0; + continue; + }; +} + +function effB(_n) { + while (true) { + let n = _n; + recorded.push("b"); + if (n <= 0) { + return [n]; + } + _n = n - 1 | 0; + continue; + }; +} + +let x = effA(0); + +let y = effB(0); + +Primitive_object.equal(x, y.slice()); + +export { + recorded, + equalish, + copy, + helper, + effA, + effB, +} +/* x Not a pure module */ diff --git a/tests/tests/src/inline_arg_order_test.res b/tests/tests/src/inline_arg_order_test.res new file mode 100644 index 00000000000..89818997914 --- /dev/null +++ b/tests/tests/src/inline_arg_order_test.res @@ -0,0 +1,30 @@ +// The beta reducer used to stack inlined-call argument bindings in reverse +// parameter order, so the last argument was evaluated first. The checked-in +// JS pins evaluation to source order: effA runs before effB. +let recorded: array = [] + +let equalish = (a: array, b: array) => a == b +let copy = (a: array) => Array.copy(a) +let helper = (x, y) => equalish(x, copy(y)) + +let rec effA = n => { + recorded->Array.push("a") + if n > 0 { + effA(n - 1) + } else { + [n] + } +} + +let rec effB = n => { + recorded->Array.push("b") + if n > 0 { + effB(n - 1) + } else { + [n] + } +} + +let _ = { + helper(effA(0), effB(0)) +}