Fix #4278

Author: abs0luty

compiler-core/src/exhaustiveness.rs

@@ -787,6 +787,20 @@ enum BranchMode {
 }
 
 impl BranchMode {
+    /// Returns a heuristic estimate of the branching factor.
+    ///
+    /// This value is used by the pivot-selection to prefer splits
+    /// with fewer branches, which tends to produce smaller and shallower
+    /// decision trees.
+    fn branching_factor(&self) -> usize {
+        match self {
+            BranchMode::Infinite => usize::MAX,
+            BranchMode::Tuple { elements } => elements.len(),
+            BranchMode::List { .. } => 2,
+            BranchMode::NamedType { constructors, .. } => constructors.len(),
+        }
+    }
+
     fn is_infinite(&self) -> bool {
         match self {
             BranchMode::Infinite => true,
@@ -2250,7 +2264,7 @@ impl<'a> Compiler<'a> {
         // the first branch, and use the variable it's pattern matching on to create
         // a new node in the tree. All the branches will be split into different
         // possible paths of this tree.
-        match find_pivot_check(first_branch, &branches) {
+        match find_pivot_check(first_branch, &branches, self.environment) {
             Some(PatternCheck { var, .. }) => self.split_and_compile_with_pivot_var(var, branches),
 
             // If the branch has no remaining checks, it means that we've moved all
@@ -2713,7 +2727,11 @@ impl<'a> Compiler<'a> {
 /// Returns a pattern check from `first_branch` to be used as a pivot to split all
 /// the `branches`.
 ///
-fn find_pivot_check(first_branch: &Branch, branches: &VecDeque<Branch>) -> Option<PatternCheck> {
+fn find_pivot_check(
+    first_branch: &Branch,
+    branches: &VecDeque<Branch>,
+    env: &Environment<'_>,
+) -> Option<PatternCheck> {
     // To try and minimise code duplication, we use the following heuristic: we
     // choose the check matching on the variable that is referenced the most
     // across all checks in all branches.
@@ -2727,10 +2745,28 @@ fn find_pivot_check(first_branch: &Branch, branches: &VecDeque<Branch>) -> Optio
         }
     }
 
+    // Maximize ( -branching_factor, references ).
+    //
+    // - 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).
+    //
+    // Both parts mirror standard guidance for good match trees (small branching
+    // 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| var_references.get(&check.var.id).cloned().unwrap_or(0))
+        .max_by_key(|check| {
+            let mode = check.var.branch_mode(env);
+            let branching_factor = mode.branching_factor();
+            let references = var_references.get(&check.var.id).cloned().unwrap_or(0);
+
+            (usize::MAX - branching_factor, references)
+        })
         .cloned()
 }
 

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[3];
-  if ($ === 1) {
-    let $1 = x[1][2];
-    if ($1 === 2) {
+  let $ = x[1][2];
+  if ($ === 2) {
+    let $1 = x[3];
+    if ($1 === 1) {
       a = x[0];
       t = x[1];
       b = x[1][0];

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

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

compiler-core/src/type_/tests/exhaustiveness.rs

@@ -1787,3 +1787,19 @@ pub fn main(w: Wibble) {
 "
     );
 }
+
+#[test]
+// https://github.com/gleam-lang/gleam/issues/4278
+fn redundant_missing_patterns() {
+    assert_module_error!(
+        r#"
+fn wibble(b: Bool, i: Int) {
+  case b, i {
+    False, 1 -> todo
+    True, 2 -> todo
+  }
+}
+
+pub fn main() { wibble(False, 1) }"#
+    );
+}

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__nested_type_parameter_usage.snap

@@ -26,7 +26,7 @@ one of the other values is used then the assignment will crash.
 
 The missing patterns are:
 
+    Returned([#(), _, ..])
     Returned([])
-    Returned([_, _, ..])
 
 Hint: Use a more general pattern or use `let assert` instead.

compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__exhaustiveness__redundant_missing_patterns.snap

@@ -0,0 +1,32 @@
+---
+source: compiler-core/src/type_/tests/exhaustiveness.rs
+expression: "\nfn wibble(b: Bool, i: Int) {\n  case b, i {\n    False, 1 -> todo\n    True, 2 -> todo\n  }\n}\n\npub fn main() { wibble(False, 1) }"
+---
+----- SOURCE CODE
+
+fn wibble(b: Bool, i: Int) {
+  case b, i {
+    False, 1 -> todo
+    True, 2 -> todo
+  }
+}
+
+pub fn main() { wibble(False, 1) }
+
+----- ERROR
+error: Inexhaustive patterns
+  ┌─ /src/one/two.gleam:3:3
+  │  
+3 │ ╭   case b, i {
+4 │ │     False, 1 -> todo
+5 │ │     True, 2 -> todo
+6 │ │   }
+  │ ╰───^
+
+This case expression does not have a pattern for all possible values. If it
+is run on one of the values without a pattern then it will crash.
+
+The missing patterns are:
+
+    False, _
+    True, _