From fdc312f50389a5c80440d9e72a911798348bb849 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Mon, 19 Oct 2026 13:31:33 +0000 Subject: [PATCH 1/7] Warn about untagged payload patterns shadowed by literal cases A payload pattern whose values are all literal-case values can never match (warning 11/12), and a literal case covers the equal bool payload value in the exhaustiveness check. Fixes #8806. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- .../untagged-literal-overlap-warnings.fix.md | 1 + compiler/ext/warnings.ml | 14 +- compiler/ext/warnings.mli | 4 +- compiler/ml/parmatch.ml | 237 +++++++++++++++++- ...untagged_bool_payload_overlap.res.expected | 12 + ...agged_literal_shadows_payload.res.expected | 65 +++++ ...literal_shadows_payload_kinds.res.expected | 38 +++ ...ed_literal_shadows_or_pattern.res.expected | 12 + ...rning_11_untagged_bool_payload_overlap.res | 16 ++ ...ng_11_untagged_literal_shadows_payload.res | 44 ++++ ...untagged_literal_shadows_payload_kinds.res | 26 ++ ...12_untagged_literal_shadows_or_pattern.res | 8 + tests/tests/src/unboxed_variant_fold_test.mjs | 145 ++++++----- tests/tests/src/unboxed_variant_fold_test.res | 12 +- 14 files changed, 553 insertions(+), 81 deletions(-) create mode 100644 changes/untagged-literal-overlap-warnings.fix.md create mode 100644 tests/build_tests/super_errors/expected/warning_11_untagged_bool_payload_overlap.res.expected create mode 100644 tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_payload.res.expected create mode 100644 tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_payload_kinds.res.expected create mode 100644 tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_or_pattern.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/warning_11_untagged_bool_payload_overlap.res create mode 100644 tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_payload.res create mode 100644 tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_payload_kinds.res create mode 100644 tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res diff --git a/changes/untagged-literal-overlap-warnings.fix.md b/changes/untagged-literal-overlap-warnings.fix.md new file mode 100644 index 0000000000..96d2acf47c --- /dev/null +++ b/changes/untagged-literal-overlap-warnings.fix.md @@ -0,0 +1 @@ +- Warn about untagged variant payload patterns that can never match because a literal case has the same value, like `S("a")` with `@as("a") A | S(string)`, and treat `@as(true) Yes | Ans(bool)` as fully covered by `Yes | Ans(false)`. diff --git a/compiler/ext/warnings.ml b/compiler/ext/warnings.ml index cef394251d..83e5b65426 100644 --- a/compiler/ext/warnings.ml +++ b/compiler/ext/warnings.ml @@ -34,8 +34,8 @@ type t = | Partial_application (* 5 *) | Partial_match of string (* 8 *) | Non_closed_record_pattern of string (* 9 *) - | Unused_match (* 11 *) - | Unused_pat (* 12 *) + | Unused_match of string (* 11 *) + | Unused_pat of string (* 12 *) | Unused_argument (* 20 *) | Nonreturning_statement (* 21 *) | Preprocessor of string (* 22 *) @@ -88,8 +88,8 @@ let number = function | Partial_application -> 5 | Partial_match _ -> 8 | Non_closed_record_pattern _ -> 9 - | Unused_match -> 11 - | Unused_pat -> 12 + | Unused_match _ -> 11 + | Unused_pat _ -> 12 | Unused_argument -> 20 | Nonreturning_statement -> 21 | Preprocessor _ -> 22 @@ -297,8 +297,10 @@ let message = function | Non_closed_record_pattern s -> "the following labels are not bound in this record pattern: " ^ s ^ "\nEither bind these labels explicitly or add ', _' to the pattern." - | Unused_match -> "this match case is unused." - | Unused_pat -> "this sub-pattern is unused." + | Unused_match "" -> "this match case is unused." + | Unused_match reason -> "this match case is unused.\n " ^ reason + | Unused_pat "" -> "this sub-pattern is unused." + | Unused_pat reason -> "this sub-pattern is unused.\n " ^ reason | Unused_argument -> "this argument will not be used by the function." | Nonreturning_statement -> "This statement does not continue execution; following code is unreachable." diff --git a/compiler/ext/warnings.mli b/compiler/ext/warnings.mli index 2d43ceb710..b5ce7f62bc 100644 --- a/compiler/ext/warnings.mli +++ b/compiler/ext/warnings.mli @@ -27,8 +27,8 @@ type t = | Partial_application (* 5 *) | Partial_match of string (* 8 *) | Non_closed_record_pattern of string (* 9 *) - | Unused_match (* 11 *) - | Unused_pat (* 12 *) + | Unused_match of string (* 11 *) + | Unused_pat of string (* 12 *) | Unused_argument (* 20 *) | Nonreturning_statement (* 21 *) | Preprocessor of string (* 22 *) diff --git a/compiler/ml/parmatch.ml b/compiler/ml/parmatch.ml index 15557ec177..eb2af9d279 100644 --- a/compiler/ml/parmatch.ml +++ b/compiler/ml/parmatch.ml @@ -2260,6 +2260,147 @@ let do_check_fragile_param exhaust loc casel pss = let do_check_fragile_gadt = do_check_fragile_param exhaust_gadt +(*****************************************) +(* Payloads shadowed by untagged literals *) +(*****************************************) + +(* An untagged variant tests its literal cases before its payload cases, so in + [@as("a") A | S(string)] the value "a" is always matched as [A]. A payload + pattern can therefore never match a value that is also the runtime value of + a literal case, wherever it appears in the match: [S("a")] is dead. *) + +type shadowed_payload = { + payload_constructor: string; + value: Variant_runtime.literal_tag; + literal_constructor: string; +} + +type payload_liveness = + | Never_matches of shadowed_payload list + | Matches of (pattern * shadowed_payload list) list + (** The pattern can match, except for these dead or-pattern alternatives *) + +let is_type_path path ty = + match (Ctype.repr ty).desc with + | Tconstr (p, _, _) -> Path.same p path + | _ -> false + +(* The runtime value of the only value [p] matches, when it is a literal *) +let rec pattern_literal p : Variant_runtime.literal_tag option = + match p.pat_desc with + | Tpat_constant (Const_string s) -> Some (String s) + | Tpat_constant (Const_int i | Const_char i) -> Some (Int i) + | Tpat_constant (Const_float f) -> Some (Float f) + | Tpat_constant (Const_bigint _) -> + (* Bigints are never compared, see [Variant_runtime.same_literal_value] *) + None + | Tpat_alias (p, _, _) -> pattern_literal p + | Tpat_construct (_, {cstr_name; cstr_res}, []) + when is_type_path Predef.path_bool cstr_res -> + Some (Bool (cstr_name = "true")) + | Tpat_construct (_, {cstr_res}, _) + when List.exists + (fun path -> is_type_path path cstr_res) + [Predef.path_unit; Predef.path_list; Predef.path_option] -> + None + | Tpat_construct (_, {cstr_kind = Ordinary_constructor r}, args) -> ( + match (Variant_runtime.representation r, args) with + | Constant tag, [] -> Some tag.literal + | Block (Untagged _), [arg] -> + (* An untagged payload is represented by the payload itself *) + pattern_literal arg + | (Constant _ | Block _), _ -> None) + | Tpat_construct (_, {cstr_kind = Extension_constructor _}, _) + | Tpat_any | Tpat_var _ | Tpat_interval _ | Tpat_tuple _ | Tpat_variant _ + | Tpat_record _ | Tpat_dict _ | Tpat_array _ | Tpat_or _ -> + None + +(* The literal cases that take precedence over an untagged payload constructor, + as runtime values and constructor names *) +let shadowing_literal_cases cd = + match cd.cstr_kind with + | Ordinary_constructor r -> ( + match Variant_runtime.representation r with + | Block (Untagged _) -> + let layout = Variant_runtime.get_layout r.variant in + List.init (Variant_runtime.length layout) Fun.id + |> List.filter_map (fun position -> + match Variant_runtime.constructor_at layout position with + | Constant tag -> Some (tag.literal, tag.name) + | Block _ -> None) + | Constant _ | Block (Tagged _) -> []) + | Extension_constructor _ -> [] + +let or_liveness liveness p1 p2 = + match (liveness p1, liveness p2) with + | Never_matches s1, Never_matches s2 -> Never_matches (s1 @ s2) + | Matches d1, Never_matches s2 -> Matches (d1 @ [(p2, s2)]) + | Never_matches s1, Matches d2 -> Matches ((p1, s1) :: d2) + | Matches d1, Matches d2 -> Matches (d1 @ d2) + +let rec payload_liveness p = + match p.pat_desc with + | Tpat_or (p1, p2, _) -> or_liveness payload_liveness p1 p2 + | Tpat_construct (_, cd, [arg]) -> ( + match shadowing_literal_cases cd with + | [] -> subpatterns_liveness p + | literal_cases -> + shadowed_argument_liveness ~payload_constructor:cd.cstr_name + ~literal_cases arg) + | _ -> subpatterns_liveness p + +(* A pattern that is not an or-pattern matches nothing as soon as one of its + subpatterns matches nothing *) +and subpatterns_liveness p = + let result = ref (Matches []) in + Typedtree.iter_pattern_desc + (fun sub -> + match (!result, payload_liveness sub) with + | Never_matches _, _ -> () + | Matches _, (Never_matches _ as dead) -> result := dead + | Matches d1, Matches d2 -> result := Matches (d1 @ d2)) + p.pat_desc; + !result + +and shadowed_argument_liveness ~payload_constructor ~literal_cases arg = + match arg.pat_desc with + | Tpat_or (p1, p2, _) -> + or_liveness + (shadowed_argument_liveness ~payload_constructor ~literal_cases) + p1 p2 + | _ -> ( + let shadowing_case = + match pattern_literal arg with + | Some value -> + List.find_opt + (fun (literal, _) -> Variant_runtime.same_literal_value value literal) + literal_cases + | None -> None + in + match shadowing_case with + | Some (value, literal_constructor) -> + Never_matches [{payload_constructor; value; literal_constructor}] + | None -> payload_liveness arg) + +let literal_to_string : Variant_runtime.literal_tag -> string = function + | String s -> "\"" ^ s ^ "\"" + | Int i -> string_of_int i + | Float f -> f + | BigInt b -> b ^ "n" + | Bool b -> string_of_bool b + | Null -> "null" + | Undefined -> "undefined" + +let shadowed_payload_reason shadowed = + shadowed |> List.sort_uniq compare + |> List.map (fun {payload_constructor; value; literal_constructor} -> + Printf.sprintf + "%s is the runtime value of constructor %s, so it is always matched as \ + %s, never as %s." + (literal_to_string value) literal_constructor literal_constructor + payload_constructor) + |> String.concat "\n " + (********************************) (* Exported unused clause check *) (********************************) @@ -2268,11 +2409,40 @@ let lower_cases casel = let lower = Dict_pattern.lowering (List.map (fun c -> c.c_lhs) casel) in List.map (fun c -> {c with c_lhs = lower c.c_lhs}) casel +(* Report cases and or-pattern alternatives that can never match because of + [payload_liveness]. Returns the cases that match nothing, and whether a + sub-pattern has already been reported. *) +let check_shadowed_payloads casel = + let reported = ref [] in + let dead_cases = + List.filter + (fun {c_lhs = q} -> + match payload_liveness q with + | Never_matches shadowed -> + Location.prerr_warning q.pat_loc + (Warnings.Unused_match (shadowed_payload_reason shadowed)); + true + | Matches dead_alternatives -> + List.iter + (fun (p, shadowed) -> + reported := p.pat_loc :: !reported; + Location.prerr_warning p.pat_loc + (Warnings.Unused_pat (shadowed_payload_reason shadowed))) + dead_alternatives; + false) + casel + in + (dead_cases, fun p -> List.mem p.pat_loc !reported) + let check_unused pred casel = - if Warnings.is_active Warnings.Unused_match then + if Warnings.is_active (Warnings.Unused_match "") then let casel = lower_cases casel in + let dead_cases, already_reported = check_shadowed_payloads casel in let rec do_rec pref = function | [] -> () + | case :: rem when List.memq case dead_cases -> + (* A case that matches nothing cannot make later cases unused *) + do_rec pref rem | {c_lhs = q; c_guard} :: rem -> let qs = [q] in (try @@ -2323,13 +2493,16 @@ let check_unused pred casel = | _ -> r in match r with - | Unused -> Location.prerr_warning q.pat_loc Warnings.Unused_match + | Unused -> + Location.prerr_warning q.pat_loc (Warnings.Unused_match "") | Upartial ps -> ps |> List.filter (fun p -> - not (Variant_type_spread.is_pat_from_variant_spread_attr p)) + not + (Variant_type_spread.is_pat_from_variant_spread_attr p + || already_reported p)) |> List.iter (fun p -> - Location.prerr_warning p.pat_loc Warnings.Unused_pat) + Location.prerr_warning p.pat_loc (Warnings.Unused_pat "")) | Used -> () with Empty | Not_found | NoGuard -> assert false); @@ -2361,8 +2534,62 @@ let check_partial_param do_check_partial do_check_fragile loc casel = do_check_fragile loc casel pss; total +(* For the exhaustiveness check only: a literal case of an untagged variant + also covers the payload values equal to its runtime value. This matters for + a bool payload, whose values can all be covered, as in + [@as(true) Yes | Ans(bool)] where [Yes | Ans(false)] is exhaustive. Other + payload types have infinitely many values, so the overlap can't complete a + match and is left out. *) +let rec with_bool_payload_overlaps p = + match p.pat_desc with + | Tpat_construct (_, {cstr_kind = Ordinary_constructor r}, []) -> ( + match Variant_runtime.representation r with + | Constant tag -> ( + match tag.literal with + | Bool b -> ( + let is_bool_payload cd = + match cd.cstr_kind with + | Ordinary_constructor r -> ( + match Variant_runtime.representation r with + | Block (Untagged {block_type = BooleanType}) -> true + | Constant _ | Block _ -> false) + | Extension_constructor _ -> false + in + match + List.find_opt is_bool_payload + (get_variant_constructors p.pat_env p.pat_type) + with + | Some payload_cd -> + let bool_cd = + List.find + (fun cd -> cd.cstr_name = string_of_bool b) + (get_variant_constructors p.pat_env (Predef.type_bool ())) + in + let construct cd args ty = + make_pat + (Tpat_construct (mknoloc (Longident.Lident cd.cstr_name), cd, args)) + ty p.pat_env + in + orify p + (construct payload_cd + [construct bool_cd [] (Predef.type_bool ())] + p.pat_type) + | None -> p) + | String _ | Int _ | Float _ | BigInt _ | Null | Undefined -> p) + | Block _ -> p) + | _ -> + { + p with + pat_desc = + Typedtree.map_pattern_desc with_bool_payload_overlaps p.pat_desc; + } + let check_partial_gadt ?partial_match_warning_hint pred loc casel = - let casel = lower_cases casel in + let casel = + List.map + (fun c -> {c with c_lhs = with_bool_payload_overlaps c.c_lhs}) + (lower_cases casel) + in check_partial_param (do_check_partial_gadt ?partial_match_warning_hint pred) do_check_fragile_gadt loc casel diff --git a/tests/build_tests/super_errors/expected/warning_11_untagged_bool_payload_overlap.res.expected b/tests/build_tests/super_errors/expected/warning_11_untagged_bool_payload_overlap.res.expected new file mode 100644 index 0000000000..ba54d0b761 --- /dev/null +++ b/tests/build_tests/super_errors/expected/warning_11_untagged_bool_payload_overlap.res.expected @@ -0,0 +1,12 @@ + + Warning number 11 + /.../fixtures/warning_11_untagged_bool_payload_overlap.res:14:5-13 + + 12 ┆ switch x { + 13 ┆ | Yes => 1 + 14 ┆ | Ans(true) => 3 + 15 ┆ | Ans(false) => 2 + 16 ┆ } + + this match case is unused. + true is the runtime value of constructor Yes, so it is always matched as Yes, never as Ans. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_payload.res.expected b/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_payload.res.expected new file mode 100644 index 0000000000..a4157f9042 --- /dev/null +++ b/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_payload.res.expected @@ -0,0 +1,65 @@ + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_payload.res:7:5-10 + + 5 ┆ switch x { + 6 ┆ | A => 1 + 7 ┆ | S("a") => 2 + 8 ┆ | S(_) => 3 + 9 ┆ } + + this match case is unused. + "a" is the runtime value of constructor A, so it is always matched as A, never as S. + + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_payload.res:13:5-10 + + 11 │ let beforeLiteral = x => + 12 │ switch x { + 13 │ | S("a") => 2 + 14 │ | A => 1 + 15 │ | S(_) => 3 + + this match case is unused. + "a" is the runtime value of constructor A, so it is always matched as A, never as S. + + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_payload.res:20:5-10 + + 18 │ let withoutLiteralCase = x => + 19 │ switch x { + 20 │ | S("a") => 2 + 21 │ | S(_) | A => 3 + 22 │ } + + this match case is unused. + "a" is the runtime value of constructor A, so it is always matched as A, never as S. + + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_payload.res:26:5-15 + + 24 │ let nested = x => + 25 │ switch x { + 26 │ | (S("a"), _) => 1 + 27 │ | (S(_) | A, _) => 2 + 28 │ } + + this match case is unused. + "a" is the runtime value of constructor A, so it is always matched as A, never as S. + + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_payload.res:42:5-21 + + 40 │ let allAlternatives = x => + 41 │ switch x { + 42 │ | US("a") | US("b") => 1 + 43 │ | US(_) | UA | UB => 2 + 44 │ } + + this match case is unused. + "a" is the runtime value of constructor UA, so it is always matched as UA, never as US. + "b" is the runtime value of constructor UB, so it is always matched as UB, never as US. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_payload_kinds.res.expected b/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_payload_kinds.res.expected new file mode 100644 index 0000000000..8d7b6b7fbc --- /dev/null +++ b/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_payload_kinds.res.expected @@ -0,0 +1,38 @@ + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_payload_kinds.res:7:5-20 + + 5 │ let byName = x => + 6 │ switch x { + 7 │ | Color("Primary") => 1 + 8 │ | Color(_) | Primary => 2 + 9 │ } + + this match case is unused. + "Primary" is the runtime value of constructor Primary, so it is always matched as Primary, never as Color. + + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_payload_kinds.res:15:5-10 + + 13 │ let intAndFloat = x => + 14 │ switch x { + 15 │ | F(1.0) => 1 + 16 │ | F(_) | One => 2 + 17 │ } + + this match case is unused. + 1 is the runtime value of constructor One, so it is always matched as One, never as F. + + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_payload_kinds.res:24:5-8 + + 22 │ let nestedConstant = x => + 23 │ switch x { + 24 │ | W(P) => 1 + 25 │ | W(Q(_)) | X => 2 + 26 │ } + + this match case is unused. + "x" is the runtime value of constructor X, so it is always matched as X, never as W. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_or_pattern.res.expected b/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_or_pattern.res.expected new file mode 100644 index 0000000000..8ff24dce37 --- /dev/null +++ b/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_or_pattern.res.expected @@ -0,0 +1,12 @@ + + Warning number 12 + /.../fixtures/warning_12_untagged_literal_shadows_or_pattern.res:6:7-9 + + 4 │ let f = x => + 5 │ switch x { + 6 │ | S("a" | "b") => 2 + 7 │ | S(_) | A => 3 + 8 │ } + + this sub-pattern is unused. + "a" is the runtime value of constructor A, so it is always matched as A, never as S. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/warning_11_untagged_bool_payload_overlap.res b/tests/build_tests/super_errors/fixtures/warning_11_untagged_bool_payload_overlap.res new file mode 100644 index 0000000000..7cade40e6a --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/warning_11_untagged_bool_payload_overlap.res @@ -0,0 +1,16 @@ +// true is always matched as Yes, so Ans(true) can never match, and a match +// covering Yes and Ans(false) is exhaustive (no warning for `exhaustive`). +@unboxed type b = | @as(true) Yes | Ans(bool) + +let exhaustive = x => + switch x { + | Yes => 1 + | Ans(false) => 2 + } + +let deadArm = x => + switch x { + | Yes => 1 + | Ans(true) => 3 + | Ans(false) => 2 + } diff --git a/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_payload.res b/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_payload.res new file mode 100644 index 0000000000..435ca4cc11 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_payload.res @@ -0,0 +1,44 @@ +// "a" is always matched as A, so S("a") can never match, wherever it appears. +@unboxed type t = | @as("a") A | S(string) + +let afterLiteral = x => + switch x { + | A => 1 + | S("a") => 2 + | S(_) => 3 + } + +let beforeLiteral = x => + switch x { + | S("a") => 2 + | A => 1 + | S(_) => 3 + } + +let withoutLiteralCase = x => + switch x { + | S("a") => 2 + | S(_) | A => 3 + } + +let nested = x => + switch x { + | (S("a"), _) => 1 + | (S(_) | A, _) => 2 + } + +// Payloads that are not literal values still match. +let fine = x => + switch x { + | S("b") => 1 + | S(s) => String.length(s) + | A => 0 + } + +// Every alternative can never match. +@unboxed type u = | @as("a") UA | @as("b") UB | US(string) +let allAlternatives = x => + switch x { + | US("a") | US("b") => 1 + | US(_) | UA | UB => 2 + } diff --git a/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_payload_kinds.res b/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_payload_kinds.res new file mode 100644 index 0000000000..0f1aaa929f --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_payload_kinds.res @@ -0,0 +1,26 @@ +// Literal values are compared as runtime values. + +// Without @as, a constructor is represented by its name. +@unboxed type color = Primary | Color(string) +let byName = x => + switch x { + | Color("Primary") => 1 + | Color(_) | Primary => 2 + } + +// @as(1) and 1.0 are the same JavaScript number. +@unboxed type num = | @as(1) One | F(float) +let intAndFloat = x => + switch x { + | F(1.0) => 1 + | F(_) | One => 2 + } + +// A nested untagged constant has the value of its @as. +@unboxed type inner = | @as("x") P | Q(int) +@unboxed type outer = | @as("x") X | W(inner) +let nestedConstant = x => + switch x { + | W(P) => 1 + | W(Q(_)) | X => 2 + } diff --git a/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res b/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res new file mode 100644 index 0000000000..43271e479c --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res @@ -0,0 +1,8 @@ +// Only the "a" alternative can never match: "a" is always matched as A. +@unboxed type t = | @as("a") A | S(string) + +let f = x => + switch x { + | S("a" | "b") => 2 + | S(_) | A => 3 + } diff --git a/tests/tests/src/unboxed_variant_fold_test.mjs b/tests/tests/src/unboxed_variant_fold_test.mjs index 686732a8af..3f6e07e1bd 100644 --- a/tests/tests/src/unboxed_variant_fold_test.mjs +++ b/tests/tests/src/unboxed_variant_fold_test.mjs @@ -55,6 +55,14 @@ function answerName(v) { } } +function answerName2(v) { + if (v === true) { + return "yes"; + } else { + return "answer false"; + } +} + function mixedName(v) { if (v === false) { return "off"; @@ -169,94 +177,96 @@ function makeRecord(value) { Mocha.describe("Unboxed_variant_fold_test", () => { Mocha.test("constructors preserve dynamic payloads and effects", () => { - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 174, characters 7-14", colorName(makeColor("primary")), "not Color"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 175, characters 7-14", colorName(makeColor("blue")), "blue"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 176, characters 7-14", recName(makeRecord(1)), "rec"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 182, characters 7-14", colorName(makeColor("primary")), "not Color"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 183, characters 7-14", colorName(makeColor("blue")), "blue"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 184, characters 7-14", recName(makeRecord(1)), "rec"); let calls = 0; calls = calls + 1 | 0; - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 182, characters 7-14", colorName("primary"), "not Color"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 183, characters 7-14", calls, 1); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 190, characters 7-14", colorName("primary"), "not Color"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 191, characters 7-14", calls, 1); }); Mocha.test("folding agrees with runtime dispatch", () => { - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 186, characters 7-14", "not Color", colorName(id("primary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 187, characters 7-14", "not Color", colorName(id("secondary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 188, characters 7-14", "blue", colorName(id("blue"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 189, characters 7-14", "not Color", colorName(id("primary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 190, characters 7-14", "one", numberName(id(1))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 191, characters 7-14", "number", numberName(id(2))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 192, characters 7-14", "one", numberName(id(1))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 193, characters 7-14", "int", pureName(id(1))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 194, characters 7-14", "string", pureName(id("x"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 195, characters 7-14", "obj", optName(id({ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 194, characters 7-14", "not Color", colorName(id("primary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 195, characters 7-14", "not Color", colorName(id("secondary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 196, characters 7-14", "blue", colorName(id("blue"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 197, characters 7-14", "not Color", colorName(id("primary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 198, characters 7-14", "one", numberName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 199, characters 7-14", "number", numberName(id(2))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 200, characters 7-14", "one", numberName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 201, characters 7-14", "int", pureName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 202, characters 7-14", "string", pureName(id("x"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 203, characters 7-14", "obj", optName(id({ x: 1 }))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 196, characters 7-14", "null", optName(id(null))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 197, characters 7-14", "undef", optName(id(undefined))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 198, characters 7-14", "str", flagName(id("true"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 199, characters 7-14", "yes", flagName(id(true))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 200, characters 7-14", "yes", answerName(id(true))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 201, characters 7-14", "answer false", answerName(id(false))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 202, characters 7-14", "yes", answerName(id(true))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 203, characters 7-14", "toggle true", mixedName(id(true))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 204, characters 7-14", "off", mixedName(id(false))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 205, characters 7-14", "off", mixedName(id(false))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 206, characters 7-14", "unset", mixedName(id(null))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 207, characters 7-14", "false", mixedName(id("false"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 208, characters 7-14", "one", numName(id(1.0))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 209, characters 7-14", "float", numName(id(2.5))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 210, characters 7-14", "one", numName(id(1))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 211, characters 7-14", "rec", recName(id({ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 204, characters 7-14", "null", optName(id(null))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 205, characters 7-14", "undef", optName(id(undefined))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 206, characters 7-14", "str", flagName(id("true"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 207, characters 7-14", "yes", flagName(id(true))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 208, characters 7-14", "yes", answerName(id(true))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 209, characters 7-14", "answer false", answerName(id(false))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 210, characters 7-14", "yes", answerName(id(true))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 211, characters 7-14", "toggle true", mixedName(id(true))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 212, characters 7-14", "off", mixedName(id(false))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 213, characters 7-14", "off", mixedName(id(false))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 214, characters 7-14", "unset", mixedName(id(null))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 215, characters 7-14", "false", mixedName(id("false"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 216, characters 7-14", "one", numName(id(1.0))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 217, characters 7-14", "float", numName(id(2.5))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 218, characters 7-14", "one", numName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 219, characters 7-14", "rec", recName(id({ y: 1 }))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 212, characters 7-14", "empty", recName(id("empty"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 213, characters 7-14", "lit", pick(id("a"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 214, characters 7-14", "z", pick(id("z"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 215, characters 7-14", "lit", pick(id("a"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 216, characters 7-14", "w", outerName(id("primary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 217, characters 7-14", "x", outerName(id("x"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 218, characters 7-14", "x", outerName(id("x"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 219, characters 7-14", "primary", boxedName(id({ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 220, characters 7-14", "empty", recName(id("empty"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 221, characters 7-14", "lit", pick(id("a"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 222, characters 7-14", "z", pick(id("z"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 223, characters 7-14", "lit", pick(id("a"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 224, characters 7-14", "w", outerName(id("primary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 225, characters 7-14", "x", outerName(id("x"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 226, characters 7-14", "x", outerName(id("x"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 227, characters 7-14", "primary", boxedName(id({ TAG: "BoxedColor", _0: "primary" }))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 220, characters 7-14", "not Color", boxedName(id("primary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 221, characters 7-14", "tuple", objectName(id([ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 228, characters 7-14", "not Color", boxedName(id("primary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 229, characters 7-14", "tuple", objectName(id([ 1, 2 ]))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 222, characters 7-14", "record", objectName(id({ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 230, characters 7-14", "record", objectName(id({ x: 1 }))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 223, characters 7-14", "zero", listName(id(/* [] */0))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 224, characters 7-14", "list", listName(id({hd: 1, tl: /* [] */0}))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 225, characters 7-14", bigintName(10n), bigintName(id(10n))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 226, characters 7-14", "one", wideName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 231, characters 7-14", "zero", listName(id(/* [] */0))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 232, characters 7-14", "list", listName(id({hd: 1, tl: /* [] */0}))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 233, characters 7-14", bigintName(10n), bigintName(id(10n))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 234, characters 7-14", "one", wideName(id(1))); }); Mocha.test("the folded answers themselves are correct", () => { - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 230, characters 7-14", "not Color", "not Color"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 231, characters 7-14", "blue", "blue"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 232, characters 7-14", "one", "one"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 233, characters 7-14", "number", "number"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 234, characters 7-14", "int", "int"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 235, characters 7-14", "yes", "yes"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 236, characters 7-14", "answer false", "answer false"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 237, characters 7-14", "off", "off"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 238, characters 7-14", "toggle true", "toggle true"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 239, characters 7-14", "false", "false"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 238, characters 7-14", "not Color", "not Color"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 239, characters 7-14", "blue", "blue"); Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 240, characters 7-14", "one", "one"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 241, characters 7-14", "lit", "lit"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 242, characters 7-14", "x", "x"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 243, characters 7-14", "primary", "primary"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 244, characters 7-14", "tuple", "tuple"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 245, characters 7-14", "zero", "zero"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 246, characters 7-14", bigintName(10n), "one"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 247, characters 7-14", "one", "one"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 248, characters 7-14", numberOrBigintName(1n), "bigint"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 241, characters 7-14", "number", "number"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 242, characters 7-14", "int", "int"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 243, characters 7-14", "yes", "yes"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 244, characters 7-14", "answer false", "answer false"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 245, characters 7-14", answerName2(id(true)), "yes"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 246, characters 7-14", answerName2(id(false)), "answer false"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 247, characters 7-14", "off", "off"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 248, characters 7-14", "toggle true", "toggle true"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 249, characters 7-14", "false", "false"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 250, characters 7-14", "one", "one"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 251, characters 7-14", "lit", "lit"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 252, characters 7-14", "x", "x"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 253, characters 7-14", "primary", "primary"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 254, characters 7-14", "tuple", "tuple"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 255, characters 7-14", "zero", "zero"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 256, characters 7-14", bigintName(10n), "one"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 257, characters 7-14", "one", "one"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 258, characters 7-14", numberOrBigintName(1n), "bigint"); }); Mocha.test("a number and a bigint literal with the same digits", () => { - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 252, characters 7-14", numberOrBigintName(id(1)), "number"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 253, characters 7-14", numberOrBigintName(id(1n)), "bigint"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 254, characters 7-14", numberOrBigintName(id("x")), "x"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 262, characters 7-14", numberOrBigintName(id(1)), "number"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 263, characters 7-14", numberOrBigintName(id(1n)), "bigint"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 264, characters 7-14", numberOrBigintName(id("x")), "x"); }); }); @@ -267,6 +277,7 @@ export { optName, flagName, answerName, + answerName2, mixedName, numName, recName, diff --git a/tests/tests/src/unboxed_variant_fold_test.res b/tests/tests/src/unboxed_variant_fold_test.res index bcb7765568..d2b1440502 100644 --- a/tests/tests/src/unboxed_variant_fold_test.res +++ b/tests/tests/src/unboxed_variant_fold_test.res @@ -57,14 +57,22 @@ let flagName = v => | Str(_) => "str" } -// A boolean payload only receives the boolean that is not a literal case. +// A boolean payload only receives the boolean that is not a literal case. The +// first arm can never match (warning 11); folding must not select it either. @unboxed type answer = | @as(true) Yes2 | Answer(bool) +@warning("-11") let answerName = v => switch v { | Answer(true) => "answer true" | Yes2 => "yes" | Answer(false) => "answer false" } +// Yes2 also covers Answer(true), so this match is exhaustive. +let answerName2 = v => + switch v { + | Yes2 => "yes" + | Answer(false) => "answer false" + } @unboxed type mixed = @@ -234,6 +242,8 @@ describe(__MODULE__, () => { eq(__LOC__, pureName(I(1)), "int") eq(__LOC__, answerName(Answer(true)), "yes") eq(__LOC__, answerName(Answer(false)), "answer false") + eq(__LOC__, answerName2(id(Answer(true))), "yes") + eq(__LOC__, answerName2(id(Answer(false))), "answer false") eq(__LOC__, mixedName(Toggle(false)), "off") eq(__LOC__, mixedName(Toggle(true)), "toggle true") eq(__LOC__, mixedName(Label("false")), "false") From a9780f6c1d674ede677d194e969a4ccdaedf8c0c Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Tue, 20 Oct 2026 11:53:17 +0000 Subject: [PATCH 2/7] Report shadowed payload alternatives when only warning 12 is on The check for untagged payload patterns shadowed by literal cases ran only when warning 11 was active, so with `-w -11+12` or `@warning("-11")` a dead or-pattern alternative got no warning 12. It now runs when either warning is active; the usefulness analysis is still gated on warning 11. Example (rescript): @unboxed type t = | @as("a") A | S(string) @warning("-11") let g = x => switch x { | S("a") => 1 | S("a" | "c") => 2 | S(_) | A => 3 } Diagnostic (text): Before: no warnings After: Warning 12 on the "a" in S("a" | "c"); the dead S("a") case stays silent because warning 11 is off. Area: typechecker tests Output: unchanged Formats: none PPX: unchanged Editor: unchanged Tests: ~tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- compiler/ml/parmatch.ml | 7 +++++-- ...untagged_literal_shadows_or_pattern.res.expected | 13 +++++++++++++ ...rning_12_untagged_literal_shadows_or_pattern.res | 10 ++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/compiler/ml/parmatch.ml b/compiler/ml/parmatch.ml index eb2af9d279..beb35fce47 100644 --- a/compiler/ml/parmatch.ml +++ b/compiler/ml/parmatch.ml @@ -2435,7 +2435,10 @@ let check_shadowed_payloads casel = (dead_cases, fun p -> List.mem p.pat_loc !reported) let check_unused pred casel = - if Warnings.is_active (Warnings.Unused_match "") then + let unused_match = Warnings.is_active (Warnings.Unused_match "") in + (* Shadowed payloads are reported as warning 11 or 12, so they are checked + when either is active; the usefulness analysis below only when 11 is. *) + if unused_match || Warnings.is_active (Warnings.Unused_pat "") then let casel = lower_cases casel in let dead_cases, already_reported = check_shadowed_payloads casel in let rec do_rec pref = function @@ -2509,7 +2512,7 @@ let check_unused pred casel = if c_guard <> None then do_rec pref rem else do_rec ([q] :: pref) rem in - do_rec [] casel + if unused_match then do_rec [] casel (*********************************) (* Exported irrefutability tests *) diff --git a/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_or_pattern.res.expected b/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_or_pattern.res.expected index 8ff24dce37..6d716a4d87 100644 --- a/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_or_pattern.res.expected +++ b/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_or_pattern.res.expected @@ -8,5 +8,18 @@ 7 │ | S(_) | A => 3 8 │ } + this sub-pattern is unused. + "a" is the runtime value of constructor A, so it is always matched as A, never as S. + + + Warning number 12 + /.../fixtures/warning_12_untagged_literal_shadows_or_pattern.res:16:7-9 + + 14 ┆ switch x { + 15 ┆ | S("a") => 1 + 16 ┆ | S("a" | "c") => 2 + 17 ┆ | S(_) | A => 3 + 18 ┆ } + this sub-pattern is unused. "a" is the runtime value of constructor A, so it is always matched as A, never as S. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res b/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res index 43271e479c..364db46971 100644 --- a/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res +++ b/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res @@ -6,3 +6,13 @@ let f = x => | S("a" | "b") => 2 | S(_) | A => 3 } + +// With warning 11 off, the dead alternative still gets warning 12, while the +// dead case gets nothing. +@warning("-11") +let g = x => + switch x { + | S("a") => 1 + | S("a" | "c") => 2 + | S(_) | A => 3 + } From 552b55fe2fc0f876fa033503d03472a2a4a2f2a5 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 24 Oct 2026 11:39:57 +0000 Subject: [PATCH 3/7] Describe the shadowed payload warnings in the spec Rule 7 says which payload patterns a literal case shadows, which warnings report them, and how a literal case counts in the exhaustiveness check. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t Signed-off-by: Christoph Knittel --- compiler/ml/UntaggedVariants.md | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/compiler/ml/UntaggedVariants.md b/compiler/ml/UntaggedVariants.md index c40b835f77..a76c2e6fe6 100644 --- a/compiler/ml/UntaggedVariants.md +++ b/compiler/ml/UntaggedVariants.md @@ -341,10 +341,23 @@ otherwise it reaches the default through the payload dispatch, which then checks its last arm. A `null` literal that only the object check would accept adds `x !== null` to that check instead (rule 9). -- Implementation: `lam_compile.ml:compile_switch`. +A payload pattern whose every value is a literal case's can therefore never +match, and is reported: warning 11 for an arm, such as `S("a")` beside +`@as("a") A`, and warning 12 for an alternative of an or-pattern, both with +"\"a\" is the runtime value of constructor A, so it is always matched as A, +never as S". Values are compared as `===` compares them (rule 6). For the +exhaustiveness check a literal case covers the payload values equal to it, so +`Yes | Ans(false)` is a complete match of `@as(true) Yes | Ans(bool)`. + +- Implementation: `lam_compile.ml:compile_switch`; + `parmatch.ml` (shadowed payload patterns, exhaustiveness). - Tests: `spec_test` rule 7; `tests/tests/src/untagged_variants_heuristic.res` - (literals that skip the `switch`). -- Shaped by #6125, #7128, #7135. + (literals that skip the `switch`); `unboxed_variant_fold_test.res` + (`answerName2`); fixtures + `warning_11_untagged_literal_shadows_payload.res`, + `warning_11_untagged_literal_shadows_payload_kinds.res`, + `warning_12_untagged_literal_shadows_or_pattern.res`. +- Shaped by #6125, #7128, #7135, #8806. ### 8. Payload cases From bbdc52db778235a755322b8bb308a992bb2c7afc Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 25 Oct 2026 13:58:12 +0000 Subject: [PATCH 4/7] Compare bigint patterns and constants by their values Why: A bigint payload pattern equal to a bigint literal case, such as `BI(2n)` beside `@as(2n) Two`, got no warning although it can never match, and a match on a bigint constant was never folded: both treated bigints as never comparable, from before literals were compared by their normalized values. Bigint pattern constants and bigint constants have their literal, so the shadowed-payload warnings and constant folding compare them as `===` does: `02n` is `2n`. Diagnostic added (shadowed bigint payload): Example: @unboxed type t = | @as(2n) Two | BI(bigint) | S(string) let f = (v: t) => switch v { | BI(02n) => "dead" | BI(_) => "bigint" | _ => "other" } Before (diagnostic): none After (diagnostic): this match case is unused. 2n is the runtime value of constructor Two, so it is always matched as Two, never as BI. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t Signed-off-by: Christoph Knittel --- compiler/ml/UntaggedVariants.md | 7 +- compiler/ml/lambda.ml | 9 +- compiler/ml/parmatch.ml | 5 +- ...tagged_literal_shadows_bigint.res.expected | 28 ++++ ...ing_11_untagged_literal_shadows_bigint.res | 7 + .../ounit_lambda_constant_tests.ml | 23 ++- tests/tests/src/unboxed_variant_fold_test.mjs | 151 ++++++++++-------- tests/tests/src/unboxed_variant_fold_test.res | 10 ++ 8 files changed, 153 insertions(+), 87 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_bigint.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_bigint.res diff --git a/compiler/ml/UntaggedVariants.md b/compiler/ml/UntaggedVariants.md index a76c2e6fe6..5d43531232 100644 --- a/compiler/ml/UntaggedVariants.md +++ b/compiler/ml/UntaggedVariants.md @@ -345,7 +345,8 @@ A payload pattern whose every value is a literal case's can therefore never match, and is reported: warning 11 for an arm, such as `S("a")` beside `@as("a") A`, and warning 12 for an alternative of an or-pattern, both with "\"a\" is the runtime value of constructor A, so it is always matched as A, -never as S". Values are compared as `===` compares them (rule 6). For the +never as S". Values are compared as `===` compares them (rule 6), so +`BI(02n)` beside `@as(2n) Two` is reported too. For the exhaustiveness check a literal case covers the payload values equal to it, so `Yes | Ans(false)` is a complete match of `@as(true) Yes | Ans(bool)`. @@ -353,9 +354,11 @@ exhaustiveness check a literal case covers the payload values equal to it, so `parmatch.ml` (shadowed payload patterns, exhaustiveness). - Tests: `spec_test` rule 7; `tests/tests/src/untagged_variants_heuristic.res` (literals that skip the `switch`); `unboxed_variant_fold_test.res` - (`answerName2`); fixtures + (`answerName2`, `bigintOrPayloadName`); fixtures `warning_11_untagged_literal_shadows_payload.res`, `warning_11_untagged_literal_shadows_payload_kinds.res`, + `warning_11_untagged_literal_shadows_bigint.res`, + `warning_11_untagged_bool_payload_overlap.res`, `warning_12_untagged_literal_shadows_or_pattern.res`. - Shaped by #6125, #7128, #7135, #8806. diff --git a/compiler/ml/lambda.ml b/compiler/ml/lambda.ml index 71cc84490b..306a7803c0 100644 --- a/compiler/ml/lambda.ml +++ b/compiler/ml/lambda.ml @@ -825,8 +825,9 @@ let runtime_value_kind (c : structured_constant) = | Const_js_null -> Is_literal Null | Const_js_undefined _ -> Is_literal Undefined | Const_polyvar name -> Is_literal (String name) - | Const_constructor {literal = BigInt _} -> Unknown_value | Const_constructor {literal} -> Is_literal literal + | Const_bigint (sign, digits) -> + Is_literal (BigInt (Bigint_utils.to_string sign digits)) | Const_block (Blk_tuple, _) -> Is_array | Const_block (Blk_record {fields}, _) -> if record_fields_are_array fields then Is_array else Is_object @@ -836,13 +837,11 @@ let runtime_value_kind (c : structured_constant) = ), _ ) -> Is_object - | Const_char _ | Const_bigint _ | Const_some _ | Const_module_alias - | Const_assertfalse -> + | Const_char _ | Const_some _ | Const_module_alias | Const_assertfalse -> Unknown_value (* Runtime equality, rather than equality of tags: [@as(1)] and a payload - [1.0] are the same JavaScript number. Bigint spellings are not compared; - both bigint payloads and bigint constructors are classified as unknown. + [1.0] are the same JavaScript number, and [1_0n] and [10n] the same bigint. Integer tags are emitted through [Int32.of_int], just like [Const_int]. *) let literal_denotes_same = Variant_runtime.same_literal_value diff --git a/compiler/ml/parmatch.ml b/compiler/ml/parmatch.ml index beb35fce47..587baa57d8 100644 --- a/compiler/ml/parmatch.ml +++ b/compiler/ml/parmatch.ml @@ -2291,9 +2291,8 @@ let rec pattern_literal p : Variant_runtime.literal_tag option = | Tpat_constant (Const_string s) -> Some (String s) | Tpat_constant (Const_int i | Const_char i) -> Some (Int i) | Tpat_constant (Const_float f) -> Some (Float f) - | Tpat_constant (Const_bigint _) -> - (* Bigints are never compared, see [Variant_runtime.same_literal_value] *) - None + | Tpat_constant (Const_bigint (sign, digits)) -> + Some (BigInt (Bigint_utils.to_string sign digits)) | Tpat_alias (p, _, _) -> pattern_literal p | Tpat_construct (_, {cstr_name; cstr_res}, []) when is_type_path Predef.path_bool cstr_res -> diff --git a/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_bigint.res.expected b/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_bigint.res.expected new file mode 100644 index 0000000000..97df3e9b06 --- /dev/null +++ b/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_bigint.res.expected @@ -0,0 +1,28 @@ + + Warning number 4 + /.../fixtures/warning_11_untagged_literal_shadows_bigint.res:3:3-7:3 + + 1 │ @unboxed type t = | @as(2n) Two | BI(bigint) | S(string) + 2 │ let f = (v: t) => + 3 │ switch v { + 4 │  | BI(02n) => "dead" + 5 │  | BI(_) => "bigint" + 6 │  | _ => "other" + 7 │  } + 8 │ + + this pattern-matching is fragile. +It will remain exhaustive when constructors are added to type t. + + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_bigint.res:4:5-11 + + 2 │ let f = (v: t) => + 3 │ switch v { + 4 │ | BI(02n) => "dead" + 5 │ | BI(_) => "bigint" + 6 │ | _ => "other" + + this match case is unused. + 2n is the runtime value of constructor Two, so it is always matched as Two, never as BI. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_bigint.res b/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_bigint.res new file mode 100644 index 0000000000..c96bb45b6d --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_bigint.res @@ -0,0 +1,7 @@ +@unboxed type t = | @as(2n) Two | BI(bigint) | S(string) +let f = (v: t) => + switch v { + | BI(02n) => "dead" + | BI(_) => "bigint" + | _ => "other" + } diff --git a/tests/ounit_tests/ounit_lambda_constant_tests.ml b/tests/ounit_tests/ounit_lambda_constant_tests.ml index 8b674b5cba..d218cbac14 100644 --- a/tests/ounit_tests/ounit_lambda_constant_tests.ml +++ b/tests/ounit_tests/ounit_lambda_constant_tests.ml @@ -166,13 +166,22 @@ let switch_tests = ~cases:[(one, "one")] ~default:(Some "default") in - List.iter (assert_deferred sw) - [ - Const_bigint (false, "1"); - Const_constructor {name = "DecimalTen"; literal = BigInt "1_0"}; - Const_char 65; - Const_some (Const_int 1l); - ] ); + List.iter (assert_deferred sw) [Const_char 65; Const_some (Const_int 1l)] + ); + ( "bigints compare by value" >:: fun _ -> + let one = literal "One" (BigInt "1") in + let value = block "Value" UnknownType in + let sw = + variant_switch ~constructors:[one; value] + ~cases:[(one, "one")] + ~default:(Some "default") + in + assert_fold sw (Const_bigint (true, "1")) "one"; + assert_fold sw (Const_bigint (true, "01")) "one"; + assert_fold sw (Const_bigint (false, "1")) "default"; + assert_fold sw + (Const_constructor {name = "DecimalTen"; literal = BigInt "1_0"}) + "default" ); ( "tagged switch still folds by constructor" >:: fun _ -> let runtime = runtime "Color" in let case = Variant_runtime.Block (Tagged runtime) in diff --git a/tests/tests/src/unboxed_variant_fold_test.mjs b/tests/tests/src/unboxed_variant_fold_test.mjs index 3f6e07e1bd..6a4fa8d2e3 100644 --- a/tests/tests/src/unboxed_variant_fold_test.mjs +++ b/tests/tests/src/unboxed_variant_fold_test.mjs @@ -161,6 +161,14 @@ function numberOrBigintName(value) { } } +function bigintOrPayloadName(value) { + if (value === 2n) { + return "two"; + } else { + return "other"; + } +} + function id(x) { return x; } @@ -177,96 +185,98 @@ function makeRecord(value) { Mocha.describe("Unboxed_variant_fold_test", () => { Mocha.test("constructors preserve dynamic payloads and effects", () => { - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 182, characters 7-14", colorName(makeColor("primary")), "not Color"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 183, characters 7-14", colorName(makeColor("blue")), "blue"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 184, characters 7-14", recName(makeRecord(1)), "rec"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 190, characters 7-14", colorName(makeColor("primary")), "not Color"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 191, characters 7-14", colorName(makeColor("blue")), "blue"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 192, characters 7-14", recName(makeRecord(1)), "rec"); let calls = 0; calls = calls + 1 | 0; - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 190, characters 7-14", colorName("primary"), "not Color"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 191, characters 7-14", calls, 1); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 198, characters 7-14", colorName("primary"), "not Color"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 199, characters 7-14", calls, 1); }); Mocha.test("folding agrees with runtime dispatch", () => { - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 194, characters 7-14", "not Color", colorName(id("primary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 195, characters 7-14", "not Color", colorName(id("secondary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 196, characters 7-14", "blue", colorName(id("blue"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 197, characters 7-14", "not Color", colorName(id("primary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 198, characters 7-14", "one", numberName(id(1))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 199, characters 7-14", "number", numberName(id(2))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 200, characters 7-14", "one", numberName(id(1))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 201, characters 7-14", "int", pureName(id(1))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 202, characters 7-14", "string", pureName(id("x"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 203, characters 7-14", "obj", optName(id({ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 202, characters 7-14", "not Color", colorName(id("primary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 203, characters 7-14", "not Color", colorName(id("secondary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 204, characters 7-14", "blue", colorName(id("blue"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 205, characters 7-14", "not Color", colorName(id("primary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 206, characters 7-14", "one", numberName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 207, characters 7-14", "number", numberName(id(2))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 208, characters 7-14", "one", numberName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 209, characters 7-14", "int", pureName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 210, characters 7-14", "string", pureName(id("x"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 211, characters 7-14", "obj", optName(id({ x: 1 }))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 204, characters 7-14", "null", optName(id(null))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 205, characters 7-14", "undef", optName(id(undefined))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 206, characters 7-14", "str", flagName(id("true"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 207, characters 7-14", "yes", flagName(id(true))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 208, characters 7-14", "yes", answerName(id(true))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 209, characters 7-14", "answer false", answerName(id(false))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 210, characters 7-14", "yes", answerName(id(true))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 211, characters 7-14", "toggle true", mixedName(id(true))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 212, characters 7-14", "off", mixedName(id(false))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 213, characters 7-14", "off", mixedName(id(false))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 214, characters 7-14", "unset", mixedName(id(null))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 215, characters 7-14", "false", mixedName(id("false"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 216, characters 7-14", "one", numName(id(1.0))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 217, characters 7-14", "float", numName(id(2.5))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 218, characters 7-14", "one", numName(id(1))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 219, characters 7-14", "rec", recName(id({ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 212, characters 7-14", "null", optName(id(null))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 213, characters 7-14", "undef", optName(id(undefined))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 214, characters 7-14", "str", flagName(id("true"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 215, characters 7-14", "yes", flagName(id(true))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 216, characters 7-14", "yes", answerName(id(true))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 217, characters 7-14", "answer false", answerName(id(false))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 218, characters 7-14", "yes", answerName(id(true))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 219, characters 7-14", "toggle true", mixedName(id(true))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 220, characters 7-14", "off", mixedName(id(false))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 221, characters 7-14", "off", mixedName(id(false))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 222, characters 7-14", "unset", mixedName(id(null))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 223, characters 7-14", "false", mixedName(id("false"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 224, characters 7-14", "one", numName(id(1.0))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 225, characters 7-14", "float", numName(id(2.5))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 226, characters 7-14", "one", numName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 227, characters 7-14", "rec", recName(id({ y: 1 }))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 220, characters 7-14", "empty", recName(id("empty"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 221, characters 7-14", "lit", pick(id("a"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 222, characters 7-14", "z", pick(id("z"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 223, characters 7-14", "lit", pick(id("a"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 224, characters 7-14", "w", outerName(id("primary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 225, characters 7-14", "x", outerName(id("x"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 226, characters 7-14", "x", outerName(id("x"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 227, characters 7-14", "primary", boxedName(id({ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 228, characters 7-14", "empty", recName(id("empty"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 229, characters 7-14", "lit", pick(id("a"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 230, characters 7-14", "z", pick(id("z"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 231, characters 7-14", "lit", pick(id("a"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 232, characters 7-14", "w", outerName(id("primary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 233, characters 7-14", "x", outerName(id("x"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 234, characters 7-14", "x", outerName(id("x"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 235, characters 7-14", "primary", boxedName(id({ TAG: "BoxedColor", _0: "primary" }))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 228, characters 7-14", "not Color", boxedName(id("primary"))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 229, characters 7-14", "tuple", objectName(id([ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 236, characters 7-14", "not Color", boxedName(id("primary"))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 237, characters 7-14", "tuple", objectName(id([ 1, 2 ]))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 230, characters 7-14", "record", objectName(id({ + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 238, characters 7-14", "record", objectName(id({ x: 1 }))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 231, characters 7-14", "zero", listName(id(/* [] */0))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 232, characters 7-14", "list", listName(id({hd: 1, tl: /* [] */0}))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 233, characters 7-14", bigintName(10n), bigintName(id(10n))); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 234, characters 7-14", "one", wideName(id(1))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 239, characters 7-14", "zero", listName(id(/* [] */0))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 240, characters 7-14", "list", listName(id({hd: 1, tl: /* [] */0}))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 241, characters 7-14", "one", bigintName(id(10n))); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 242, characters 7-14", "one", wideName(id(1))); }); Mocha.test("the folded answers themselves are correct", () => { - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 238, characters 7-14", "not Color", "not Color"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 239, characters 7-14", "blue", "blue"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 240, characters 7-14", "one", "one"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 241, characters 7-14", "number", "number"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 242, characters 7-14", "int", "int"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 243, characters 7-14", "yes", "yes"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 244, characters 7-14", "answer false", "answer false"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 245, characters 7-14", answerName2(id(true)), "yes"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 246, characters 7-14", answerName2(id(false)), "answer false"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 247, characters 7-14", "off", "off"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 248, characters 7-14", "toggle true", "toggle true"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 249, characters 7-14", "false", "false"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 250, characters 7-14", "one", "one"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 251, characters 7-14", "lit", "lit"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 252, characters 7-14", "x", "x"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 253, characters 7-14", "primary", "primary"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 254, characters 7-14", "tuple", "tuple"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 255, characters 7-14", "zero", "zero"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 256, characters 7-14", bigintName(10n), "one"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 257, characters 7-14", "one", "one"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 258, characters 7-14", numberOrBigintName(1n), "bigint"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 246, characters 7-14", "not Color", "not Color"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 247, characters 7-14", "blue", "blue"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 248, characters 7-14", "one", "one"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 249, characters 7-14", "number", "number"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 250, characters 7-14", "int", "int"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 251, characters 7-14", "yes", "yes"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 252, characters 7-14", "answer false", "answer false"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 253, characters 7-14", answerName2(id(true)), "yes"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 254, characters 7-14", answerName2(id(false)), "answer false"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 255, characters 7-14", "off", "off"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 256, characters 7-14", "toggle true", "toggle true"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 257, characters 7-14", "false", "false"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 258, characters 7-14", "one", "one"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 259, characters 7-14", "lit", "lit"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 260, characters 7-14", "x", "x"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 261, characters 7-14", "primary", "primary"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 262, characters 7-14", "tuple", "tuple"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 263, characters 7-14", "zero", "zero"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 264, characters 7-14", "one", "one"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 265, characters 7-14", "one", "one"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 266, characters 7-14", "bigint", "bigint"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 267, characters 7-14", "two", "two"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 268, characters 7-14", "two", "two"); }); Mocha.test("a number and a bigint literal with the same digits", () => { - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 262, characters 7-14", numberOrBigintName(id(1)), "number"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 263, characters 7-14", numberOrBigintName(id(1n)), "bigint"); - Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 264, characters 7-14", numberOrBigintName(id("x")), "x"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 272, characters 7-14", numberOrBigintName(id(1)), "number"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 273, characters 7-14", numberOrBigintName(id(1n)), "bigint"); + Test_utils.eq("File \"unboxed_variant_fold_test.res\", line 274, characters 7-14", numberOrBigintName(id("x")), "x"); }); }); @@ -289,6 +299,7 @@ export { bigintName, wideName, numberOrBigintName, + bigintOrPayloadName, id, makeColor, makeRecord, diff --git a/tests/tests/src/unboxed_variant_fold_test.res b/tests/tests/src/unboxed_variant_fold_test.res index d2b1440502..14d842dabf 100644 --- a/tests/tests/src/unboxed_variant_fold_test.res +++ b/tests/tests/src/unboxed_variant_fold_test.res @@ -171,6 +171,14 @@ let numberOrBigintName = value => | Other(s) => s } +// A bigint payload equal to a bigint literal folds to the literal's case +@unboxed type bigintOrPayload = | @as(2n) BigTwo | BigOther(bigint) +let bigintOrPayloadName = value => + switch value { + | BigTwo => "two" + | BigOther(_) => "other" + } + @inline(never) let id = x => x // Nonconstant scalar payloads are passed through; inline records remain objects. @@ -256,6 +264,8 @@ describe(__MODULE__, () => { eq(__LOC__, bigintName(Wrapped(DecimalTen)), "one") eq(__LOC__, wideName(WideInt(1)), "one") eq(__LOC__, numberOrBigintName(BigintOne), "bigint") + eq(__LOC__, bigintOrPayloadName(BigOther(2n)), "two") + eq(__LOC__, bigintOrPayloadName(BigOther(02n)), "two") }) test("a number and a bigint literal with the same digits", () => { From 6fd2835468591b665ff5e73eff2d3711038b9d2b Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 25 Oct 2026 15:03:38 +0000 Subject: [PATCH 5/7] Report shadowed polymorphic variant and aliased or-pattern payloads Why: A polymorphic variant payload equal to a literal case and the alternatives of an or-pattern named with `as` got no warning, though they can never match. A polymorphic variant without payload stands for its name, or the number its name spells, as in `Lambda.const_polyvar`, and an alias is looked through when collecting or-pattern alternatives. Diagnostic added (a shadowed polymorphic variant payload): Example: @unboxed type t = | @as("Foo") F | P([#Foo | #Bar]) let f = x => switch x { | F => 1 | P(#Foo) => 2 | P(_) => 3 } Before (diagnostic): none After (diagnostic): Warning number 11 this match case is unused. "Foo" is the runtime value of constructor F, so it is always matched as F, never as P. Diagnostic added (an aliased or-pattern): Example: @unboxed type t = | @as("a") A | S(string) let f = x => switch x { | S(("a" | "b") as s) => s | S(_) | A => "other" } Before (diagnostic): none After (diagnostic): Warning number 12 this sub-pattern is unused. "a" is the runtime value of constructor A, so it is always matched as A, never as S. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t Signed-off-by: Christoph Knittel --- compiler/ml/UntaggedVariants.md | 7 ++++-- compiler/ml/parmatch.ml | 11 +++++++- ...d_literal_shadows_polyvariant.res.expected | 25 +++++++++++++++++++ ...al_shadows_aliased_or_pattern.res.expected | 12 +++++++++ ...1_untagged_literal_shadows_polyvariant.res | 12 +++++++++ ...ged_literal_shadows_aliased_or_pattern.res | 8 ++++++ 6 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_polyvariant.res.expected create mode 100644 tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_aliased_or_pattern.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_polyvariant.res create mode 100644 tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_aliased_or_pattern.res diff --git a/compiler/ml/UntaggedVariants.md b/compiler/ml/UntaggedVariants.md index 5d43531232..dc6f0b0b7d 100644 --- a/compiler/ml/UntaggedVariants.md +++ b/compiler/ml/UntaggedVariants.md @@ -346,7 +346,8 @@ match, and is reported: warning 11 for an arm, such as `S("a")` beside `@as("a") A`, and warning 12 for an alternative of an or-pattern, both with "\"a\" is the runtime value of constructor A, so it is always matched as A, never as S". Values are compared as `===` compares them (rule 6), so -`BI(02n)` beside `@as(2n) Two` is reported too. For the +`BI(02n)` beside `@as(2n) Two` and `P(#Foo)` beside `@as("Foo") F` are +reported too, and an or-pattern also counts when it is named with `as`. For the exhaustiveness check a literal case covers the payload values equal to it, so `Yes | Ans(false)` is a complete match of `@as(true) Yes | Ans(bool)`. @@ -359,7 +360,9 @@ exhaustiveness check a literal case covers the payload values equal to it, so `warning_11_untagged_literal_shadows_payload_kinds.res`, `warning_11_untagged_literal_shadows_bigint.res`, `warning_11_untagged_bool_payload_overlap.res`, - `warning_12_untagged_literal_shadows_or_pattern.res`. + `warning_11_untagged_literal_shadows_polyvariant.res`, + `warning_12_untagged_literal_shadows_or_pattern.res`, + `warning_12_untagged_literal_shadows_aliased_or_pattern.res`. - Shaped by #6125, #7128, #7135, #8806. ### 8. Payload cases diff --git a/compiler/ml/parmatch.ml b/compiler/ml/parmatch.ml index 587baa57d8..507fc53c3a 100644 --- a/compiler/ml/parmatch.ml +++ b/compiler/ml/parmatch.ml @@ -2294,6 +2294,12 @@ let rec pattern_literal p : Variant_runtime.literal_tag option = | Tpat_constant (Const_bigint (sign, digits)) -> Some (BigInt (Bigint_utils.to_string sign digits)) | Tpat_alias (p, _, _) -> pattern_literal p + | Tpat_variant (label, None, _) -> + (* A polymorphic variant is its name, or the number its name spells, as + in [Lambda.const_polyvar] *) + if Ext_string.is_valid_hash_number label then + Some (Int (Int32.to_int (Ext_string.hash_number_as_i32_exn label))) + else Some (String label) | Tpat_construct (_, {cstr_name; cstr_res}, []) when is_type_path Predef.path_bool cstr_res -> Some (Bool (cstr_name = "true")) @@ -2310,7 +2316,8 @@ let rec pattern_literal p : Variant_runtime.literal_tag option = pattern_literal arg | (Constant _ | Block _), _ -> None) | Tpat_construct (_, {cstr_kind = Extension_constructor _}, _) - | Tpat_any | Tpat_var _ | Tpat_interval _ | Tpat_tuple _ | Tpat_variant _ + | Tpat_any | Tpat_var _ | Tpat_interval _ | Tpat_tuple _ + | Tpat_variant (_, Some _, _) | Tpat_record _ | Tpat_dict _ | Tpat_array _ | Tpat_or _ -> None @@ -2367,6 +2374,8 @@ and shadowed_argument_liveness ~payload_constructor ~literal_cases arg = or_liveness (shadowed_argument_liveness ~payload_constructor ~literal_cases) p1 p2 + | Tpat_alias (p, _, _) -> + shadowed_argument_liveness ~payload_constructor ~literal_cases p | _ -> ( let shadowing_case = match pattern_literal arg with diff --git a/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_polyvariant.res.expected b/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_polyvariant.res.expected new file mode 100644 index 0000000000..c11a0b3199 --- /dev/null +++ b/tests/build_tests/super_errors/expected/warning_11_untagged_literal_shadows_polyvariant.res.expected @@ -0,0 +1,25 @@ + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_polyvariant.res:9:5-11 + + 7 ┆ | F => 1 + 8 ┆ | One => 2 + 9 ┆ | P(#Foo) => 3 + 10 ┆ | P(#1) => 4 + 11 ┆ | P(_) => 5 + + this match case is unused. + "Foo" is the runtime value of constructor F, so it is always matched as F, never as P. + + + Warning number 11 + /.../fixtures/warning_11_untagged_literal_shadows_polyvariant.res:10:5-9 + + 8 ┆ | One => 2 + 9 ┆ | P(#Foo) => 3 + 10 ┆ | P(#1) => 4 + 11 ┆ | P(_) => 5 + 12 ┆ } + + this match case is unused. + 1 is the runtime value of constructor One, so it is always matched as One, never as P. \ No newline at end of file diff --git a/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_aliased_or_pattern.res.expected b/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_aliased_or_pattern.res.expected new file mode 100644 index 0000000000..d89b2eeb44 --- /dev/null +++ b/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_aliased_or_pattern.res.expected @@ -0,0 +1,12 @@ + + Warning number 12 + /.../fixtures/warning_12_untagged_literal_shadows_aliased_or_pattern.res:6:8-10 + + 4 │ let f = x => + 5 │ switch x { + 6 │ | S(("a" | "b") as s) => s + 7 │ | S(_) | A => "other" + 8 │ } + + this sub-pattern is unused. + "a" is the runtime value of constructor A, so it is always matched as A, never as S. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_polyvariant.res b/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_polyvariant.res new file mode 100644 index 0000000000..dac846c0b6 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/warning_11_untagged_literal_shadows_polyvariant.res @@ -0,0 +1,12 @@ +// A polymorphic variant payload is its name at run time, so #Foo is always +// matched as F, and #1 as One. +@unboxed type t = | @as("Foo") F | @as(1) One | P([#Foo | #Bar | #1]) + +let f = x => + switch x { + | F => 1 + | One => 2 + | P(#Foo) => 3 + | P(#1) => 4 + | P(_) => 5 + } diff --git a/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_aliased_or_pattern.res b/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_aliased_or_pattern.res new file mode 100644 index 0000000000..daf818af56 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_aliased_or_pattern.res @@ -0,0 +1,8 @@ +// The "a" alternative of an aliased or-pattern can never match either. +@unboxed type t = | @as("a") A | S(string) + +let f = x => + switch x { + | S(("a" | "b") as s) => s + | S(_) | A => "other" + } From 7c05b9a79bdd02497bd398b788a1b8881d75854f Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 25 Oct 2026 16:51:05 +0000 Subject: [PATCH 6/7] Cover payloads by a bool literal only when they are bools Why: The exhaustiveness check let a bool literal case cover the equal value of any boolean-kind payload, including a `@typeOf("boolean")` type that is not `bool`. The synthesized `true` pattern did not fit that type, so a match missing the payload was taken as complete. Only a payload whose type expands to `bool` gets the synthesized pattern. Diagnostic added (a @typeOf("boolean") payload): Example: @typeOf("boolean") type jsBool @unboxed type t = | @as(true) Yes | Value(jsBool) let f = (v: t) => switch v { | Yes => 1 } Before (diagnostic): none After (diagnostic): Warning number 8 You forgot to handle a possible case here, for example: | Value(_) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t Signed-off-by: Christoph Knittel --- compiler/ml/UntaggedVariants.md | 6 +++++- compiler/ml/parmatch.ml | 11 +++++++---- ...ng_8_untagged_boolean_kind_payload.res.expected | 14 ++++++++++++++ .../warning_8_untagged_boolean_kind_payload.res | 14 ++++++++++++++ 4 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/warning_8_untagged_boolean_kind_payload.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/warning_8_untagged_boolean_kind_payload.res diff --git a/compiler/ml/UntaggedVariants.md b/compiler/ml/UntaggedVariants.md index dc6f0b0b7d..0958b72efc 100644 --- a/compiler/ml/UntaggedVariants.md +++ b/compiler/ml/UntaggedVariants.md @@ -349,7 +349,10 @@ never as S". Values are compared as `===` compares them (rule 6), so `BI(02n)` beside `@as(2n) Two` and `P(#Foo)` beside `@as("Foo") F` are reported too, and an or-pattern also counts when it is named with `as`. For the exhaustiveness check a literal case covers the payload values equal to it, so -`Yes | Ans(false)` is a complete match of `@as(true) Yes | Ans(bool)`. +`Yes | Ans(false)` is a complete match of `@as(true) Yes | Ans(bool)`. This +needs a payload of type `bool`, also through an abbreviation: a +`@typeOf("boolean")` type has no `true` and `false` patterns, so `Yes` alone +does not cover it. - Implementation: `lam_compile.ml:compile_switch`; `parmatch.ml` (shadowed payload patterns, exhaustiveness). @@ -360,6 +363,7 @@ exhaustiveness check a literal case covers the payload values equal to it, so `warning_11_untagged_literal_shadows_payload_kinds.res`, `warning_11_untagged_literal_shadows_bigint.res`, `warning_11_untagged_bool_payload_overlap.res`, + `warning_8_untagged_boolean_kind_payload.res`, `warning_11_untagged_literal_shadows_polyvariant.res`, `warning_12_untagged_literal_shadows_or_pattern.res`, `warning_12_untagged_literal_shadows_aliased_or_pattern.res`. diff --git a/compiler/ml/parmatch.ml b/compiler/ml/parmatch.ml index 507fc53c3a..b981167fc9 100644 --- a/compiler/ml/parmatch.ml +++ b/compiler/ml/parmatch.ml @@ -2558,13 +2558,16 @@ let rec with_bool_payload_overlaps p = | Constant tag -> ( match tag.literal with | Bool b -> ( + (* The payload type must be [bool] itself: a [@typeOf("boolean")] + type has boolean values, but no [true] and [false] patterns *) let is_bool_payload cd = - match cd.cstr_kind with - | Ordinary_constructor r -> ( + match (cd.cstr_kind, cd.cstr_args) with + | Ordinary_constructor r, [arg] -> ( match Variant_runtime.representation r with - | Block (Untagged {block_type = BooleanType}) -> true + | Block (Untagged {block_type = BooleanType}) -> + is_type_path Predef.path_bool (Ctype.expand_head p.pat_env arg) | Constant _ | Block _ -> false) - | Extension_constructor _ -> false + | Ordinary_constructor _, _ | Extension_constructor _, _ -> false in match List.find_opt is_bool_payload diff --git a/tests/build_tests/super_errors/expected/warning_8_untagged_boolean_kind_payload.res.expected b/tests/build_tests/super_errors/expected/warning_8_untagged_boolean_kind_payload.res.expected new file mode 100644 index 0000000000..cced36fcc6 --- /dev/null +++ b/tests/build_tests/super_errors/expected/warning_8_untagged_boolean_kind_payload.res.expected @@ -0,0 +1,14 @@ + + Warning number 8 + /.../fixtures/warning_8_untagged_boolean_kind_payload.res:4:3-6:3 + + 2 │ @unboxed type t = | @as(true) Yes | Value(jsBool) + 3 │ let f = (v: t) => + 4 │ switch v { + 5 │  | Yes => 1 + 6 │  } + 7 │ + 8 │ type alias = bool + + You forgot to handle a possible case here, for example: + | Value(_) \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/warning_8_untagged_boolean_kind_payload.res b/tests/build_tests/super_errors/fixtures/warning_8_untagged_boolean_kind_payload.res new file mode 100644 index 0000000000..d3b18fd280 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/warning_8_untagged_boolean_kind_payload.res @@ -0,0 +1,14 @@ +@typeOf("boolean") type jsBool +@unboxed type t = | @as(true) Yes | Value(jsBool) +let f = (v: t) => + switch v { + | Yes => 1 + } + +type alias = bool +@unboxed type u = | @as(true) Yes2 | Value2(alias) +let g = (v: u) => + switch v { + | Yes2 => 1 + | Value2(false) => 2 + } From 8cb50ef57a2ee7be870c01da008ee6c4adbfacad Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 25 Oct 2026 16:58:35 +0000 Subject: [PATCH 7/7] Keep outer literal cases when looking into nested untagged payloads Why: Below a second untagged constructor, an or-pattern was checked against the inner variant's literal cases only, although the outer variant's literal cases are tested first on the same value. The literal cases that take precedence are carried down through untagged payloads, outermost first, each with the payload constructor it shadows. Diagnostic added (an or-pattern below a nested untagged payload): Example: @unboxed type inner = | @as("x") X | N(int) | S(string) @unboxed type outer = | @as(1) One | W(inner) let f = v => switch v { | One => "one" | W(N(1 | 2)) => "small" | W(_) => "other" } Before (diagnostic): none After (diagnostic): Warning number 12 this sub-pattern is unused. 1 is the runtime value of constructor One, so it is always matched as One, never as W. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t Signed-off-by: Christoph Knittel --- compiler/ml/UntaggedVariants.md | 7 +- compiler/ml/parmatch.ml | 64 +++++++++++-------- ...iteral_shadows_nested_payload.res.expected | 54 ++++++++++++++++ ...ntagged_literal_shadows_nested_payload.res | 14 ++++ 4 files changed, 112 insertions(+), 27 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_nested_payload.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_nested_payload.res diff --git a/compiler/ml/UntaggedVariants.md b/compiler/ml/UntaggedVariants.md index 0958b72efc..cd1a7093c8 100644 --- a/compiler/ml/UntaggedVariants.md +++ b/compiler/ml/UntaggedVariants.md @@ -347,7 +347,9 @@ match, and is reported: warning 11 for an arm, such as `S("a")` beside "\"a\" is the runtime value of constructor A, so it is always matched as A, never as S". Values are compared as `===` compares them (rule 6), so `BI(02n)` beside `@as(2n) Two` and `P(#Foo)` beside `@as("Foo") F` are -reported too, and an or-pattern also counts when it is named with `as`. For the +reported too, and an or-pattern also counts when it is named with `as`. An +untagged payload inside another is the value itself, so the outer literal +cases also shadow it: `W(N(1 | 2))` reports `1` beside `@as(1) One | W(inner)`. For the exhaustiveness check a literal case covers the payload values equal to it, so `Yes | Ans(false)` is a complete match of `@as(true) Yes | Ans(bool)`. This needs a payload of type `bool`, also through an abbreviation: a @@ -366,7 +368,8 @@ does not cover it. `warning_8_untagged_boolean_kind_payload.res`, `warning_11_untagged_literal_shadows_polyvariant.res`, `warning_12_untagged_literal_shadows_or_pattern.res`, - `warning_12_untagged_literal_shadows_aliased_or_pattern.res`. + `warning_12_untagged_literal_shadows_aliased_or_pattern.res`, + `warning_12_untagged_literal_shadows_nested_payload.res`. - Shaped by #6125, #7128, #7135, #8806. ### 8. Payload cases diff --git a/compiler/ml/parmatch.ml b/compiler/ml/parmatch.ml index b981167fc9..09ef24f2dd 100644 --- a/compiler/ml/parmatch.ml +++ b/compiler/ml/parmatch.ml @@ -2321,21 +2321,32 @@ let rec pattern_literal p : Variant_runtime.literal_tag option = | Tpat_record _ | Tpat_dict _ | Tpat_array _ | Tpat_or _ -> None -(* The literal cases that take precedence over an untagged payload constructor, - as runtime values and constructor names *) -let shadowing_literal_cases cd = +let is_untagged_payload cd = match cd.cstr_kind with | Ordinary_constructor r -> ( match Variant_runtime.representation r with - | Block (Untagged _) -> - let layout = Variant_runtime.get_layout r.variant in - List.init (Variant_runtime.length layout) Fun.id - |> List.filter_map (fun position -> - match Variant_runtime.constructor_at layout position with - | Constant tag -> Some (tag.literal, tag.name) - | Block _ -> None) - | Constant _ | Block (Tagged _) -> []) - | Extension_constructor _ -> [] + | Block (Untagged _) -> true + | Constant _ | Block (Tagged _) -> false) + | Extension_constructor _ -> false + +(* The literal cases that take precedence over an untagged payload + constructor *) +let shadowing_literal_cases cd = + match cd.cstr_kind with + | Ordinary_constructor r when is_untagged_payload cd -> + let layout = Variant_runtime.get_layout r.variant in + List.init (Variant_runtime.length layout) Fun.id + |> List.filter_map (fun position -> + match Variant_runtime.constructor_at layout position with + | Constant tag -> + Some + { + payload_constructor = cd.cstr_name; + value = tag.literal; + literal_constructor = tag.name; + } + | Block _ -> None) + | Ordinary_constructor _ | Extension_constructor _ -> [] let or_liveness liveness p1 p2 = match (liveness p1, liveness p2) with @@ -2350,9 +2361,7 @@ let rec payload_liveness p = | Tpat_construct (_, cd, [arg]) -> ( match shadowing_literal_cases cd with | [] -> subpatterns_liveness p - | literal_cases -> - shadowed_argument_liveness ~payload_constructor:cd.cstr_name - ~literal_cases arg) + | shadowing -> shadowed_argument_liveness ~shadowing arg) | _ -> subpatterns_liveness p (* A pattern that is not an or-pattern matches nothing as soon as one of its @@ -2368,26 +2377,31 @@ and subpatterns_liveness p = p.pat_desc; !result -and shadowed_argument_liveness ~payload_constructor ~literal_cases arg = +(* [shadowing] lists the literal cases that take precedence over the value + [arg] matches, outermost first *) +and shadowed_argument_liveness ~shadowing arg = match arg.pat_desc with | Tpat_or (p1, p2, _) -> - or_liveness - (shadowed_argument_liveness ~payload_constructor ~literal_cases) - p1 p2 - | Tpat_alias (p, _, _) -> - shadowed_argument_liveness ~payload_constructor ~literal_cases p + or_liveness (shadowed_argument_liveness ~shadowing) p1 p2 + | Tpat_alias (p, _, _) -> shadowed_argument_liveness ~shadowing p + | Tpat_construct (_, cd, [inner]) when is_untagged_payload cd -> + (* An untagged payload is the value itself, so the literal cases around + it still take precedence, before its own *) + shadowed_argument_liveness + ~shadowing:(shadowing @ shadowing_literal_cases cd) + inner | _ -> ( let shadowing_case = match pattern_literal arg with | Some value -> List.find_opt - (fun (literal, _) -> Variant_runtime.same_literal_value value literal) - literal_cases + (fun {value = literal} -> + Variant_runtime.same_literal_value value literal) + shadowing | None -> None in match shadowing_case with - | Some (value, literal_constructor) -> - Never_matches [{payload_constructor; value; literal_constructor}] + | Some shadowed -> Never_matches [shadowed] | None -> payload_liveness arg) let literal_to_string : Variant_runtime.literal_tag -> string = function diff --git a/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_nested_payload.res.expected b/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_nested_payload.res.expected new file mode 100644 index 0000000000..633f94d305 --- /dev/null +++ b/tests/build_tests/super_errors/expected/warning_12_untagged_literal_shadows_nested_payload.res.expected @@ -0,0 +1,54 @@ + + Warning number 4 + /.../fixtures/warning_12_untagged_literal_shadows_nested_payload.res:8:3-14:3 + + 6 │ + 7 │ let f = v => + 8 │ switch v { + 9 │  | One => "one" + . │ ... + 13 │  | W(_) => "other" + 14 │  } + 15 │ + + this pattern-matching is fragile. +It will remain exhaustive when constructors are added to type inner. + + + Warning number 12 + /.../fixtures/warning_12_untagged_literal_shadows_nested_payload.res:10:9 + + 8 ┆ switch v { + 9 ┆ | One => "one" + 10 ┆ | W(N(1 | 2)) => "small" + 11 ┆ | W(S("x" | "y")) => "letter" + 12 ┆ | W(N(1)) => "dead" + + this sub-pattern is unused. + 1 is the runtime value of constructor One, so it is always matched as One, never as W. + + + Warning number 12 + /.../fixtures/warning_12_untagged_literal_shadows_nested_payload.res:11:9-11 + + 9 ┆ | One => "one" + 10 ┆ | W(N(1 | 2)) => "small" + 11 ┆ | W(S("x" | "y")) => "letter" + 12 ┆ | W(N(1)) => "dead" + 13 ┆ | W(_) => "other" + + this sub-pattern is unused. + "x" is the runtime value of constructor X, so it is always matched as X, never as S. + + + Warning number 11 + /.../fixtures/warning_12_untagged_literal_shadows_nested_payload.res:12:5-11 + + 10 ┆ | W(N(1 | 2)) => "small" + 11 ┆ | W(S("x" | "y")) => "letter" + 12 ┆ | W(N(1)) => "dead" + 13 ┆ | W(_) => "other" + 14 ┆ } + + this match case is unused. + 1 is the runtime value of constructor One, so it is always matched as One, never as W. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_nested_payload.res b/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_nested_payload.res new file mode 100644 index 0000000000..b144e13028 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_nested_payload.res @@ -0,0 +1,14 @@ +// An untagged payload inside another is the value itself, so the outer +// literal cases take precedence over it: 1 is always matched as One, and "x" +// as X inside W. +@unboxed type inner = | @as("x") X | N(int) | S(string) +@unboxed type outer = | @as(1) One | W(inner) + +let f = v => + switch v { + | One => "one" + | W(N(1 | 2)) => "small" + | W(S("x" | "y")) => "letter" + | W(N(1)) => "dead" + | W(_) => "other" + }