refactor pattern matching to favour first appearing variable

Author: giacomocavalieri

compiler-core/src/exhaustiveness.rs

@@ -2748,27 +2748,29 @@ fn find_pivot_check(
         }
     }
 
-    // Maximize ( -branching_factor, references ).
+    // We want to picke the variable that has the smallest branching factor
+    // possible, this tends to yield smaller, shallower decision trees.
+    // So, for example, we will pick a list (branching factor of 2 `[]` and
+    // `[_, ..]`) over an integer (infinitely many choices).
     //
-    // - branching_factor: prefer the split that creates the FEWEST switch arms.
-    //   Fewer children tends to yield smaller, shallower decision trees.
-    //
-    // - references: break ties by choosing the subject that appears in the most
-    //   clauses (i.e. a test that will pay off for more rows).
+    // In case two variables are tied we break the tie by choosing the subject
+    // that appears in the most clauses (i.e. a test that will pay off for more
+    // rows).
     //
     // Both parts mirror standard guidance for good match trees (small branching
-    // factor; prioritize columns that are widely useful).
-    //
+    // factor; prioritize columns that are widely useful):
     // https://www.cs.tufts.edu/~nr/cs257/archive/luc-maranget/jun08.pdf
     first_branch
         .checks
         .iter()
-        .max_by_key(|check| {
-            let mode = check.var.branch_mode(env);
-            let branching_factor = mode.branching_factor();
+        .min_by_key(|check| {
+            let branching_factor = check.var.branch_mode(env).branching_factor();
             let references = var_references.get(&check.var.id).cloned().unwrap_or(0);
 
-            (usize::MAX - branching_factor, references)
+            // Notice how we're using `-references`: we want to favour the one
+            // that has the most references, so the one were `-references` is
+            // the smallest.
+            (branching_factor, -references)
         })
         .cloned()
 }

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__assignments__assert1.snap

@@ -11,10 +11,10 @@ import { makeError } from "../gleam.mjs";
 const FILEPATH = "src/module.gleam";
 
 export function go(x) {
-  let $ = x[1];
-  if ($ === 2) {
-    let $1 = x[0];
-    if (!($1 === 1)) {
+  let $ = x[0];
+  if ($ === 1) {
+    let $1 = x[1];
+    if (!($1 === 2)) {
       throw makeError(
         "let_assert",
         FILEPATH,

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__assignments__nested_binding.snap

@@ -19,10 +19,10 @@ export function go(x) {
   let t;
   let b;
   let c;
-  let $ = x[1][2];
-  if ($ === 2) {
-    let $1 = x[3];
-    if ($1 === 1) {
+  let $ = x[3];
+  if ($ === 1) {
+    let $1 = x[1][2];
+    if ($1 === 2) {
       a = x[0];
       t = x[1];
       b = x[1][0];

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__assignments__tuple_matching.snap

@@ -15,10 +15,10 @@ import { makeError } from "../gleam.mjs";
 const FILEPATH = "src/module.gleam";
 
 export function go(x) {
-  let $ = x[1];
-  if ($ === 2) {
-    let $1 = x[0];
-    if (!($1 === 1)) {
+  let $ = x[0];
+  if ($ === 1) {
+    let $1 = x[1];
+    if (!($1 === 2)) {
       throw makeError(
         "let_assert",
         FILEPATH,

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__bit_arrays__tuple_multiple_bit_arrays.snap

@@ -15,17 +15,18 @@ import { makeError } from "../gleam.mjs";
 const FILEPATH = "src/module.gleam";
 
 export function go(x) {
-  let $ = x[2];
-  if (
-    $.bitSize >= 8 &&
-    $.byteAt(0) === 2 &&
-    $.bitSize === 16 &&
-    $.byteAt(1) === 3
-  ) {
+  let $ = x[0];
+  if ($.bitSize === 0) {
     let $1 = x[1];
-    if ($1.bitSize === 8 && $1.byteAt(0) === 1) {
-      let $2 = x[0];
-      if (!($2.bitSize === 0)) {
+    if ($1.bitSize === 8) {
+      let $2 = x[2];
+      if (!(
+        $2.bitSize >= 8 &&
+        $1.byteAt(0) === 1 &&
+        $2.byteAt(0) === 2 &&
+        $2.bitSize === 16 &&
+        $2.byteAt(1) === 3
+      )) {
         throw makeError(
           "let_assert",
           FILEPATH,

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__bit_arrays__tuple_multiple_bit_arrays_case.snap

@@ -14,17 +14,18 @@ pub fn go(x) {
 
 ----- COMPILED JAVASCRIPT
 export function go(x) {
-  let $ = x[2];
-  if (
-    $.bitSize >= 8 &&
-    $.byteAt(0) === 2 &&
-    $.bitSize === 16 &&
-    $.byteAt(1) === 3
-  ) {
+  let $ = x[0];
+  if ($.bitSize === 0) {
     let $1 = x[1];
-    if ($1.bitSize === 8 && $1.byteAt(0) === 1) {
-      let $2 = x[0];
-      if ($2.bitSize === 0) {
+    if ($1.bitSize === 8) {
+      let $2 = x[2];
+      if (
+        $2.bitSize >= 8 &&
+        $1.byteAt(0) === 1 &&
+        $2.byteAt(0) === 2 &&
+        $2.bitSize === 16 &&
+        $2.byteAt(1) === 3
+      ) {
         return true;
       } else {
         return false;

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__case__case_with_multiple_subjects_building_list_matched_by_pattern.snap

@@ -25,23 +25,29 @@ export function go(n, x) {
   } else {
     let $ = x.tail;
     if ($ instanceof $Empty) {
-      let $1 = x.head;
-      if ($1 === 1 && n === 3) {
-        return x;
+      if (n === 3) {
+        let $1 = x.head;
+        if ($1 === 1) {
+          return x;
+        } else {
+          return x;
+        }
       } else {
         return x;
       }
     } else {
       let $1 = $.tail;
       if ($1 instanceof $Empty) {
         return x;
-      } else {
+      } else if (n === 3) {
         let $2 = x.head;
-        if ($2 === 1 && n === 3) {
+        if ($2 === 1) {
           return x;
         } else {
           return x;
         }
+      } else {
+        return x;
       }
     }
   }

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__case__case_with_multiple_subjects_building_record_matched_by_pattern.snap

@@ -15,19 +15,19 @@ pub fn go(x, y) {
 import { Ok, Error } from "../gleam.mjs";
 
 export function go(x, y) {
-  if (y instanceof Ok) {
-    if (x instanceof Error) {
-      return y;
-    } else {
-      return new Error(undefined);
-    }
-  } else if (x instanceof Ok) {
-    let $ = x[0];
-    if ($ === 1) {
-      return x;
+  if (x instanceof Ok) {
+    if (y instanceof Error) {
+      let $ = x[0];
+      if ($ === 1) {
+        return x;
+      } else {
+        return new Error(undefined);
+      }
     } else {
       return new Error(undefined);
     }
+  } else if (y instanceof Ok) {
+    return y;
   } else {
     return new Error(undefined);
   }

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__case__case_with_multiple_subjects_building_same_value_as_two_subjects_one_is_picked.snap

@@ -21,9 +21,9 @@ import { Ok, Error } from "../gleam.mjs";
 export function go(x, y) {
   if (y instanceof Ok) {
     if (x instanceof $gleam.Ok) {
-      let $ = x[0];
+      let $ = y[0];
       if ($ === 1) {
-        let $1 = y[0];
+        let $1 = x[0];
         if ($1 === 1) {
           return x;
         } else {

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__case__multi_subject_catch_all.snap

@@ -1,8 +1,6 @@
 ---
 source: compiler-core/src/javascript/tests/case.rs
-assertion_line: 95
 expression: "\npub fn go(x, y) {\n  case x, y {\n    True, True -> 1\n    _, _ -> 0\n  }\n}\n"
-snapshot_kind: text
 ---
 ----- SOURCE CODE
 
@@ -16,7 +14,7 @@ pub fn go(x, y) {
 
 ----- COMPILED JAVASCRIPT
 export function go(x, y) {
-  if (y && x) {
+  if (x && y) {
     return 1;
   } else {
     return 0;