Remove dead error variants and warning constructors#1
Conversation
Drop Ctype.Tags and Ctype.Recursive_abbrev because both exceptions were declared and caught but had no raise site anywhere under compiler/. This also removes the unreachable Typetexp.Variant_tags and Typecore.Recursive_local_constraint wrappers that only existed to report those exceptions. Validation: rg found no remaining references to the removed constructors, and make completed successfully.
Drop Conflict_bs_bs_this_bs_meth because the constructor was only declared and printed; rg found no Bs_syntaxerr.err call or other construction site for it under compiler/. Validation: rg found no remaining references. The full rebuild is delayed by an unrelated coverage Dune process that currently holds _build/.lock.
Drop Label_mismatch, Abstract_wrong_label, and Incoherent_label_order. Label_mismatch's pattern path was catching Ctype.Unify around unify_pat_types even though that helper already translates Unify to Pattern_type_clash. The expression path now uses the existing Expr_type_clash machinery if the defensive invariant ever fails. Abstract_wrong_label and Incoherent_label_order were defensive branches behind modern function and labeled-argument normalization; any fallback now uses the existing Too_many_arguments or Apply_wrong_label diagnostics. Validation: rg found no remaining references. A full rebuild will run once the unrelated coverage Dune process releases _build/.lock.
Drop Invalid_interval and Invalid_for_of_pattern from Typecore.error. The ReScript parser has no source construction site for Ppat_interval, and normalize_for_of_pattern reports destructuring for...of syntax before replacing it with Ppat_any. The old typer guards now use direct Location errors only as malformed-AST fallbacks. Validation: rg found no remaining references to the removed named variants.
Drop Unbound_type_constructor_2, Ill_typed_functor_application, and Apply_structure_as_functor from Typetexp.error. The first needs an inherited constructor whose body expands to a bare Tvar, which ReScript source cannot construct; the latter two only sit under Longident.Lapply, which the parser does not emit. The old guard sites now use direct Location errors as malformed-AST fallbacks. Validation: rg found no remaining references to the named variants.
Drop Type_clash, Parameters_differ, Null_arity_external, Rebind_wrong_type, Bad_fixed_type, Varying_anonymous, and Val_in_structure from Typedecl.error. These guarded paths are rejected or normalized before the named diagnostics can fire in modern ReScript: recursive aliases are caught earlier, external declarations are encoded or rejected earlier, extension rebind result types unify by construction, fixed rows preserve their open row shape, anonymous type parameters are not parsed, and structure val declarations with empty prims do not reach the typer from source. The old guard sites now use direct Location errors as malformed-AST fallbacks. Validation: rg found no remaining references to the named variants.
Drop Cannot_eliminate_dependency, With_makes_applicative_functor_ill_typed, With_cannot_remove_constrained_type, and Scoping_pack from Typemod.error. The dead reports were guarded by paths the current module pipeline does not reach: nondep_supertype falls back instead of raising Not_found, applicative Papply constraints are not parsed from source, destructive type substitutions only get fresh type variables, and the package scoping mismatch is a defensive fallback. The old guard sites now use direct Location errors as malformed-AST fallbacks. Validation: rg found no remaining references to the named variants.
Drop Unhandled_poly_type and Misplaced_label_syntax from Bs_syntaxerr.error. Inline polytypes in external arrow chains and labeled arguments to the legacy ->/#=/## rewrites are not emitted by the ReScript parser. The old guard sites now raise direct Location errors as malformed-AST fallbacks. Validation: rg found no remaining references to the named variants.
Drop Illegal_value_name from Env.error. The ReScript parser does not emit value identifiers named -> or containing #, so the named environment error is unreachable from source. The check_value_name guard now raises a direct Location error for malformed PPX-forged identifiers. Validation: rg found no remaining references to the named variant.
Drop has_label after removing the unreachable Incoherent_label_order path. The full make checkpoint caught it as an unused helper under the repository warning policy. Validation: make -j2 completed successfully after this cleanup.
Drop warning constructors that have no live raise path: old OCaml lexer/class/docstring warnings, unused BuckleScript warnings, Statement_type, Unerasable_optional_argument, and Bs_uninterpreted_delimiters. Statement_type was guarded by a check_application_result argument that is only ever false. Unerasable_optional_argument was disabled around its only check. Bs_uninterpreted_delimiters only covered the old unprocessed js delimiter warning path, which modern string handling no longer uses. The warning numbers remain holes; no existing warning numbers were reused. Validation: rg found no remaining references to the removed warning constructors, and make -j2 completed successfully.
Run the repository formatter after removing the unreachable diagnostics. This keeps the edited compiler modules passing make checkformat without changing behavior.
Record the diagnostics removed on jono/remove-dead-errors and remove their table rows. Reclassify the audited items that are not completely dead, including stale-CMI module-type paths, the live Syntaxerr printer gap, and UTF-8 helper errors retained for defensive/test entry points.
Full build testing showed extension_rebind_mismatch.res exercises Rebind_wrong_type, not Rebind_mismatch. Restore the typed unification diagnostic and update the audit to mark Rebind_wrong_type live while leaving Rebind_mismatch unconfirmed.
Reproductions during PR review showed that two of the three typecore label variants removed by 52bed6c7f are still reachable from regular ReScript source: - Label_mismatch fires when a record literal without an expected type mixes fields from two different record types — disambiguation picks one type per label and the cross-type unify inside type_label_exp fails. Fixture: label_mismatch_record_literal.res. - Abstract_wrong_label fires when a multi-arg function literal has an inner argument label that doesn't match the expected arrow's label. Fixture: abstract_wrong_label.res. Restore both variants, their raise sites, and reporter cases. The third removed variant (Incoherent_label_order) stays removed — every attempt to reach it routed through Expr_type_clash via the modern arity-aware unify path. Update tests/ERROR_VARIANTS.md to mark both rows ✓ with the new fixtures and drop them from the removal list. The pattern-path catch in type_pat_aux stays removed (was genuinely dead because unify_pat_types already translates Ctype.Unify to Pattern_type_clash).
| | Abstract_wrong_label of arg_label * type_expr | ||
| | Scoping_let_module of string * type_expr | ||
| | Not_a_variant_type of Longident.t | ||
| | Incoherent_label_order |
There was a problem hiding this comment.
Verification: sibling error fires first (S). Original raise site: typecore.ml:3894 in type_unknown_args's argument-label dispatch — fires when arity_ok && has_label l1 ty_fun (the label exists in the function type but at a different position). In modern uncurried ReScript, type_function (typecore.ml:3480–3488) eagerly unifies the function literal's arity-aware Tarrow with ty_expected_ via unify_exp_types, which raises Expr_type_clash before the wrong-position branch is reachable. Reproduction attempts — f(~b=1, ~a=2), cross-pipe label-flipping (apply_both(f, f) with mirrored call sites), partial application against fresh Tvars — all routed through Expr_type_clash. The dead-removal collapses the else raise Incoherent_label_order arm into Apply_wrong_label, which is the same diagnostic the sibling not (has_label l1 ty_fun) arm already raises.
| | Modules_not_allowed | ||
| | Cannot_infer_signature | ||
| | Not_a_packed_module of type_expr | ||
| | Recursive_local_constraint of (type_expr * type_expr) list |
There was a problem hiding this comment.
Verification: transitively dead (G). Only raised in unify_pat_types_gadt's Unification_recursive_abbrev → catch arm (typecore.ml:368, removed in the same commit). Unification_recursive_abbrev is itself only raised in Ctype.unify's Recursive_abbrev → catch arm (ctype.ml:2734). Ctype.Recursive_abbrev has zero raise sites under compiler/ — rg 'raise.*Recursive_abbrev' (excluding the unrelated live typedecl.Recursive_abbrev variant) returns nothing. Chain is dead end-to-end.
| | Recursive_local_constraint of (type_expr * type_expr) list | ||
| | Unexpected_existential | ||
| | Unqualified_gadt_pattern of Path.t * string | ||
| | Invalid_interval |
There was a problem hiding this comment.
Verification: parser blocks AST (P). Only raised when type_pat_aux sees Ppat_interval _. ReScript's parser (compiler/syntax/src/res_core.ml) has no construction site for Ppat_interval — interval patterns ('a' .. 'z') are OCaml syntax not part of the ReScript grammar. Fixture 'a' .. 'z' in a switch compiles silently (the parser sees a range expression, not an interval pattern). Replacement Location.raise_errorf keeps the original message for any PPX-forged AST.
| | Unqualified_gadt_pattern of Path.t * string | ||
| | Invalid_interval | ||
| | Invalid_for_loop_index | ||
| | Invalid_for_of_pattern |
There was a problem hiding this comment.
Verification: parser normalizes (P). Only raised in type_pat_aux for for ... in ... bindings whose pattern isn't a variable or _. ReScript's parser runs normalize_for_of_pattern (compiler/syntax/src/res_core.ml:3841) which catches every non-var, non-_ shape, emits a syntax error, and replaces the pattern with Ppat_any before the typer runs. Fixture for (a, b) in xs { ... } rejected at parse time with Did you forget a \"(\" here?. The named typer variant is dead behind the parser check; Location.raise_errorf preserves the message for any PPX-forged shape.
| type error = | ||
| | Unbound_type_variable of string | ||
| | Unbound_type_constructor of Longident.t | ||
| | Unbound_type_constructor_2 of Path.t |
There was a problem hiding this comment.
Verification: parser blocks AST (P). Raised in transl_type_aux (typetexp.ml:472/617) when an inherited type's body expands to a bare Tvar — needs type 'a t = 'a plus a downstream inherit. ReScript syntax requires type parameters in angle brackets (type t<'a> = 'a); the parser rejects leading-'a form. Fixture type 'a t = 'a rejected as syntax error. The shape needed (bare-Tvar Tconstr body) is parser-unreachable; Location.raise_errorf covers any PPX-forged AST.
| | Open_shadow_identifier of string * string (* 44 *) | ||
| | Open_shadow_label_constructor of string * string (* 45 *) | ||
| | Attribute_payload of string * string (* 47 *) | ||
| | Eliminated_optional_arguments of string list (* 48 *) |
There was a problem hiding this comment.
Verification: no raise site (G). Declared but never raised. rg 'prerr_warning.*Eliminated_optional_arguments' returns zero matches under compiler/.
| | Attribute_payload of string * string (* 47 *) | ||
| | Eliminated_optional_arguments of string list (* 48 *) | ||
| | No_cmi_file of string * string option (* 49 *) | ||
| | Bad_docstring of bool (* 50 *) |
There was a problem hiding this comment.
Verification: no raise site (G). Declared but never raised. rg returns zero matches outside warnings.ml. Also default-disabled in bsc_warnings.ml.
| | Bs_polymorphic_comparison (* 102 *) | ||
| | Bs_ffi_warning of string (* 103 *) | ||
| | Bs_derive_warning of string (* 104 *) | ||
| | Bs_fragile_external of string (* 105 *) |
There was a problem hiding this comment.
Verification: no raise site (G). Declared but never raised. rg returns zero matches outside warnings.ml.
| | Bs_ffi_warning of string (* 103 *) | ||
| | Bs_derive_warning of string (* 104 *) | ||
| | Bs_fragile_external of string (* 105 *) | ||
| | Bs_unimplemented_primitive of string (* 106 *) |
There was a problem hiding this comment.
Verification: no raise site (G). Declared but never raised. rg returns zero matches outside warnings.ml.
| | Bs_fragile_external of string (* 105 *) | ||
| | Bs_unimplemented_primitive of string (* 106 *) | ||
| | Bs_integer_literal_overflow (* 107 *) | ||
| | Bs_uninterpreted_delimiters of string (* 108 *) |
There was a problem hiding this comment.
Verification: PPX rewrites the trigger AST (S). Raised at bs_warnings.ml:29 for Pconst_string (_, Some "js"). The parser does emit Some "js" for all strings in ParseForTypeChecker mode (res_core.ml:1066), but the builtin PPX runs first: Bs_builtin_ppx.mapper calls Ast_utf8_string_interp.transform_exp which replaces Some "js" with Some "j" (escaped) or Some "*j" (backtick) before Bs_ast_invariant.check_constant ever sees the constant. With -bs-no-builtin-ppx, the warning iterator is skipped entirely (ppx_entry.ml:58–62). Either way the warning never fires for a real ReScript source program.
JonoPrest
left a comment
There was a problem hiding this comment.
Inline validation comments — for each removed item, the verification method and concrete evidence. Categories: G grepped dead (no raise site under compiler/), P parser blocks the required AST, S sibling error fires before this one, R reachable but the post-removal Location.raise_errorf emits the same user-facing message. Two variants are marked R — they're still safe to remove, but the original audit's "unreachable" framing was too strong.
…d_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.
JonoPrest
left a comment
There was a problem hiding this comment.
Update (commit 4c8c646)
Reworked per review feedback. Two principled changes:
1. Defensive unreachable raise sites now use assert false (* reason *) instead of Location.raise_errorf — matching the established typer-module convention (57 existing assert false uses in typecore/typedecl/typemod/typetexp; zero Location.raise_errorf calls in those four modules in master). The earlier approach moved code rather than removing it and introduced a foreign pattern.
2. Five variants restored to named-variant form:
typemod.With_cannot_remove_constrained_type— confirmed reachable (module type T = S with type t<'a> := 'awith constrainedS). New fixture:with_cannot_remove_constrained_type.res.bs_syntaxerr.Misplaced_label_syntax— confirmed reachable (\"->"(x, ~b=...)). New fixture:misplaced_label_syntax.res.typemod.Cannot_eliminate_dependency,typemod.With_makes_applicative_functor_ill_typed,typemod.Scoping_pack— couldn't reproduce, retained conservatively as named variants with?status in the catalog rather than removed.
Inline comments above: the 14 still-anchored variant-removal comments correctly identify each variant as unreachable. The replacement-mechanism notes in them say Location.raise_errorf — that has now been switched to assert false (* reason *) (same justification, different syntax). The 5 comments for now-restored variants have been deleted.
Net diff: 22 named error variants removed (down from the original 24) + 3 supporting Ctype exceptions + 1 helper (has_label) + 14 warning constructors. 545 deletions / 328 additions across 29 files.
Remove dead error variants and warning constructors
Cleanup follow-up to rescript-lang#8446. Splits dead-variant removals into two
principled categories:
Truly dead — variants with no raise site, transitively dead
chains, or always-false guards. Declaration, raise, reporter, and
any supporting helpers are deleted entirely.
Defensive unreachable — variants whose raise site exists but is
provably unreachable from the ReScript parser (parser blocks the
required AST shape, or a sibling check fires first). The raise site
becomes
assert false (* reason *)with a comment citing the specificblocker; the named variant and its reporter are deleted. This matches
the existing typer-module convention (57
assert falseuses acrosstypecore/typedecl/typemod/typetexp; zero
Location.raise_errorfcalls in those four modules in master).
Variants that turned out to be reachable, or that I couldn't conclusively
prove unreachable, are kept as named variants — no code change to them.
Truly dead — declaration + raise + reporter all removed
ctype.ml(3 exceptions, never raised undercompiler/):Tags of label * labelRecursive_abbrevUnification_recursive_abbrev of type_pairsRelay variants (only raised inside catch arms for the dead exceptions above):
typecore.Recursive_local_constrainttypetexp.Variant_tagsbs_syntaxerr.ml:Conflict_bs_bs_this_bs_meth— no construction site anywherewarnings.ml(14 constructors):compiler/:Comment_start(1),Comment_not_end(2),Method_override(7),Instance_variable_override(13),Illegal_backslash(14),Implicit_public_methods(15),Eol_in_string(29),Eliminated_optional_arguments(48),Bad_docstring(50),Bs_fragile_external(105),Bs_unimplemented_primitive(106)Statement_type(10) — caller hard-codesstatement = false;Unerasable_optional_argument(16) — disabled around its only check;Bs_uninterpreted_delimiters(108) — trigger AST rewritten by builtin PPX before the warning iterator runsWarning numbers are kept as holes so existing user configs like
-w "+a-1-2"keep working (parse_optionssilently ignores numbers no live constructor maps to).Defensive unreachable — raise becomes
assert false (* reason *)Variant + reporter removed; raise site rewritten to match the established convention.
typecore.ml:Invalid_interval→assert false (* parser has no construction site for Ppat_interval *)Invalid_for_of_pattern→assert false (* parser's normalize_for_of_pattern replaces non-var patterns with Ppat_any *)Incoherent_label_order→ collapsed intoApply_wrong_labelwith a comment (restoringhas_labelfor an assert-false marker would mean re-introducing 3 helper functions just to flag one branch)typetexp.ml:Unbound_type_constructor_2→assert false (* parser rejects bare-Tvar Tconstr body shapes liketype 'a t = 'a*)Ill_typed_functor_application,Apply_structure_as_functor→assert false (* both branches live behind Longident.Lapply, which the parser never constructs *)typedecl.ml:Type_clash,Parameters_differ→assert false (* every recursive abbreviation hits Cycle_in_def first *)Null_arity_external→assert false (* Primitive.parse_declaration always assigns the magic prim_native_name encoding *)Bad_fixed_type→assert false (* is_fixed_type and expand_head agree on every parser-produced shape *)Varying_anonymous→assert false (* parser rejects \_` in type-parameter positions *)`Val_in_structure→assert false (* pval_prim = [] outside a signature only arises from parser external-recovery, post syntax error *)bs_syntaxerr.ml:Unhandled_poly_type→assert false (* parser misreads inline 'a. as deprecated uncurried (. …) syntax *)env.ml:Illegal_value_name→assert falseon both guard branches (parser doesn't emit"->"or#-containing identifiers)Variants restored to named-variant form
Three variants had already been restored during the original audit pass
(
typedecl.Rebind_wrong_typeviaextension_rebind_mismatch.res,typecore.Label_mismatchvialabel_mismatch_record_literal.res, andtypecore.Abstract_wrong_labelviaabstract_wrong_label.res).Two additional variants restored in the same review pass — empirical
fixtures confirmed they fire on real ReScript source:
typemod.With_cannot_remove_constrained_type—module type T = S with type t<'a> := 'aagainstScontainingtype t<'a> constraint 'a = int. Fixture:with_cannot_remove_constrained_type.res.bs_syntaxerr.Misplaced_label_syntax— labelled args passed via operator-identifier syntax:\"->"(x, ~b=...). Fixture:misplaced_label_syntax.res.Three more
typemodvariants restored conservatively — I couldn'tconstruct a reproducing fixture, but couldn't conclusively prove
unreachability either, so they stay as named variants pending stronger
evidence:
typemod.Cannot_eliminate_dependency(non-generative functor application paths)typemod.With_makes_applicative_functor_ill_typed(destructive substitution on applicative-functor signatures)typemod.Scoping_pack(first-class module packing withwith typeconstraints)What's left in the catalog
After this PR,
ERROR_VARIANTS.mdcontains:<X>harness) — reachable but requires a CLI / binary / PPX / build harness, out of scope forsuper_errors{,_multi}.typemodvariants where reachability couldn't be confirmed or denied with a fixture.No
⚠rows.Validation
rgconfirmed no remaining references to truly-dead removed variants.super_errors+ 54super_errors_multifixtures pass; 925 stdlib mocha tests pass.make checkformatclean.extension_rebind_mismatch.res,label_mismatch_record_literal.res,abstract_wrong_label.res,with_cannot_remove_constrained_type.res,misplaced_label_syntax.res.