Address review comments

Author: cknitt

compiler/core/lam_util.ml

@@ -45,13 +45,11 @@ let refine_let ~kind param (arg : Lam.t) (l : Lam.t) : Lam.t =
     | Lam_primitive.Pmakeblock _ -> true
     | _ -> false
   in
-  (* Helper for spotting RHS expressions that are safe to inline everywhere.
-     These are the obviously pure shapes where substituting the original value
-     cannot introduce extra work or side effects. *)
-  let is_alias_candidate (lam : Lam.t) =
+  let is_safe_to_inline (lam : Lam.t) =
     match lam with
     | Lvar _ | Lconst _ -> true
-    | Lprim { primitive = Psome_not_nest; _ } -> true
+    | Lprim { primitive = Psome_not_nest; args = [inner]; _ } ->
+        Lam_analysis.no_side_effects inner
     | Lprim { primitive = Pfield (_, Fld_module _); args = [ (Lglobal_module _ | Lvar _) ]; _ } -> true
     | _ -> false
   in
@@ -74,7 +72,7 @@ let refine_let ~kind param (arg : Lam.t) (l : Lam.t) : Lam.t =
          rewrite to `someFn(value)` as long as the callee does not capture `x`.
          This removes the temporary binding while preserving the call semantics. *)
       Lam.apply fn [arg] ap_info ~ap_transformed_jsx
-  | (Strict | StrictOpt), arg, _ when is_alias_candidate arg ->
+  | (Strict | StrictOpt), arg, _ when is_safe_to_inline arg ->
       (* `Strict` and `StrictOpt` bindings both evaluate the RHS immediately
          (with `StrictOpt` allowing later elimination if unused). When that RHS
          is pure — `{ let x = Some(value); ... }`, `{ let x = 3; ... }`, or a module

packages/@rescript/runtime/lib/es6/Belt_internalAVLset.js

@@ -741,13 +741,15 @@ function rotateWithRightChild(k1) {
 
 function doubleWithLeftChild(k3) {
   let k3l = k3.l;
-  k3.l = rotateWithRightChild(k3l);
+  let v = rotateWithRightChild(k3l);
+  k3.l = v;
   return rotateWithLeftChild(k3);
 }
 
 function doubleWithRightChild(k2) {
   let k2r = k2.r;
-  k2.r = rotateWithLeftChild(k2r);
+  let v = rotateWithLeftChild(k2r);
+  k2.r = v;
   return rotateWithRightChild(k2);
 }
 

packages/@rescript/runtime/lib/js/Belt_internalAVLset.js

@@ -741,13 +741,15 @@ function rotateWithRightChild(k1) {
 
 function doubleWithLeftChild(k3) {
   let k3l = k3.l;
-  k3.l = rotateWithRightChild(k3l);
+  let v = rotateWithRightChild(k3l);
+  k3.l = v;
   return rotateWithLeftChild(k3);
 }
 
 function doubleWithRightChild(k2) {
   let k2r = k2.r;
-  k2.r = rotateWithLeftChild(k2r);
+  let v = rotateWithLeftChild(k2r);
+  k2.r = v;
   return rotateWithRightChild(k2);
 }
 

tests/tests/src/option_wrapping_test.mjs

@@ -12,37 +12,10 @@ let x10 = null;
 
 let x11 = Primitive_option.some(undefined);
 
-let x1 = "hello";
-
-let x2 = 1;
-
-let x3 = {
-  TAG: "Ok",
-  _0: "hi"
-};
-
-let x4 = "polyvar";
-
-let x5 = {
-  x: 42
-};
-
-let x7 = [
-  1,
-  2,
-  3
-];
-
-let x8 = () => {};
-
-let x12 = "test";
-
 let x20 = null;
 
 let x21 = new Date();
 
-let x22 = /test/;
-
 let x23 = new Map();
 
 let x24 = new Set();
@@ -75,8 +48,6 @@ let x37 = new Intl.DateTimeFormat();
 
 let x38 = new Intl.NumberFormat();
 
-let x39 = true;
-
 let x40 = new Intl.Collator();
 
 let x41 = new Intl.RelativeTimeFormat();
@@ -85,19 +56,48 @@ let x42 = new Intl.PluralRules();
 
 let x43 = new Intl.Locale("en");
 
+let x45 = Promise.resolve(true);
+
+let x48 = Stdlib_Lazy.make(() => true);
+
+let x1 = "hello";
+
+let x2 = 1;
+
+let x3 = {
+  TAG: "Ok",
+  _0: "hi"
+};
+
+let x4 = "polyvar";
+
+let x5 = {
+  x: 42
+};
+
+let x7 = [
+  1,
+  2,
+  3
+];
+
+let x8 = () => {};
+
+let x12 = "test";
+
+let x22 = /test/;
+
+let x39 = true;
+
 let x44 = [
   1,
   2
 ];
 
-let x45 = Promise.resolve(true);
-
 let x46 = /* [] */0;
 
 let x47 = {};
 
-let x48 = Stdlib_Lazy.make(() => true);
-
 export {
   x1,
   x2,