Untagged variants: boolean literal/payload overlap is rejected while string, int, float and bigint overlap is allowed

[cristianoc]

Summary

An untagged variant may declare a constant constructor whose runtime value is also inhabitable by a payload constructor's type. This is accepted for string, int, float and bigint, and rejected for bool. The four accepted cases and the rejected one are the same phenomenon, so one of the two rules is wrong.

The acceptance matrix

Checked against master:

declaration	result
@unboxed type t = | @as("a") A | S(string)	allowed
@unboxed type t = | @as(1) A | I(int)	allowed
@unboxed type t = | @as(1) A | F(float)	allowed
@unboxed type t = | @as(1.0) A | I(int)	allowed
@unboxed type t = | @as(1n) A | Bg(bigint)	allowed
@unboxed type t = | A | S(string) (no @as)	allowed
@unboxed type t = | @as(true) A | B(bool)	rejected — At most one case can be a boolean type.

For contrast, the rules that are consistent and clearly right:

declaration	result
@unboxed type t = S1(string) | S2(string)	rejected — At most one case can be a string type.
@unboxed type t = | @as("a") A | @as("a") B	rejected — Duplicate literal a.
type t = @as("a") A(int) | @as("a") B(int) (tagged)	rejected — Duplicate literal a.

Where it comes from

compiler/ml/ast_untagged_variants.ml:275-278:

if
  !boolean_types > 0
  && (String_set.mem "true" !nonstring_literals_consts
     || String_set.mem "false" !nonstring_literals_consts)
then raise (Error (loc, InvalidUntaggedVariantDefinition AtMostOneBoolean));

Meanwhile check_invariant keeps the constant and block literal sets deliberately separate (ast_untagged_variants.ml:224-227), which is exactly what lets the string/int/float/bigint cases through:

let string_literals_consts = ref String_set.empty in
let string_literals_blocks = ref String_set.empty in
let nonstring_literals_consts = ref String_set.empty in
let nonstring_literals_blocks = ref String_set.empty in

Why the boolean rule isn't justified by totality

The intuitive defence would be that a bool payload overlapping a boolean literal is totally shadowed. It isn't: with @as(true) A | B(bool), B(false) is still reachable and distinguishable, exactly as Color("blue") is reachable in the string case. So the boolean rejection is stricter than the reasoning that would justify it, and stricter than the sibling rules.

Note also that @as is not required to create the overlap — a constructor without one is represented by its own name:

@unboxed type t = Primary | Color(string)
// Color("Primary") is the string "Primary", and matches as Primary

Why it matters now

The permissive answer has a track record. #6950 was a miscompilation arising directly from this overlap — the optimizer resolved such a match by constructor identity while codegen resolved it by value. It was fixed in #8631.

While looking at that fix I built a differential test (fold each case, then compute it again through an @inline(never) boundary, compare at runtime). Against the pre-fix compiler it found six disagreements rather than the one reported, including two mechanisms not in the original report:

• @as(1) against a float payload — Int 1 and Float "1." are different literal tags but one JavaScript number
• a payload that is itself an untagged constant, so the inner constructor's value collides with the outer's literal

So the overlap is easy to construct accidentally and hard to reason about, which is an argument for deciding the rule deliberately rather than leaving it as an artifact of which check happens to fire.

The decision

Either:

(a) The overlap is intended. Then the boolean rejection should be relaxed to match the others, and the semantics should be written down: declared literals are tested first, so a payload constructor only receives values that are not a declared literal, independent of declaration order and of the order of switch arms.

(b) The overlap is a mistake. Then it should be rejected for string, int, float and bigint too — which is a breaking change for a pattern that is idiomatic today ("a few known values, plus an escape hatch") and is presumably why it was allowed in the first place.

My own view is (a), but it is a language-design call rather than a bug fix, which is why this is an issue and not a PR.

Either way the invariant deserves a home in the source — Variant_runtime.matching_facts is where literal_tags lives and would be the natural place to state that constructor identity is not observable at runtime for these types.

Related

• Docs: unboxed variant "no overlap" rule is stated incorrectly — constant vs payload overlap is allowed and undocumented rescript-lang.org#1337 — the manual currently states the opposite ("the compiler will guide you and ensure there's no overlap"). If this is resolved as (b), that documentation change needs revisiting, so it may be worth settling this issue first.