Repository navigation
Pass the origin of a synthesized match as an argument, not an attribute - #9128
Conversation
Why:
The type checker rewrites a `let` destructuring a GADT constructor, and
an optional parameter's default, into a match, and marked that match
with the string attributes `#let_gadt_pattern` and
`#optional_arg_default` so that it gets function-style diagnostics
rather than a switch's (#9126). Both are made and read inside typecore
and nothing after typing reads them. `type_expect` now takes
`?match_context` and `type_let` takes `?optional_default`; only
`let.unwrap`, which comes from the parsed `let?`, stays an attribute.
The GADT rewrite also required the binding to have no attributes, so a
doc comment or `@warning` in front of it was an error; its attributes
now go on the case pattern, and `@warning` on a pattern now covers that
pattern's unused variables.
Typedtree modified (let destructuring a GADT constructor):
Example:
type rec any = Any('a, 'a => string): any
let get = value => {
let Any(x, f) = value
f(x)
}
Before (typed):
expression
attribute "#let_gadt_pattern"
[]
Texp_match
expression
Texp_ident "value/1004"
[
<case>
pattern
Tpat_construct "Any"
…
After (typed):
expression
Texp_match
expression
Texp_ident "value/1004"
[
<case>
pattern
Tpat_construct "Any"
…
Diagnostic removed (attribute in front of a GADT let):
Example:
type rec any = Any('a, 'a => string): any
let get = value => {
/** Destructures the existential. */
let Any(x, f) = value
f(x)
}
Before (diagnostic):
This pattern introduces an existential type from a GADT constructor,
which is not allowed here. Match on it inside a switch instead.
After (diagnostic): compiles
Diagnostic removed (unused variable of a pattern with @warning):
Example:
let silenced = o =>
switch o {
| @warning("-27") Some(x) => 1
| None => 0
}
Before (diagnostic):
Warning number 27
unused variable x.
After (diagnostic): no warning
Review: Why not a typed origin field on `Texp_match`, as #9126 suggests? Nothing after typing reads the origin; the field would change the typed tree for no consumer.
Review: Does `@warning("-8")` on a GADT let cover a non-exhaustive pattern? No: the exhaustiveness check runs for the whole match, outside the case pattern's warning scope, as for a switch case.
Fixes: #9126
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gHnoR8Goys4AMxET4q
Signed-off-by: Christoph Knittel <ck@cca.io>
Changelog🐛 Bug fix
Semantic Diff1 commit · Full page Pass the origin of a synthesized match as an argument, not an attribute3386811 · cknitt + Claude · Oct 29, 2026, 18:08 UTC
Typedtree modified (let destructuring a GADT constructor) type rec any = Any('a, 'a => string): any
let get = value => {
let Any(x, f) = value
f(x)
}
expression
- attribute "#let_gadt_pattern"
- []
Texp_match
expression
Texp_ident "value/1004"
<case>
pattern
Tpat_construct "Any"
…Diagnostic removed (attribute in front of a GADT let) type rec any = Any('a, 'a => string): any
let get = value => {
/** Destructures the existential. */
let Any(x, f) = value
f(x)
}
-This pattern introduces an existential type from a GADT constructor,
-which is not allowed here. Match on it inside a switch instead.
+compilesDiagnostic removed (unused variable of a pattern with let silenced = o =>
switch o {
| @warning("-27") Some(x) => 1
| None => 0
}
-Warning number 27
-unused variable x.
+no warningFixes: #9126 Review
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9128 +/- ##
==========================================
- Coverage 81.71% 81.71% -0.01%
==========================================
Files 460 460
Lines 64761 64757 -4
==========================================
- Hits 52918 52914 -4
Misses 11843 11843
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9128 |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ 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". |
cristianoc
left a comment
cristianoc
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks for the fix.
Fixes #9126.
The type checker rewrites two constructs into a match: a
letdestructuring a GADT constructor, and an optional parameter's default. It marked these matches with the string attributes#let_gadt_patternand#optional_arg_defaultso their diagnostics read as part of a function rather than a switch. Both attributes were created and read only insidetypecore.ml; nothing after typing reads them (translcore, printtyped, the mappers, reanalyze, editor analysis).type_expecttakes?match_context, read only by thePexp_matchcase. The GADTletrewrite passes`Function.type_lettakes?optional_default, which replaces the attribute scan (is_fake_let) and passes`Functionto the default's match.let.unwrapstays an attribute, since it is the parsed form oflet?and the formatter prints it back.What the origin selects
The diagnostics below are unchanged by this PR; they show what each origin is for. Each is compared with the same code written as a switch, which is what the compiler would report without the origin.
GADT
letis typed asswitch value { | Any(x, f) => f(x) }, so the rest of the block is the case's body:Written as a switch, the same code gets the switch wording; before #9081 the
letgot it too, although its source has no switch:Optional default is typed as
let count = switch countOpt { | Some(x) => x | None => "one" }, with the default in the synthesizedNonecase:The hand-written
let count: int = switch count { | Some(c) => c | None => "one" }gets "But this switch is expected to return: int / All branches in a switch must return…" instead.optional_defaultalso picks the unused-variable warning. Inlet f = (~count=1, ()) => "!",countis bound by the synthesizedlet, but it is a parameter in the source, so it gets warning 27 like any other unused parameter rather than warning 26 for an unusedletvariable.No change to the typed tree's shape or to
.cmtfiles. A typed origin field onTexp_match(the issue's first option) would add a field nothing reads; a typed GADTletform (second option) would touch translation,rec_check, the printers and analysis for no new behaviour.Also fixed along the way:
@inlineor@warningin front oflet Any(x, f) = valuewas rejected with "This pattern introduces an existential type … not allowed here". The binding's attributes now go on the case pattern.@warningon a pattern didn't cover that pattern's unused variables (| @warning("-27") Some(x) => 1still warned), becausetype_patternregisters those checks after leaving the pattern's warning scope. It now registers them inside it.Known limitation:
@warning("-8")on a non-exhaustive GADTletstill doesn't silence warning 8, since exhaustiveness is checked for the match as a whole (same as for a switch case).New tests:
tests/build_tests/super_errors/fixtures/pattern_warning_scope.res:@warningsilences unused variables on a GADTletwith a doc comment and on a switch case pattern; an unannotated case still warns.tests/tests/src/gadt_test.res:showOneruns a GADTletwith a doc comment and@warningin front.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kbf3gHnoR8Goys4AMxET4q