super_errors: error variant catalog + final fixture sweep#8446
Open
JonoPrest wants to merge 17 commits into
Open
super_errors: error variant catalog + final fixture sweep#8446JonoPrest wants to merge 17 commits into
JonoPrest wants to merge 17 commits into
Conversation
A single reference doc enumerating every error variant the compiler can emit, with each entry mapped to either a fixture, a reason the variant is unreachable, or an open TODO.
Cover Unknown_literal (0z), Empty_record_literal (let bad = {}),
No_value_clauses (switch with only exception clauses),
Type_params_not_supported (variant spread pattern with type params),
and Not_a_variant_type (variant spread pattern of non-variant).
Cover Rebind_mismatch (rebinding constructor into different extensible type) and Rebind_private (rebinding a private extension constructor as public).
Mark the typecore/typedecl/bs_syntaxerr variants now covered by the new super_errors fixtures. Move warnings 10, 16 and 108 to the "Confirmed dead" section: Statement_type is unreachable because the only caller passes statement=false; Unerasable_optional_argument is explicitly disabled inside type_function before its check; and Bs_uninterpreted_delimiters needs a string literal with the legacy "js" delimiter, which the modern scanner never produces. Warning 5 stays "live but no fixture" with a note that the raise sites look effectively unreachable from plain ReScript source.
Cover Kind (record vs variant), Constraint (impl adds constraint that the abstract interface doesn't), Manifest (different manifest types), Variance (interface annotates +'a, impl differs), Field_arity (constructor arg counts differ), Field_names (record field names differ at position), Record_representation (boxed vs @unboxed), and Immediate (@Immediate vs non-immediate manifest).
Type_mismatch fires when applying a type constructor with arguments that violate a constraint clause (`type t<'a> = 'a constraint 'a = int` called with `t<string>`). Not_a_variant fires when a polyvariant inherits from a non-polyvariant (`[#X | int]`). Also catalog the new fixtures, mark Conflict_bs_bs_this_bs_meth as dead (declared but never raised), and downgrade Rebind_wrong_type to ? since I couldn't construct a triggering shape.
…expected Covers every ☐ variant in ast_untagged_variants.ml: OnlyOneUnknown, AtMostOneObject/Instance/Function/Number/Bigint/Boolean, DuplicateLiteral, ConstructorMoreThanOneArg, InvalidVariantAsAnnotation, Duplicated_bs_as, InvalidVariantTagAnnotation, TagFieldNameConflict. Also adds typemod_signature_expected.res — a `with type` on an inner functor-typed module triggers Signature_expected when the parent sig-extraction hits a non-Mty_signature module type. Updates the catalog and marks ast_utf8_string_interp.* as dead (production pipeline never calls check_and_transform; only OUnit tests do).
`@obj external make: (~x: _) => _ = ""` triggers the error at optional-label position when no @as is supplied. Also marks typedecl.Val_in_structure as dead: the only way to reach it is via manually-constructed AST, since the parser's external-recovery path emits a syntax error before the typechecker ever sees the value declaration.
Adding `type t += A(int)` to a parametric extensible type `type t<'a> = ..` triggers Extension_mismatch with an Arity sub-error. Single-file fixture suffices — the variant fires inside `transl_type_extension` regardless of cross-module context.
Mark each remaining ☐ row with the specific harness it would need
("needs build harness", "needs binary harness", "needs CLI
harness", "needs PPX harness") so the catalog distinguishes
"reachable but blocked by the source-only harnesses" from
"reachable and just unwritten". Also extend the Confirmed-dead
section with the new findings: Conflict_bs_bs_this_bs_meth (never
raised), ast_utf8_string_interp.* (only called from OUnit
transform_test), Val_in_structure (parser recovery emits syntax
error first), Illegal_value_name (parser never produces "->" or
#-containing idents), and warnings 10/16/108 (each blocked by a
specific code path).
Every remaining ☐ row now carries a parenthetical tag ("needs build
harness" / "needs binary harness" / "needs CLI harness" / "needs
PPX harness" / "needs harness flag") so a reader can tell at a
glance whether a variant is unwritten coverage vs. blocked by what
the harness invokes bsc with. Promotes Label_mismatch, Type_clash,
Cannot_eliminate_dependency, Non_generalizable_module, and
Illegal_reference_to_recursive_module from ☐ to ? — they have live
raise sites but every attempted reproduction hit a different error
first. Marks With_makes_applicative_functor_ill_typed and
includemod.Unbound_modtype_path as dead with the source-level
reason.
Each fixture exercises a distinct trace-shape that the existing
catalog notes as untested: StringConcat ("hi" ++ 42 routes through
the dedicated string-concat context message), IfReturn
(`let x: string = if … { 1 } else { 2 }` swaps the generic
"expected to have type" for the if-statement-specific framing),
and MaybeUnwrapOption (`let x: int = Some(5)` — option-vs-non-option
recognition path).
Six new fixtures cover variants previously marked ?: - polymorphic_label.res — pattern that instantiates a 'a. record field - cannot_quantify.res — 'a. (int as 'a) rebinds the universal - polyvariant_present_has_conjunction.res — [< #A(int) & (string) > #A] - polyvariant_present_has_no_type.res — [< #B > #A] - non_generalizable_module.res — inner ref(None) escapes module type - illegal_recursive_module_reference.res — module rec A: B.S = … Eleven other ? rows are formally promoted to ⚠ with the specific source-level reason the trigger site can't be reached from ReScript syntax: Label_mismatch, Abstract_wrong_label, Incoherent_label_order, Type_clash, Parameters_differ, Rebind_wrong_type, Bad_fixed_type, Varying_anonymous, Unbound_type_constructor_2, Cannot_eliminate_dependency, With_cannot_remove_constrained_type, Unhandled_poly_type. After this pass the catalog has no remaining ? rows — every variant is either covered, marked dead with a concrete reason, or tagged with the harness it would need to exercise.
The formatter normalised six fixture sources (whitespace around @this self-pattern parens, object-type spread spacing, if/then/else formatting on a single line). The error locations move with the source, so re-snapshot the matching expected files.
Coverage (`make coverage`) showed 53% line coverage across the
compiler; the lowest-coverage error-emitting modules are
ast_untagged_variants.ml (56%), typemod.ml (62%), includecore.ml (63%),
includemod.ml (68%), typedecl.ml (73%), error_message_utils.ml (75%).
A targeted audit of the report-side branches in error_message_utils.ml
flagged the following untested user-visible sub-cases:
- pattern_type_clash_polyvariant.res — `switch (x: int) { | #A => …}`
trips a Pattern_type_clash with the polyvariant-vs-concrete trace.
- pattern_type_clash_tuple_arity.res — pattern `(a, b, c)` against a
`(int, string)` value (tuple-arity mismatch path).
- labeled_fn_argument_type_clash.res — `f(~x="hi")` against
`(~x: int) =>` hits FunctionArgument with an explicit label name
(distinct from the existing optional-arg / record-field sub-cases).
- Iface_variant_extra_constructor (multi) — implementation declares
one more constructor than the interface; covers the `Field_missing`
branch where the impl is the side with the extra.
Catalog notes updated to enumerate the Expr_type_clash and
Pattern_type_clash trace-shape fixtures so future readers can
audit sub-case coverage without spelunking the source.
Contributor
Author
|
@codex review |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8446 +/- ##
==========================================
+ Coverage 60.10% 60.45% +0.35%
==========================================
Files 373 373
Lines 54231 54231
==========================================
+ Hits 32593 32787 +194
+ Misses 21638 21444 -194 🚀 New features to boost your workflow:
|
JonoPrest
commented
May 26, 2026
Comment on lines
+1
to
+16
| # Compiler error variant catalog | ||
|
|
||
| A per-module table of every named error and warning variant the compiler | ||
| declares, tagged with whether a test fixture currently exercises it. | ||
|
|
||
| The catalog has two practical uses: | ||
|
|
||
| 1. **Coverage expansion** — find rows tagged `☐` (reachable but no | ||
| fixture) and add a `super_errors` / `super_errors_multi` fixture for | ||
| each. The Notes column points at the trigger site and any required | ||
| AST shape. | ||
| 2. **Dead-code removal** — rows tagged `⚠` are variants whose trigger | ||
| site is unreachable in the current parser / compiler, with a named | ||
| blocker. They can be deleted in a follow-up PR. The "Confirmed dead" | ||
| summary at the bottom groups them by reason. | ||
|
|
Contributor
Author
There was a problem hiding this comment.
This was really an intermediary working file for the agents to catalogue and work through error variants.
I have one more PR on top of this that removes the identified dead code and we can remove this file as well unless you think its valuable.
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #8433. Closes out every remaining reachable error / warning
variant in
super_errors/super_errors_multiand turnstests/ERROR_VARIANTS.mdinto an exhaustive map.After this PR every row in
ERROR_VARIANTS.mdis one of: ✓ covered, ⚠ verified dead with a source-level
reason, or ☐ (needs
<X>harness) reachable only with a CLI /binary / PPX / build harness. No
?rows remain.New fixtures (50)
compiler/ml/typecore.mlcompiler/ml/typedecl.mlcompiler/ml/typetexp.mlcompiler/ml/typemod.mlcompiler/frontend/bs_syntaxerr.mlcompiler/ml/ast_untagged_variants.mlcompiler/ml/includecore.mlExpr_type_clash/Pattern_type_clash)Plus
tests/ERROR_VARIANTS.md(605 lines) — a per-module table keyed off
type error = …constructors, every row tagged ✓ / ⚠ / ☐ with the fixture, blocker, or
required harness.
AGENTS.mdis updated with a "keep the catalog insync" rule.
Newly classified as dead (⚠)
10 variants are promoted from
?to ⚠ with a specific source-levelblocker (defensive
try unifypaths subsumed by sibling errors, ASTshapes the parser doesn't construct, warnings suppressed before their
own check fires, etc.). Combined with prior PRs, the catalog now lists
27 verified-dead error variants and 14 dead warning constructors
— all candidates for a follow-up removal PR.
Coverage impact
Measured locally with
make coverage(bisect_ppx point coverage).Baseline is
upstream/master(0a29fc8a9).Per-file moves (top by Δ lines covered):
compiler/frontend/bs_syntaxerr.mlcompiler/ml/ast_untagged_variants.mlcompiler/ml/typetexp.mlcompiler/ml/includecore.mlcompiler/ml/typemod.mlcompiler/ml/typedecl.mlcompiler/ml/typecore.ml