fix(grainfmt): Add parentheses around some binops for precedence clarity (#1514)

Author: marcusroberts

compiler/src/formatting/format.re

@@ -28,6 +28,55 @@ type expression_parent_type =
 
 let exception_primitives = [|"throw", "fail", "assert"|];
 
+let is_shift_or_concat_op = fn =>
+  if (String.length(fn) > 1) {
+    switch (String.sub(fn, 0, 2)) {
+    | "<<"
+    | ">>"
+    | "++"
+    | "||" => true
+    | _ => false
+    };
+  } else {
+    false;
+  };
+
+let is_logic_op = fn =>
+  if (String.length(fn) > 1) {
+    switch (String.sub(fn, 0, 2)) {
+    | "<="
+    | ">="
+    | "=="
+    | "!="
+    | "is"
+    | "&&"
+    | "||" => true
+    | _ => false
+    };
+  } else {
+    false;
+  };
+let is_math_op = fn =>
+  if (is_logic_op(fn) || is_shift_or_concat_op(fn)) {
+    false;
+  } else if (String.length(fn) > 0) {
+    switch (fn.[0]) {
+    | '*'
+    | '/'
+    | '%'
+    | '+'
+    | '-'
+    | '<'
+    | '>'
+    | '&'
+    | '^'
+    | '|' => true
+    | _ => false
+    };
+  } else {
+    false;
+  };
+
 let op_precedence = fn => {
   let op_precedence = fn =>
     switch (fn) {
@@ -1871,6 +1920,7 @@ and print_infix_application =
         this_prec < parent_prec || child_name != function_name;
       | _ => true
       };
+
     let right_is_leaf =
       switch (second.pexp_desc) {
       | PExpApp(fn, expr) =>
@@ -1901,7 +1951,23 @@ and print_infix_application =
       | _ => false
       };
 
-    let left_needs_parens = left_is_if || left_grouping_required;
+    // Put parens around different operators for clarity, except
+    // math and logic operations where precedence is well-known
+    let left_is_different_op =
+      switch (first.pexp_desc) {
+      | PExpApp(fn1, _) =>
+        let fn = get_function_name(fn1);
+        if (infixop(fn)) {
+          (!is_math_op(function_name) && !is_logic_op(function_name))
+          && fn != function_name;
+        } else {
+          false;
+        };
+      | _ => false
+      };
+
+    let left_needs_parens =
+      left_is_if || left_grouping_required || left_is_different_op;
     let right_needs_parens = right_is_if || right_grouping_required;
 
     let lhs =

compiler/test/formatter_inputs/binops.gr

@@ -11,3 +11,34 @@ let x= fofdfdfdfdfdfdfdfdfdfdfdo &&
 if (fofdfdfdfdfdfdfdfdfdfdfdo &&
   bafdfdfdfdfdfdddefdfdfdfdfdr &&
   badfdfdfdfdfdfdffdffdfdfdz) true
+
+let a = (2 >> 1) << 1
+
+let a1 = ((2 >> 1) << 1) >> 1
+
+let x = 1  * (2 +3)
+let y = 1 * 2 + 3
+let cond = false && false || false
+
+let z = 1 + 2 + 3
+
+let p = 1 + 2 - 3
+
+let p2 = 1 - 2 + 3
+
+let y = 1 / 2 + 3
+
+let ya = 1 / (2 + 3)
+
+let y1 = 1 + 2 /3
+
+let c = (1 < 2) || (2<3) || (3<4)
+
+let (>>>>>) = (>>)
+
+let a = (2 >>>>> 1) << 1
+
+let (+++) = (+)
+let (---) = (-)
+
+let zz = 1 +++ 2 --- 3

compiler/test/formatter_outputs/binops.gr

@@ -11,3 +11,34 @@ if (
   bafdfdfdfdfdfdddefdfdfdfdfdr &&
   badfdfdfdfdfdfdffdffdfdfdz
 ) true
+
+let a = (2 >> 1) << 1
+
+let a1 = ((2 >> 1) << 1) >> 1
+
+let x = 1 * (2 + 3)
+let y = 1 * 2 + 3
+let cond = false && false || false
+
+let z = 1 + 2 + 3
+
+let p = 1 + 2 - 3
+
+let p2 = 1 - 2 + 3
+
+let y = 1 / 2 + 3
+
+let ya = 1 / (2 + 3)
+
+let y1 = 1 + 2 /3
+
+let c = 1 < 2 || 2 < 3 || 3 < 4
+
+let (>>>>>) = (>>)
+
+let a = (2 >>>>> 1) << 1
+
+let (+++) = (+)
+let (---) = (-)
+
+let zz = 1 +++ 2 --- 3

stdlib/runtime/bigint.gr

@@ -1339,7 +1339,7 @@ export let shl = (num: WasmI32, places: WasmI32) => {
   let (&) = WasmI64.and
   let a = places / 32n
   let b = places % 32n
-  let mask = (1N << WasmI64.extendI32U(b)) - 1N << 64N - WasmI64.extendI32U(b)
+  let mask = ((1N << WasmI64.extendI32U(b)) - 1N) << 64N - WasmI64.extendI32U(b)
   let result = init(numLimbs + a)
   setFlag(result, _IS_NEGATIVE, getFlag(num, _IS_NEGATIVE))
   let numHalfLimbs = WasmI32.shl(numLimbs, 1n)
@@ -1350,7 +1350,7 @@ export let shl = (num: WasmI32, places: WasmI32) => {
     setHalfLimb(
       result,
       i + a,
-      WasmI32.wrapI64(acc << WasmI64.extendI32U(b) >> 32N)
+      WasmI32.wrapI64((acc << WasmI64.extendI32U(b)) >> 32N)
     )
   }
   let (>) = WasmI64.gtU

stdlib/runtime/numberUtils.gr

@@ -1277,7 +1277,7 @@ let genDigits = (buffer, w_frc, mp_frc, mp_exp, delta, sign) => {
         wp_w_frc = WasmI64.mul(
           wp_w_frc,
           WasmI64.extendI32U(
-            WasmI32.load(get_POWERS10() + (0n - kappa << 2n), 0n)
+            WasmI32.load(get_POWERS10() + ((0n - kappa) << 2n), 0n)
           )
         )
         grisuRound(buffer, len, delta, p2, one_frc, wp_w_frc)