Remove dead error variants and warning constructors#1
Conversation
| | 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.
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.
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5 to 6. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v5...v6) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Upload PR dev-playground bundles * Add safe dev playground PR bundle previews * Post a comment with the playground URL
* Use Yojson and lsp types for analysis JSON output Replace the vendored JSON helpers with Yojson and the OCaml lsp types across analysis, reanalyze, and tools JSON handling. This updates LSP response construction for completions, hovers, diagnostics, code actions, symbols, signature help, and related CLI commands, refreshes analysis snapshots, and adjusts extract-codeblocks decoding for the new serialized output. * Delete Protocol.ml * Delete comments * Fix LSP JSON serialization Serialize analysis command results with Yojson output helpers instead of string extraction, fixing document symbols so they return JSON arrays instead of crashing. Also use typed LSP enum constructors for completion and inlay hint kinds, clean up JSON config parsing, and update analysis test snapshots. * Use Lsp.Uri for URI handling Replace the custom URI record with Lsp.Uri.t and delegate path/URI conversion to the LSP library. This fixes interface-aware analysis results so definitions, references, and renames consistently target .resi files when an interface is present, and updates the analysis snapshots accordingly. * remove `null` * Update GenericJsxCompletion.res.txt * Update tests * Update tests/analysis_tests/tests-sourcedirs-dependency/src/expected/Main.res.txt * Update tests/tools_tests * Remove null fields, `data` and `deprecated` * Update tests/analysis_tests/tests-generic-jsx-transform/src/expected/GenericJsxCompletion.res.txt * Update analysis_tests * Update analysis_tests * Remove unused pathToUri * docstrings: remove decodeFromJson implementation * Use basename instead of full URI in Xform test output * Fix rename `newUri` * Use only `Uri` instead of `Lsp.Uri` * update CHANGELOG.md * Omit null fields from doc extraction JSON Only include optional doc extraction fields like deprecated, detail, payload, and moduletypeid when they have values. Update tool test snapshots to match the leaner JSON output. * Update dune-project and dune files with yojson * Add `version` for textDocument in rename * Apply codex suggestions * rename: use `Uri.toPath`
* Update lsp and yojson dependencies Relax LSP-related dependencies to allow 1.23.0+ and update yojson pins to 3.0.0 across dune-project and opam package files. Also trims trailing whitespace in the CI workflow. * Update dune-project
…#8454) * analysis: fix namespace parsing after Yojson migration * Changelog * Move yojson helpers to reanalyze and reuse them there * Some more tests
9ed7f8a to
4c8c646
Compare
6c59d51 to
3b05612
Compare
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).
…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.
Re-audited the defensive "unreachable" removals in this branch by driving each assert false to a real crash with a minimal .res (against a master oracle build). 8 were actually reachable or unproven and are restored: Reachable (each was a confirmed compiler crash; regression fixtures added): - typetexp.Unbound_type_constructor_2 (identity alias in inherit position) - typecore.Invalid_interval (non-char interval pattern, e.g. `1 .. 5`) - typedecl.Null_arity_external (`?`-prefixed external name, arity 0) - typedecl.Varying_anonymous (variance annotation on constrained GADT param) - typedecl.Parameters_differ (non-uniform recursion via object manifest) - typedecl.Bad_fixed_type (fully-bounded closed private polymorphic variant) - env.Illegal_value_name (escaped identifier `\"->"`) Appear dead but unproven (no structural proof, no repro found) — retained as named variants with explanatory comments at definition and raise site rather than assert false, pending further investigation: - typecore.Incoherent_label_order - typedecl.Type_clash Also restores the helpers needed by these paths (list_labels/list_labels_aux/ has_label) and updates tests/ERROR_VARIANTS.md with the corrected reachability and a re-audit correction note. The genuinely-dead removals (14 warnings, the Longident.Lapply functor paths, Tags/Variant_tags, ctype Recursive_abbrev/Unification_recursive_abbrev, Unhandled_poly_type, Conflict_bs_bs_this_bs_meth, the unescaped-"js"-delimiter path, Invalid_for_of_pattern, Val_in_structure) are kept removed.
Continued the reachability investigation of the two variants that were restored-but-unproven. - typecore.Incoherent_label_order: now PROVEN reachable. A not-yet- generalized function value applied more than once with labelled args in conflicting orders (`let f = g => (g(~a=1, ~b=2), g(~b=3, ~a=4))`) reaches the leftover/tvar path in type_unknown_args (type_args reconciles labels by name, so only this path is left). Added fixture labeled_args_incoherent_order.res and replaced the "appears dead" comments with the real trigger. - typedecl.Type_clash: still no reproduction after ~37 recursive / mutual / constraint / row / object / alias shapes. update_type unifies t<fresh params> against t's own manifest — a type against an alpha-renamed copy of itself — which cannot head-clash; every real inconsistency is caught by Cycle_in_def / Recursive_abbrev / Parameters_differ / Constraint_failed / Type_arity_mismatch first. Kept as a named variant (not assert false) with the strengthened structural rationale, pending an airtight proof. 7 of the 8 restored variants now have reproducing fixtures.
The comment claimed the parser "misreads inline `'a.` as the deprecated
uncurried `(. …)` form and rejects it" — that is wrong. Validated the
unreachability from scratch (structural + ~13 empirical externals):
- list_of_arrow only recurses an arrow's return and is only called on an
external's type annotation. The external type, and every arrow arg/return,
is parsed by parse_typ_expr, which never routes to parse_poly_type_expr.
- An inline `'a. …` in an external's arg/return position is a plain syntax
error ("Did you forget a `=` here?"), not a misread uncurried dot.
- Ptyp_poly is produced only for record/object field types and signature
`val` descriptions; a field-nested poly is a non-arrow leaf that
list_of_arrow stops at, never the recursed return.
Conclusion (unreachable) unchanged; only the rationale is corrected. No
behavioral change. ERROR_VARIANTS.md already stated the correct reason.
The branch's earlier label-diagnostic churn left Abstract_wrong_label moved from after Too_many_arguments (master) to after Apply_wrong_label, in the typecore error type (.ml + .mli) and the report_error printer. The move is purely cosmetic (sum-type constructor order / match-arm order) and only created needless diff noise. Moved all three back to match master.
Reviewed the whole diff vs master and reverted reorderings that were not
real changes, so the review shows only the actual edits in place:
- typemod.ml: reverted entirely to master. Its only diff was two report_error
arms (Cannot_eliminate_dependency, With_changes_module_alias) reordered —
pure cosmetic churn, both still raised, no content change.
- typecore.ml: moved list_labels/list_labels_aux back to their original spot
(after type_approx, before check_univars) with the original comment. I had
re-added them before lower_args when restoring, creating a spurious
delete+add pair.
- bs_syntaxerr.ml/.mli: restored to master and re-applied only the intended
change in place — Misplaced_label_syntax had been relocated to the end of
the error type and pp_error; now it stays in its original position and the
only diff is the message typo fix ("support" -> "supported"), which the
fixture already expects.
env.ml/.mli were already byte-identical to master. No behavioral change;
build + format clean, super_errors suite green.
Keep the one-liner at the definition; the full rationale lives at the update_type raise site it points to.
It's a live variant with a fixture (labeled_args_incoherent_order.res), so the behavior is captured by the test, not a comment. Comments are reserved for removals. The raise site is now identical to master.
The 9 re-audited variants (Type_clash, Parameters_differ, Null_arity_external, Bad_fixed_type, Varying_anonymous, Invalid_interval, Incoherent_label_order, Illegal_value_name, Unbound_type_constructor_2) had their per-module table rows deleted and the info relocated to the prose list, causing line drift. Each row is now back in its master position with an in-place status update (⚠ -> ✓ + fixture; Type_clash stays ⚠ "appears dead, retained"), so the diff reads as a row-level edit instead of a move. Verified all four module tables (typecore/typedecl/env/typetexp) now match master row order; remaining gaps are only the genuinely-removed variants.
…handled_poly_type) A prior revert that fixed the Misplaced_label_syntax line move also restored these two dead variants. Neither is reachable: Conflict_bs_bs_this_bs_meth is never raised, and Unhandled_poly_type's only use was replaced with assert false in ast_core_type. Remove both again; keep Misplaced_label_syntax in place.
f98f796 to
890bcc0
Compare
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.