Skip to content

Docs: unboxed variant "no overlap" rule is stated incorrectly — constant vs payload overlap is allowed and undocumented #1337

Description

@cristianoc

Problem

The manual states that unboxed variants cannot have constructors that overlap in their runtime representation, and reassures the reader that the compiler enforces it. Neither is true for one specific, legal, and fairly common shape.

apps/docs/markdown-pages/docs/manual/variant.mdx:382-389 — "Advanced: Unboxing rules / No overlap in constructors":

A variant can be unboxed if no constructors have overlap in their runtime representation.

For example, you can't have String1(string) | String2(string) in the same unboxed variant, because there's no way for ReScript to know at runtime which of String1 or String2 that string belongs to, as it could belong to both.
The same goes for two records - even if they have fully different shapes, they're still JavaScript object at runtime.

Don't worry - the compiler will guide you and ensure there's no overlap.

apps/docs/markdown-pages/syntax-lookup/decorator_unboxed.mdx:10:

The @unboxed decorator provides a way to unwrap variant constructors that have no overlap in their runtime representation, or record objects that have a single field.

What's actually true

The type-level rule is correct — String1(string) | String2(string) really is rejected. But a constant constructor's runtime value overlapping a payload constructor's type is explicitly allowed:

@unboxed
type color =
  | @as("primary") Primary
  | @as("secondary") Secondary
  | Color(string)

Color("primary") and Primary are the same runtime value — the string "primary". The compiler accepts this declaration and does not warn. check_invariant in compiler/ml/ast_untagged_variants.ml:224-227 keeps the constant and block literal sets deliberately separate, so this never registers as a duplicate.

So a reader following the manual would conclude Color("primary") cannot happen. It can, it is legal, and it has a defined meaning that the manual never states.

The undocumented semantics

Declared literals are tested first; a payload constructor only receives values that are not a declared literal:

function colorName(value) {
  if (value === "primary" || value === "secondary") { return "not Color"; }
  else { return value; }
}

So colorName(Color("primary")) is "not Color" — the literal wins, and the Color branch is never reached for that value.

This is independent of both orderings. I checked all four combinations and every one selects the literal, emitting byte-identical JS:

result
payload declared first: Color(string) | @as("primary") Primary literal
literal declared first literal
payload arm first in the switch literal
literal arm first in the switch literal

It also holds when the literal has no arm at all and is reachable only through _.

@as is not required for this, which is worth stating explicitly — a bare constructor's runtime value is its own name, so this already overlaps:

@unboxed type t = Primary | Color(string)
// Color("Primary") matches as Primary

Suggested change

Keep the existing rule about payload types, then add the constant-vs-payload case rather than implying it cannot occur. Roughly:

  • Two payload constructors may not share a runtime type — this the compiler does enforce.
  • A constant constructor's value may coincide with a value a payload constructor can hold. This is allowed, and is often what you want when modelling "a few known values, plus an escape hatch".
  • When it happens, the constant constructor wins: matching tests the declared literals first, and a payload constructor only receives values that are not one of them. This does not depend on the order of constructors in the declaration or the order of arms in the switch.
  • A constructor without @as is represented by its own name, so it participates in this too.

The line "Don't worry - the compiler will guide you and ensure there's no overlap" is the one I'd most want changed, since it tells the reader not to reason about a case they do need to reason about.

Background

This is not hypothetical. rescript-lang/rescript#6950 was a miscompilation caused by the optimizer resolving such a match by constructor identity rather than by value, fixed in rescript-lang/rescript#8631.

There is a related open question on the compiler side about whether this overlap should be permitted at all — it is currently rejected for bool but accepted for string, int, float and bigint. See rescript-lang/rescript#8632. If that is resolved in favour of rejecting the overlap, this documentation change would need revisiting, so it may be worth settling that first.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions