Switch defensive raise sites to assert false; revert typemod/Misplaced_label_syntax

PR-review feedback: replacing dead raise sites with Location.raise_errorf
moves code rather than removing it, and introduces a pattern that doesn't
exist anywhere else in the typer modules (which use raise (Error ...) for
real errors and assert false for unreachable defensive code; zero inline
Location.raise_errorf calls in master).

This commit aligns the cleanup with the existing convention:

1. Variants whose raise sites are confirmed unreachable from the ReScript
   parser have those sites rewritten to `assert false (* reason *)` with
   a comment citing the parser block or sibling check. The named variant
   (decl + reporter) stays removed.

   - typecore.Invalid_interval, Invalid_for_of_pattern (parser blocks /
     normalizes the AST)
   - typetexp.Unbound_type_constructor_2, Ill_typed_functor_application,
     Apply_structure_as_functor (parser doesn't construct Lapply or
     bare-Tvar Tconstr body)
   - typedecl.Type_clash, Parameters_differ (Cycle_in_def fires first),
     Null_arity_external (Primitive.parse_declaration sets prim_native_name
     always), Bad_fixed_type (is_fixed_type and expand_head agree),
     Varying_anonymous (parser rejects `_` type params), Val_in_structure
     (parser rejects val outside .resi)
   - bs_syntaxerr.Unhandled_poly_type (parser misreads inline 'a. as
     deprecated uncurried syntax)
   - env.Illegal_value_name (parser doesn't emit '->' or # identifiers)
   - typecore.Incoherent_label_order: collapsed into Apply_wrong_label
     with a comment explaining why the original second arm was
     unreachable

2. Variants where reproduction confirmed they ARE reachable, or where I
   couldn't prove unreachability, are restored to named-variant form:

   - typemod.Cannot_eliminate_dependency (couldn't reproduce, but
     reachable from non-generative functor application paths in
     principle - retained conservatively)
   - typemod.With_makes_applicative_functor_ill_typed (couldn't reproduce
     - retained conservatively)
   - typemod.With_cannot_remove_constrained_type (REACHABLE via
     destructive substitution on a constrained type, fixture
     with_cannot_remove_constrained_type.res)
   - typemod.Scoping_pack (couldn't reproduce - retained conservatively)
   - bs_syntaxerr.Misplaced_label_syntax (REACHABLE via operator-as-
     identifier syntax with labelled arg, fixture
     misplaced_label_syntax.res)

3. tests/ERROR_VARIANTS.md updated to (a) split the Removed section into
   "Truly dead" and "Defensive unreachable" categories, (b) note the
   restored typemod variants with `?` status, (c) add covered rows for
   the two new fixtures.

Truly-dead removals (no raise site, transitively dead, or always-false
guard) are unchanged: 3 Ctype exceptions, 2 relay variants
(typecore.Recursive_local_constraint, typetexp.Variant_tags),
bs_syntaxerr.Conflict_bs_bs_this_bs_meth, 14 warning constructors.

Validation: rg confirmed no remaining references to the removed
variants. make compiler, super_errors (745 fixtures), super_errors_multi
(54 fixtures) all pass. make checkformat clean.

Author: JonoPrest

compiler/frontend/ast_core_type.ml

@@ -136,8 +136,12 @@ let list_of_arrow (ty : t) : t * Parsetree.arg list =
     match ty.ptyp_desc with
     | Ptyp_arrow {arg; ret; arity} when arity = None || acc = [] ->
       aux ret (arg :: acc)
-    | Ptyp_poly (_, ty) ->
-      Location.raise_errorf ~loc:ty.ptyp_loc "Unhandled poly type"
+    | Ptyp_poly _ ->
+      (* unreachable: this would require an inline Ptyp_poly inside an
+         external's arrow chain. The ReScript parser misreads inline `'a.`
+         prefix syntax as the deprecated uncurried `(. …)` form and rejects
+         it, so the typer never sees the required AST shape. *)
+      assert false
     | _ -> (ty, List.rev acc)
   in
   aux ty []

compiler/frontend/bs_syntaxerr.ml

@@ -46,6 +46,7 @@ type error =
   | Bs_this_simple_pattern
   | Experimental_feature_not_enabled of Experimental_features.feature
   | LetUnwrap_not_supported_in_position of [`Toplevel | `Unsupported_type]
+  | Misplaced_label_syntax
 
 let pp_error fmt err =
   Format.pp_print_string fmt
@@ -86,7 +87,8 @@ let pp_error fmt err =
       | `Toplevel -> "`let?` is not allowed for top-level bindings."
       | `Unsupported_type ->
         "`let?` is only supported in let bindings targeting the `result` or \
-         `option` type."))
+         `option` type.")
+    | Misplaced_label_syntax -> "Label syntax is not supported in this position")
 
 type exn += Error of Location.t * error
 
@@ -103,5 +105,4 @@ let optional_err loc (lbl : Asttypes.arg_label) =
   | _ -> ()
 
 let err_if_label loc (lbl : Asttypes.arg_label) =
-  if lbl <> Nolabel then
-    Location.raise_errorf ~loc "Label syntax is not supported in this position"
+  if lbl <> Nolabel then raise (Error (loc, Misplaced_label_syntax))

compiler/frontend/bs_syntaxerr.mli

@@ -46,6 +46,7 @@ type error =
   | Bs_this_simple_pattern
   | Experimental_feature_not_enabled of Experimental_features.feature
   | LetUnwrap_not_supported_in_position of [`Toplevel | `Unsupported_type]
+  | Misplaced_label_syntax
 
 val err : Location.t -> error -> 'a
 

compiler/ml/env.ml

@@ -1616,13 +1616,16 @@ and check_usage loc id warn tbl =
 and check_value_name name loc =
   (* Note: we could also check here general validity of the
      identifier, to protect against bad identifiers forged by -pp or
-     -ppx preprocessors. *)
-  if name = "->" then
-    Location.raise_errorf ~loc "'%s' is not a valid value identifier." name
+     -ppx preprocessors.
+
+     Both guarded paths below are unreachable: the ReScript parser never
+     emits value identifiers named "->" or containing "#" — both shapes
+     are rejected as syntax errors. *)
+  ignore loc;
+  if name = "->" then assert false
   else if String.length name > 0 && name.[0] = '#' then
     for i = 1 to String.length name - 1 do
-      if name.[i] = '#' then
-        Location.raise_errorf ~loc "'%s' is not a valid value identifier." name
+      if name.[i] = '#' then assert false
     done
 
 and store_value ?check id decl env =

compiler/ml/typecore.ml

@@ -1336,8 +1336,10 @@ and type_pat_aux ~constrs ~labels ~no_existentials ~mode ~explode ~env sp
     type_pat ~explode:0 p expected_ty k
     (* TODO: record 'extra' to remember about interval *)
   | Ppat_interval _ ->
-    Location.raise_errorf ~loc
-      "Only character intervals are supported in patterns."
+    (* unreachable: the ReScript parser (compiler/syntax/src/res_core.ml) has
+       no construction site for Ppat_interval — interval patterns are OCaml
+       syntax not part of the ReScript grammar *)
+    assert false
   | Ppat_tuple spl ->
     assert (List.length spl >= 2);
     let spl_ann = List.map (fun p -> (p, newvar ())) spl in
@@ -3088,8 +3090,11 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected)
             Types.val_loc = loc;
           } env ~check:(fun s -> Warnings.Unused_for_index s)
       | _ ->
-        Location.raise_errorf ~loc:param.ppat_loc
-          "Invalid for...of binding: only variables and _ are allowed."
+        (* unreachable: the parser's normalize_for_of_pattern
+           (compiler/syntax/src/res_core.ml:3841) catches every non-var,
+           non-`_` pattern, emits a syntax error, and replaces the pattern
+           with Ppat_any before the typer runs *)
+        assert false
     in
     let body =
       with_depth loop_depth (fun () ->
@@ -3122,8 +3127,11 @@ and type_expect_ ?deprecated_context ~context ?in_function ?(recarg = Rejected)
             Types.val_loc = loc;
           } env ~check:(fun s -> Warnings.Unused_for_index s)
       | _ ->
-        Location.raise_errorf ~loc:param.ppat_loc
-          "Invalid for...of binding: only variables and _ are allowed."
+        (* unreachable: the parser's normalize_for_of_pattern
+           (compiler/syntax/src/res_core.ml:3841) catches every non-var,
+           non-`_` pattern, emits a syntax error, and replaces the pattern
+           with Ppat_any before the typer runs *)
+        assert false
     in
     let body =
       with_depth loop_depth (fun () ->
@@ -3849,6 +3857,12 @@ and type_application ~context total_app env funct (sargs : sargs) :
                 (Error
                    (sarg1.pexp_loc, env, Apply_wrong_label (l1, funct.exp_type)))
             else
+              (* Originally split between Apply_wrong_label (label not in
+                 ty_fun) and Incoherent_label_order (label in ty_fun but at
+                 a different position). The latter is unreachable: modern
+                 arity-aware unify in type_function eagerly compares against
+                 ty_expected, raising Expr_type_clash before this branch
+                 fires. Both label problems now surface as Apply_wrong_label. *)
               raise
                 (Error (sarg1.pexp_loc, env, Apply_wrong_label (l1, ty_res)))
           | _ ->

compiler/ml/typedecl.ml

@@ -108,7 +108,7 @@ let enter_type rec_flag env sdecl id =
     in
     Env.add_type ~check:true id decl env
 
-let update_type temp_env env id loc =
+let update_type temp_env env id _loc =
   let path = Path.Pident id in
   let decl = Env.find_type path temp_env in
   match decl.type_manifest with
@@ -117,8 +117,10 @@ let update_type temp_env env id loc =
     let params = List.map (fun _ -> Ctype.newvar ()) decl.type_params in
     try Ctype.unify env (Ctype.newconstr path params) ty
     with Ctype.Unify _ ->
-      Location.raise_errorf ~loc
-        "This type constructor expands to an incompatible type.")
+      (* unreachable: every recursive abbreviation shape that would reach
+         this unify failure hits Cycle_in_def in check_recursive_type
+         (see line ~902 below) before check_coherence runs *)
+      assert false)
 
 (* We use the Ctype.expand_head_opt version of expand_head to get access
    to the manifest type of private abbreviations. *)
@@ -170,7 +172,7 @@ let is_fixed_type sd =
     && sd.ptype_private = Private && has_row_var sty
 
 (* Set the row variable in a fixed type *)
-let set_fixed_row env loc p decl =
+let set_fixed_row env _loc p decl =
   let tm =
     match decl.type_manifest with
     | None -> assert false
@@ -184,10 +186,16 @@ let set_fixed_row env loc p decl =
       if Btype.static_row row then Btype.newgenty Tnil else row.row_more
     | Tobject (ty, _) -> snd (Ctype.flatten_fields ty)
     | _ ->
-      Location.raise_errorf ~loc "This fixed type is not an object or variant"
+      (* unreachable: gated by `is_fixed_type decl`, which only returns true
+         when the syntactic manifest carries an open polymorphic-variant or
+         object row. `expand_head` preserves that row, so the manifest's
+         desc is always Tvariant or Tobject here. *)
+      assert false
   in
   if not (Btype.is_Tvar rv) then
-    Location.raise_errorf ~loc "This fixed type has no row variable";
+    (* unreachable: same is_fixed_type invariant — the row is always a Tvar
+       (or extended-Tvar) when is_fixed_type passes *)
+    assert false;
   rv.desc <- Tconstr (p, decl.type_params, ref Mnil)
 
 (* Translate one type declaration *)
@@ -978,10 +986,14 @@ let check_recursion env loc path decl to_check =
         match ty.desc with
         | Tconstr (path', args', _) ->
           (if Path.same path path' then (
-             if not (Ctype.equal env false args args') then
-               Location.raise_errorf ~loc
-                 "In the definition of %s, recursive type parameters differ."
-                 (Path.name cpath))
+             if not (Ctype.equal env false args args') then (
+               (* unreachable: check_regular runs only on abbreviations,
+                  and every recursive-abbreviation shape hits Cycle_in_def
+                  in the earlier check_recursive_type pass before reaching
+                  here. Variant constructors with non-uniform recursion
+                  (`type rec t<'a> = T(t<int>)`) don't trigger check_regular. *)
+               ignore cpath;
+               assert false))
            else if
              (* Attempt to expand a type abbreviation if:
                  1- [to_check path'] holds
@@ -1234,7 +1246,7 @@ let for_constr = function
       (fun {Types.ld_mutable; ld_type} -> (ld_mutable = Mutable, ld_type))
       l
 
-let compute_variance_gadt env check ((required, loc) as rloc) decl
+let compute_variance_gadt env check ((required, _loc) as rloc) decl
     (tl, ret_type_opt) =
   match ret_type_opt with
   | None ->
@@ -1255,9 +1267,11 @@ let compute_variance_gadt env check ((required, loc) as rloc) decl
             | fv :: fv2 ->
               (* fv1 @ fv2 = free_variables of other parameters *)
               if (c || n) && constrained (fv1 @ fv2) ty then
-                Location.raise_errorf ~loc
-                  "In this GADT definition, the variance of some parameter \
-                   cannot be checked.";
+                (* unreachable: this would fire on a GADT parameter that's
+                   `_` (anonymous). ReScript's parser rejects `_` in
+                   `type t<...>` parameter positions, so the typer never
+                   sees the required AST shape. *)
+                assert false;
               (fv :: fv1, fv2))
           ([], fvl) tyl required
       in
@@ -1882,8 +1896,12 @@ let transl_value_decl env loc valdecl =
         val_attributes = valdecl.pval_attributes;
       }
     | [] ->
-      Location.raise_errorf ~loc:valdecl.pval_loc
-        "Value declarations are only allowed in signatures"
+      (* unreachable: `pval_prim = []` outside a signature can only arise
+         from the parser's `external` recovery, which sets `prim = []`
+         *after* emitting a syntax error, so the typer never sees the
+         declaration. A bare `val x: int` in a .res is also rejected at
+         parse time. *)
+      assert false
     | _ ->
       let arity, from_constructor = parse_arity env valdecl.pval_type ty in
       let prim = Primitive.parse_declaration valdecl ~arity ~from_constructor in
@@ -1897,8 +1915,12 @@ let transl_value_decl env loc valdecl =
         && (prim.prim_name = ""
            || (prim.prim_name.[0] <> '%' && prim.prim_name.[0] <> '#'))
       then
-        Location.raise_errorf ~loc:valdecl.pval_type.ptyp_loc
-          "External identifiers must be functions";
+        (* unreachable: Primitive.parse_declaration always assigns the
+           magic 20-byte prim_native_name encoding to externals; the
+           `prim_native_name = ""` precondition can't be satisfied alongside
+           `prim_arity = 0` from any externals that survive parsing. Empty
+           prim names are rejected earlier with `Not a valid global name`. *)
+        assert false;
       {
         val_type = ty;
         val_kind = Val_prim prim;

compiler/ml/typemod.ml

@@ -24,18 +24,23 @@ open Format
 type error =
   | Cannot_apply of module_type
   | Not_included of Includemod.error list
+  | Cannot_eliminate_dependency of module_type
   | Signature_expected
   | Structure_expected of module_type
   | With_no_component of Longident.t
   | With_mismatch of Longident.t * Includemod.error list
+  | With_makes_applicative_functor_ill_typed of
+      Longident.t * Path.t * Includemod.error list
   | With_changes_module_alias of Longident.t * Ident.t * Path.t
+  | With_cannot_remove_constrained_type
   | Repeated_name of string * string * Warnings.loc
   | Non_generalizable of type_expr
   | Non_generalizable_module of module_type
   | Interface_not_compiled of string
   | Not_allowed_in_functor_body
   | Not_a_packed_module of type_expr
   | Incomplete_packed_module of type_expr
+  | Scoping_pack of Longident.t * type_expr
   | Recursive_module_require_explicit_type
   | Apply_generative
   | Cannot_scrape_alias of Path.t
@@ -246,13 +251,12 @@ let check_usage_of_path_of_substituted_item paths env signature ~loc ~lid =
                 let env = !env in
                 try retype_applicative_functor_type ~loc env funct arg
                 with Includemod.Error explanation ->
-                  Location.raise_errorf ~loc
-                    "@[<v>@[This `with' constraint on %a makes the applicative \
-                     functor type %s ill-typed in the constrained \
-                     signature:@]@ %a@]"
-                    Printtyp.longident lid.txt
-                    (Path.name referenced_path)
-                    Includemod.report_error explanation));
+                  raise
+                    (Error
+                       ( loc,
+                         env,
+                         With_makes_applicative_functor_ill_typed
+                           (lid.txt, referenced_path, explanation) ))));
     }
   in
   iterator.Btype.it_signature iterator signature;
