Pattern-matching test coverage (matching.ml / parmatch.ml)

tests/ERROR_VARIANTS.md

@@ -590,6 +590,14 @@ warnings still fire.
 Fixtures follow the naming convention `warning_<NN>_<description>.res`
 so coverage gaps stay greppable.
 
+Pattern-matching warnings (8 non-exhaustive, 11/12 unused-case /
+sub-pattern, all-clauses-guarded) additionally have shape-oriented
+fixtures named `non_exhaustive_*`, `redundant_or_branch`,
+`overlapping_or_lub`, and `all_clauses_guarded`. These exist to drive the
+`parmatch.ml` witness pretty-printers and redundancy analysis per pattern
+category; see [`PATTERN_MATCHING_COVERAGE.md`](./PATTERN_MATCHING_COVERAGE.md)
+for the reachable-vs-dead breakdown of `matching.ml` / `parmatch.ml`.
+
 ### Removed warnings
 
 The warning constructors listed in **Removed in `jono/remove-dead-errors`**

tests/PATTERN_MATCHING_COVERAGE.md

@@ -0,0 +1,120 @@
+# Pattern-matching coverage notes
+
+Companion to [`ERROR_VARIANTS.md`](./ERROR_VARIANTS.md), scoped to the two
+central pattern-matching modules:
+
+- `compiler/ml/matching.ml` — pattern compilation to Lambda IR
+- `compiler/ml/parmatch.ml` — exhaustiveness / redundancy analysis
+
+It records which uncovered (cold) lines are **reachable** from `.res`
+fixtures and which are **structurally unreachable** (dead code, debug-only,
+or defensive). The goal is to stop coverage reviews from chasing lines that
+no test can ever hit, and to point future fixtures at the lines that remain.
+
+Baseline (fresh `make coverage` off `master` @ `7d96fe384`):
+
+| Module | Coverage | Cold lines |
+|---|---|---|
+| `compiler/ml/matching.ml` | 72.6% | 366 |
+| `compiler/ml/parmatch.ml`  | 75.6% | 256 |
+
+> **Why the common paths are already covered.** Every `switch` in the stdlib
+> and the existing test suite flows through `matching.ml`, so the ordinary
+> per-category dispatchers (constants, constructors, variants, tuples,
+> records, arrays, or-patterns) are already exercised. The cold lines are the
+> *residue*: dead OCaml-heritage branches, debug printers, and genuinely
+> exotic edge cases. Adding fixtures for "common" shapes does **not** move the
+> metric — they overlap existing coverage. Realistic ceiling for both files is
+> roughly **82–88%**, not ~100%.
+
+Line numbers are as of `master` @ `7d96fe384` and will drift; treat them as
+approximate anchors and re-confirm against the current source.
+
+---
+
+## Structurally unreachable — do not chase
+
+### `compiler/ml/matching.ml`
+
+| Lines | What | Why unreachable |
+|---|---|---|
+| ~1530–1600 | `expand_stringswitch`, `do_make_string_test_tree`, `make_string_test_sequence`, `split`, `tree_way_test`, `bind_sw` | **Dead for the JS target.** `combine_constant` routes `Const_string` to `Lstringswitch` (≈ line 2060); the dichotomic string-search tree is only used by the OCaml bytecode compiler (`bytegen.ml`), which ReScript does not use. |
+| 2064–2065 | `Const_int32 -> assert false`, `Const_int64 -> assert false` | ReScript has no `int32`/`int64` literal patterns, so these constant cases never reach `combine_constant`. |
+| ~2175, ~2190 | `... when false ->` branches in `combine_constructor` | Guarded by a literal `false` ("relies on tag being an int"); intentionally disabled. |
+| 1976–1979, 2007–2009, 2013, 2015–2019 | `if dbg then (...)` blocks in `mk_failaction_pos` | `dbg` is a compile-time `false`; debug tracing only. |
+| 69–75, 377–428, 1084–1089, 2540–2552 | `pretty_*` / `pretty_pm` / `pretty_precompiled` / `prerr_endline "** SPLIT **"` / `"COMPILE: "` | Debug pretty-printers, reachable only under `dbg`. |
+| 153–155, 227, 1158–1161, 1665, 1679 | `fatal_error "Matching.filter_matrix"` / `"Matching.ctx_matcher"` / `"BAD: " ^ caller; assert false` / `Invalid_argument "cut"` / `fatal_error "Matching.do_tests_nofail"` | Defensive assertions guarding internal invariants; not reachable from well-typed input. |
+
+### `compiler/ml/parmatch.ml`
+
+| Lines | What | Why unreachable |
+|---|---|---|
+| ~364–434 | `pretty_const`, `pretty_val`, `pretty_car`, `pretty_cdr`, `pretty_arg`, `pretty_lvals` | **Debug-only.** These are *not* the user-facing witness printer. The non-exhaustive warning builds its counter-example via `!print_res_pat` (parmatch.ml:2088), which is bound to `Pattern_printer.print_pattern` in `compiler/common/pattern_printer.ml` (already ~88% covered). The `pretty_*` family here is reachable only from the `dbg` tracing in this module. |
+| 369–370 | `Const_int32 -> "%ldl"`, `Const_int64 -> "%LdL"` in `pretty_const` | (Subsumed by the row above; also: no `int32`/`int64` literal patterns exist.) |
+| 461–476 | `pretty_line` / `pretty_matrix` (`prerr_endline "begin matrix"` …) | Debug-only matrix dumpers. |
+| 981–986 | `fatal_error "Parmatch.get_variant_constructors"` | Defensive; the guarded shapes are guaranteed by typing. |
+
+---
+
+## Reachable but cold — worth fixtures
+
+> **Correction / lesson.** An earlier draft of this file claimed the
+> `pretty_val` / `pretty_const` family (~364–434) was the reachable witness
+> printer and that one non-exhaustive fixture *per pattern shape* would light
+> it up. That is **wrong** — those are debug printers (see the dead-code table
+> above); the user-facing witness is rendered by `Pattern_printer.print_pattern`.
+> Direct measurement (compile each fixture with a dedicated `BISECT_FILE`)
+> confirmed the `pretty_*` lines stay cold even when the witness prints. Always
+> verify a "reachable" hypothesis against an actual `.coverage` file, not
+> against the visible warning text.
+
+### `compiler/ml/parmatch.ml` — analysis edges
+
+These *are* reachable, but the payoff is small (a handful of lines), because the
+surrounding exhaustiveness/redundancy machinery is already exercised by the
+existing suite.
+
+| Lines | What | Trigger | Status |
+|---|---|---|---|
+| 2058–2061 | `Warnings.All_clauses_guarded` | every clause carries an `if` guard | partially covered by `all_clauses_guarded` |
+| 1649–1652, 1757–1761 | `Upartial` merge in or-pattern redundancy | an or-pattern clause whose *some* sub-branch is redundant | partially covered by `redundant_or_branch` |
+| 1814–1859 | `lub` (least upper bound) for tuple/variant/record/array | a clause covered by the *combination* of two overlapping or-branches | still cold — `overlapping_or_lub` did **not** reach the tuple-`lub` branch; needs a sharper trigger |
+
+### `compiler/ml/matching.ml` — compilation edges
+
+| Lines | What | Trigger | Status |
+|---|---|---|---|
+| 1662–1705 | `make_test_sequence` dichotomic split (`split_sequence`/`cut`) | float/bigint/char match with **≥ 4** value cases | ✓ covered by `pattern_match_constants_test` |
+| 2182–2186, 2396–2399, 2479–2482, 2911–2915, 2952–2957 | partial-match failaction / unused-handler / cannot-flatten paths | a *simple* non-exhaustive match is **not** enough; these need an unused or-pattern handler, a `Cannot_flatten` tuple-`let`, or a partial match whose non-constant constructors share an action | still cold — needs targeted fixtures |
+| 1295–1300, 1311–1314, 1390–1393, 1491–1494 | `matcher_*` `Tpat_or` (`raise OrPat`) | or-patterns at a column being specialized during precompilation | still cold |
+| 1877–1923 | `as_interval_canfail` / `as_interval_nofail` hole logic | integer switches with non-contiguous values and a shared fail action | still cold |
+
+> **Measured contribution of this PR's fixtures.** Confirmed via per-fixture
+> `BISECT_FILE` compilation (full JS, not `-bs-cmi-only`): ~10 lines in
+> `matching.ml` (the dichotomic split) and ~5 in `parmatch.ml`
+> (`All_clauses_guarded`, `Upartial`). The rest of the apparent full-suite
+> delta is measurement noise from differing compile sets between runs. The
+> behaviour fixtures' main value is **regression protection + cross-`bsc`
+> portability**, not the coverage metric — most pattern shapes were already
+> covered by the existing suite.
+
+---
+
+## How to re-measure
+
+```bash
+# Fresh baseline (records compiler line hits via bisect_ppx):
+make coverage            # writes _coverage/coverage.json (Coveralls format)
+
+# Per-file cold lines:
+jq -r --arg f compiler/ml/matching.ml \
+  '.source_files[] | select(.name|endswith($f)) | .coverage
+   | to_entries | map(select(.value==0) | .key+1) | @csv' \
+  _coverage/coverage.json
+```
+
+To check whether one fixture hits a target line without a full run, compile it
+with the bisect-instrumented `bsc` and a dedicated `BISECT_FILE`, then inspect
+the resulting `*.coverage`. For witness branches it is usually enough to read
+the warning text: if the printed counter-example has the shape you targeted,
+the matching `pretty_val` branch executed.

