Fix argument evaluation order under function inlining

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

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

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,

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 */

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<string> = []
+
+let equalish = (a: array<int>, b: array<int>) => a == b
+let copy = (a: array<int>) => 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))
+}