@@ -435,11 +439,8 @@ let merge_constraint initial_env loc sg constr =
             in
             let params = tdecl.typ_type.type_params in
             if params_are_constrained params then
-              Location.raise_errorf ~loc
-                "@[<v>Destructive substitutions are not supported for \
-                 constrained types (other than when replacing a type \
-                 constructor with a type constructor with the same \
-                 arguments).@]";
+              raise
+                (Error (loc, initial_env, With_cannot_remove_constrained_type));
             fun s path -> Subst.add_type_function path ~params ~body s
         in
         let sub = List.fold_left how_to_extend_subst Subst.identity !real_ids in
@@ -1329,11 +1330,10 @@ and type_module_aux ~alias sttn funct_body anchor env smod =
                 (Env.add_module ~arg:true param arg.mod_type env)
                 param mty_res
             with Not_found ->
-              Location.raise_errorf ~loc:smod.pmod_loc
-                "@[This functor has type@ %a@ The parameter cannot be \
-                 eliminated in the result type.@  Bind the argument to a \
-                 module identifier.@]"
-                Printtyp.modtype mty_functor)
+              raise
+                (Error
+                   (smod.pmod_loc, env, Cannot_eliminate_dependency mty_functor))
+          )
       in
       rm
         {
@@ -1714,10 +1714,7 @@ let type_package env m p nl =
       (fun n ty ->
         try Ctype.unify env ty (Ctype.newvar ())
         with Ctype.Unify _ ->
-          Location.raise_errorf ~loc:m.pmod_loc
-            "@[The type %a in this module cannot be exported.@ Its type \
-             contains local dependencies:@ %a@]"
-            Printtyp.longident n Printtyp.type_expr ty)
+          raise (Error (m.pmod_loc, env, Scoping_pack (n, ty))))
       nl tl';
     (wrap_constraint env modl mty Tmodtype_implicit, tl')
 
@@ -1833,6 +1830,16 @@ let report_error ppf = function
       "@[<v>@[In this `with' constraint, the new definition of %a@ does not \
        match its original definition@ in the constrained signature:@]@ %a@]"
       longident lid Includemod.report_error explanation
+  | With_makes_applicative_functor_ill_typed (lid, path, explanation) ->
+    fprintf ppf
+      "@[<v>@[This `with' constraint on %a makes the applicative functor @ \
+       type %s ill-typed in the constrained signature:@]@ %a@]"
+      longident lid (Path.name path) Includemod.report_error explanation
+  | With_cannot_remove_constrained_type ->
+    fprintf ppf
+      "@[<v>Destructive substitutions are not supported for constrained @ \
+       types (other than when replacing a type constructor with @ a type \
+       constructor with the same arguments).@]"
   | With_changes_module_alias (lid, id, path) ->
     fprintf ppf
       "@[<v>@[This `with' constraint on %a changes %s, which is aliased @ in \
@@ -1866,6 +1873,11 @@ let report_error ppf = function
   | Interface_not_compiled intf_name ->
     fprintf ppf "@[Could not find the .cmi file for interface@ %a.@]"
       Location.print_filename intf_name
+  | Cannot_eliminate_dependency mty ->
+    fprintf ppf
+      "@[This functor has type@ %a@ The parameter cannot be eliminated in the \
+       result type.@  Bind the argument to a module identifier.@]"
+      modtype mty
   | Not_allowed_in_functor_body ->
     fprintf ppf "@[This expression creates fresh types.@ %s@]"
       "It is not allowed inside applicative functors."
@@ -1875,6 +1887,9 @@ let report_error ppf = function
   | Incomplete_packed_module ty ->
     fprintf ppf "The type of this packed module contains variables:@ %a"
       type_expr ty
+  | Scoping_pack (lid, ty) ->
+    fprintf ppf "The type %a in this module cannot be exported.@ " longident lid;
+    fprintf ppf "Its type contains local dependencies:@ %a" type_expr ty
   | Recursive_module_require_explicit_type ->
     fprintf ppf "Recursive modules require an explicit module type."
   | Apply_generative ->

compiler/ml/typemod.mli

@@ -64,18 +64,23 @@ val save_signature :
 type error =
   | Cannot_apply of module_type
   | Not_included of Includemod.error list
+  | Cannot_eliminate_dependency of module_type
   | Signature_expected
   | Structure_expected of module_type
   | With_no_component of Longident.t
   | With_mismatch of Longident.t * Includemod.error list
+  | With_makes_applicative_functor_ill_typed of
+      Longident.t * Path.t * Includemod.error list
   | With_changes_module_alias of Longident.t * Ident.t * Path.t
+  | With_cannot_remove_constrained_type
   | Repeated_name of string * string * Warnings.loc
   | Non_generalizable of type_expr
   | Non_generalizable_module of module_type
   | Interface_not_compiled of string
   | Not_allowed_in_functor_body
   | Not_a_packed_module of type_expr
   | Incomplete_packed_module of type_expr
+  | Scoping_pack of Longident.t * type_expr
   | Recursive_module_require_explicit_type
   | Apply_generative
   | Cannot_scrape_alias of Path.t