tests/build_tests/super_errors/expected/all_clauses_guarded.res.expected

@@ -0,0 +1,14 @@
+
+  Warning number 8
+  /.../fixtures/all_clauses_guarded.res:4:3-7:3
+
+  2 │ // exercises Warnings.All_clauses_guarded in parmatch.ml.
+  3 │ let f = (x: int) =>
+  4 │   switch x {
+  5 │   | n if n > 0 => "pos"
+  6 │   | n if n <= 0 => "nonpos"
+  7 │   }
+  8 │ 
+
+  this pattern-matching is not exhaustive.
+All clauses in this pattern-matching are guarded.

tests/build_tests/super_errors/expected/non_exhaustive_array.res.expected

@@ -0,0 +1,14 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_array.res:4:3-7:3
+
+  2 │ // witness.
+  3 │ let f = (x: array<int>) =>
+  4 │   switch x {
+  5 │   | [] => 0
+  6 │   | [_] => 1
+  7 │   }
+  8 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | [_, _]

tests/build_tests/super_errors/expected/non_exhaustive_bigint.res.expected

@@ -0,0 +1,13 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_bigint.res:3:3-5:3
+
+  1 │ // Witness uses pretty_const for Const_bigint.
+  2 │ let f = (x: bigint) =>
+  3 │   switch x {
+  4 │   | 0n => 1
+  5 │   }
+  6 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | 0n

