Repository navigation
Conversation
Changelog🐛 Bug fix
Semantic Diff1 commit · Full page ✏️ Compare constructor representations in interfaces and coercions alike594ff28 · cknitt + Claude · Oct 25, 2026, 06:54 UTC · edited
Diagnostic added (interface with an module M: {
type t = | @unboxed A(int) | B({x: int})
} = {
type t = | A(int) | B({x: int})
}
-none; clients read {TAG: "A", _0: 3} as B
+Type declarations do not match:
+ type t = A(int) | B({x: int})
+is not included in
+ type t = @unboxed A(int) | B({x: int})
+Their internal representations differ:
+case A is @unboxed only in the second declaration.Diagnostic added (coercion between representations) type a = | @unboxed A(int) | B({x: int})
type b = | A(int) | B({x: int}) | C
let x: b = (A(3): a :> b)
-none; the coerced 3 matches C
+Type a is not a subtype of bDiagnostic modified (unboxed configurations) @unboxed type x = One(bool) | Two
type y = One(bool) | Two
-- Variant x is not unboxed.
-- Variant y is unboxed.
+- Variant x is unboxed.
+- Variant y is not unboxed.Fixes #9038 |
|
@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. Hooray! 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". |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9039 +/- ##
=======================================
Coverage 81.32% 81.32%
=======================================
Files 456 456
Lines 64042 64046 +4
=======================================
+ Hits 52082 52087 +5
+ Misses 11960 11959 -1
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
Why:
An interface or a coercion accepted a tagged constructor for an
`@unboxed` one: inclusion compared only the tags of tagged constructors,
and the coercion check only the declared `@as` (#9038). The coercion
error also said that the wrong variant was unboxed.
`Variant_runtime.same_representation` compares two constructors: constant
with the same literal, tagged with the same tag, or both untagged. Interface
inclusion and the coercion check use it. Type declarations in messages print
the `@unboxed` of an untagged constructor of a type that is not `@unboxed`
itself, and an interface mismatch names the constructor that is `@unboxed`
in only one declaration. The predefined `unknown`, an
`@unboxed` variant, now has the untagged layout a declaration of it gets, so
`@unboxed type rec t = unknown = Unknown(_): t` still agrees with it.
Diagnostic added (interface with an @unboxed constructor):
Example:
module M: {
type t = | @unboxed A(int) | B({x: int})
} = {
type t = | A(int) | B({x: int})
}
Before (diagnostic): none; clients read {TAG: "A", _0: 3} as B
After (diagnostic):
Type declarations do not match:
type t = A(int) | B({x: int})
is not included in
type t = @unboxed A(int) | B({x: int})
Their internal representations differ:
case A is @unboxed only in the second declaration.
Diagnostic added (coercion between representations):
Example:
type a = | @unboxed A(int) | B({x: int})
type b = | A(int) | B({x: int}) | C
let x: b = (A(3): a :> b)
Before (diagnostic): none; the coerced 3 matches C
After (diagnostic):
Type a is not a subtype of b
Diagnostic modified (unboxed configurations):
Example:
@unboxed type x = One(bool) | Two
type y = One(bool) | Two
Before (diagnostic):
- Variant x is not unboxed.
- Variant y is unboxed.
After (diagnostic):
- Variant x is unboxed.
- Variant y is not unboxed.
Fixes #9038
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t
Signed-off-by: Christoph Knittel <[email protected]>
8db3240 to
594ff28
Compare
cristianoc
left a comment
cristianoc
left a comment
There was a problem hiding this comment.
I think we better remove per-case unboxed.
It obviously never worked and is probably not needed anymore.
|
Superseded by #9040, which removes |
Pull request was closed
Fixes #9038.
An interface or a coercion accepted a tagged constructor for an
@unboxedone. Interface inclusion compared only the tags of tagged constructors, and the coercion check only the declared@as, so a value built as{TAG: "A", _0: 3}was read by clients as another case.Variant_runtime.same_representationcompares two constructors at run time: both constant with the same literal, both tagged with the same tag, or both untagged.Includecore.compare_variantsand the coercion check inCtypeboth use it.Variant_coercion.variant_representation_matchesis gone.unknown, an@unboxedvariant, had a tagged layout. It now has the untagged layout that a declaration of it gets, so@unboxed type rec t = unknown = Unknown(_): tstill agrees with it.@unboxedof an untagged constructor when the type isn't@unboxedas a whole. Before, both sides of the mismatch printed astype t = A(int) | B({x: int}). The mismatch now says "case A is @unboxed only in the second declaration".@unboxedconfigurations named the wrong variant as unboxed. The existing snapshotvariant_to_variant_coercion_unboxedshowed this, and it is corrected.New tests: fixtures
untagged_constructor_interface_mismatch.resanduntagged_constructor_coercion_mismatch.res.Found by a review of the untagged variant work (#8950).
🤖 Generated with Claude Code
https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t