tests/build_tests/super_errors/expected/non_exhaustive_float.res.expected

@@ -0,0 +1,13 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_float.res:3:3-5:3
+
+  1 │ // Witness uses pretty_const for Const_float.
+  2 │ let f = (x: float) =>
+  3 │   switch x {
+  4 │   | 0.0 => 1
+  5 │   }
+  6 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | 1.

tests/build_tests/super_errors/expected/non_exhaustive_int.res.expected

@@ -0,0 +1,13 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_int.res:3:3-5:3
+
+  1 │ // Witness uses pretty_const for Const_int.
+  2 │ let f = (x: int) =>
+  3 │   switch x {
+  4 │   | 0 => "zero"
+  5 │   }
+  6 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | 1

tests/build_tests/super_errors/expected/non_exhaustive_list.res.expected

@@ -0,0 +1,13 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_list.res:4:3-6:3
+
+  2 │ // printer (pretty_car / pretty_cdr) in parmatch.ml.
+  3 │ let f = (x: list<int>) =>
+  4 │   switch x {
+  5 │   | list{} => 0
+  6 │   }
+  7 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | list{_, ..._}

tests/build_tests/super_errors/expected/non_exhaustive_polyvariant.res.expected

@@ -0,0 +1,13 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_polyvariant.res:4:3-6:3
+
+  2 │ // witness printer.
+  3 │ let f = (x: [#A | #B(int)]) =>
+  4 │   switch x {
+  5 │   | #A => 0
+  6 │   }
+  7 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | #B(_)

tests/build_tests/super_errors/expected/non_exhaustive_record.res.expected

@@ -0,0 +1,14 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_record.res:5:3-7:3
+
+  3 │ // Non-exhaustive record match: exercises the Tpat_record witness printe
+    │ r.
+  4 │ let f = x =>
+  5 │   switch x {
+  6 │   | {a: true, b: true} => 1
+  7 │   }
+  8 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | {a: true, b: false, _} | {a: false, b: _, _}

tests/build_tests/super_errors/expected/non_exhaustive_string.res.expected

@@ -0,0 +1,13 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_string.res:3:3-5:3
+
+  1 │ // Witness uses pretty_const for Const_string.
+  2 │ let f = (x: string) =>
+  3 │   switch x {
+  4 │   | "a" => 1
+  5 │   }
+  6 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | ""

tests/build_tests/super_errors/expected/non_exhaustive_tuple.res.expected

@@ -0,0 +1,14 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_tuple.res:4:3-7:3
+
+  2 │ // printer in parmatch.ml (pretty_val).
+  3 │ let f = (x: (bool, bool)) =>
+  4 │   switch x {
+  5 │   | (true, true) => 1
+  6 │   | (false, false) => 2
+  7 │   }
+  8 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | (false, true) | (true, false)

tests/build_tests/super_errors/expected/non_exhaustive_variant.res.expected

@@ -0,0 +1,13 @@
+
+  Warning number 8
+  /.../fixtures/non_exhaustive_variant.res:6:3-8:3
+
+  4 │ // witness printer.
+  5 │ let f = x =>
+  6 │   switch x {
+  7 │   | A => 0
+  8 │   }
+  9 │ 
+
+  You forgot to handle a possible case here, for example: 
+  | B(_) | D(_, _)

tests/build_tests/super_errors/expected/overlapping_or_lub.res.expected

@@ -0,0 +1,11 @@
+
+  Warning number 11
+  /.../fixtures/overlapping_or_lub.res:7:5-10
+
+  5 ┆ switch x {
+  6 ┆ | (1, _) | (_, 2) => "a"
+  7 ┆ | (1, 2) => "b"
+  8 ┆ | _ => "c"
+  9 ┆ }
+
+  this match case is unused.

tests/build_tests/super_errors/expected/redundant_or_branch.res.expected

@@ -0,0 +1,39 @@
+
+  Warning number 4
+  /.../fixtures/redundant_or_branch.res:6:3-10:3
+
+   4 │ // Upartial (partially-redundant or-pattern) path in parmatch.ml.
+   5 │ let f = x =>
+   6 │   switch x {
+   7 │   | A | B => 1
+   8 │   | B | C => 2
+   9 │   | _ => 3
+  10 │   }
+  11 │ 
+
+  this pattern-matching is fragile.
+It will remain exhaustive when constructors are added to type t.
+
+
+  Warning number 12
+  /.../fixtures/redundant_or_branch.res:8:5
+
+   6 ┆ switch x {
+   7 ┆ | A | B => 1
+   8 ┆ | B | C => 2
+   9 ┆ | _ => 3
+  10 ┆ }
+
+  this sub-pattern is unused.
+
+
+  Warning number 11
+  /.../fixtures/redundant_or_branch.res:9:5
+
+   7 ┆ | A | B => 1
+   8 ┆ | B | C => 2
+   9 ┆ | _ => 3
+  10 ┆ }
+  11 ┆ 
+
+  this match case is unused.

tests/build_tests/super_errors/fixtures/all_clauses_guarded.res

@@ -0,0 +1,7 @@
+// Every clause is guarded, so exhaustiveness cannot be proven:
+// exercises Warnings.All_clauses_guarded in parmatch.ml.
+let f = (x: int) =>
+  switch x {
+  | n if n > 0 => "pos"
+  | n if n <= 0 => "nonpos"
+  }

tests/build_tests/super_errors/fixtures/non_exhaustive_array.res

@@ -0,0 +1,7 @@
+// Non-exhaustive array match: exercises array-length exhaustiveness and its
+// witness.
+let f = (x: array<int>) =>
+  switch x {
+  | [] => 0
+  | [_] => 1
+  }

tests/build_tests/super_errors/fixtures/non_exhaustive_bigint.res

@@ -0,0 +1,5 @@
+// Witness uses pretty_const for Const_bigint.
+let f = (x: bigint) =>
+  switch x {
+  | 0n => 1
+  }

tests/build_tests/super_errors/fixtures/non_exhaustive_char.res

@@ -0,0 +1,5 @@
+// Witness uses pretty_const for Const_char.
+let f = (x: char) =>
+  switch x {
+  | 'a' => 1
+  }

tests/build_tests/super_errors/fixtures/non_exhaustive_float.res

@@ -0,0 +1,5 @@
+// Witness uses pretty_const for Const_float.
+let f = (x: float) =>
+  switch x {
+  | 0.0 => 1
+  }

tests/build_tests/super_errors/fixtures/non_exhaustive_int.res

@@ -0,0 +1,5 @@
+// Witness uses pretty_const for Const_int.
+let f = (x: int) =>
+  switch x {
+  | 0 => "zero"
+  }