Skip to content

Commit bcf81d1

Browse files
cknittclaude
andauthored
Treat an empty record literal with mutable fields as expansive (#9152)
Why: The value restriction looks for mutable fields among a record literal's entries, but `{}` of an all-optional record has none, so it was always non-expansive and its type variables were generalized even when a field is mutable (#9150). It now looks up the record's declaration and requires every field to be immutable. Diagnostic added (empty record literal with a mutable field): Example: type cell<'a> = {tag?: int, mutable value?: 'a} let cell: cell<_> = {} cell.value = Some(1) let s: option<string> = cell.value Before (diagnostic): none; `s` holds the number 1 at run time After (diagnostic): This has type: option<int> But it's expected to have type: option<string> The incompatible parts: int vs string Review: Does `{}` of an immutable record still generalize? Yes; the super_errors fixture uses one such value at both option<int> and option<string> before reaching the error. Claude-Session: https://claude.ai/code/session_01Kbf3gKyiYMf54wPRxFdUWe Signed-off-by: Christoph Knittel <ck@cca.io> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 940b761 commit bcf81d1

4 files changed

Lines changed: 35 additions & 1 deletion

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Stop generalizing the type of an empty record literal `{}` whose record has a mutable field, which let a value be stored at one type and read at another.

‎compiler/ml/typecore.ml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2095,7 +2095,14 @@ let rec is_nonexpansive exp =
20952095
| None -> (
20962096
(* The optional fields the literal leaves out are allocated too *)
20972097
match fields with
2098-
| [||] -> true
2098+
| [||] -> (
2099+
(* [{}] has no field to find the record's labels through *)
2100+
match extract_concrete_record exp.exp_env exp.exp_type with
2101+
| _, _, decls, _ ->
2102+
List.for_all
2103+
(fun (ld : Types.label_declaration) -> ld.ld_mutable = Immutable)
2104+
decls
2105+
| exception Not_found -> false)
20992106
| _ ->
21002107
let lbl, _, _ = fields.(0) in
21012108
Array.for_all (fun lbl -> lbl.lbl_mut = Immutable) lbl.lbl_all))
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
2+
We've found a bug for you!
3+
/.../fixtures/record_empty_mutable_does_not_generalize.res:10:25-34
4+
5+
8 │ let cell: cell<_> = {}
6+
9 │ cell.value = Some(1)
7+
10 │ let s: option<string> = cell.value
8+
11 │
9+
10+
This has type: option<int>
11+
But it's expected to have type: option<string>
12+
13+
The incompatible parts:
14+
int vs string
15+
16+
You can convert int to string with Int.toString.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// An immutable record still generalizes.
2+
type options<'a> = {value?: 'a}
3+
let defaults: options<_> = {}
4+
let ints: options<int> = defaults
5+
let strings: options<string> = defaults
6+
7+
type cell<'a> = {tag?: int, mutable value?: 'a}
8+
let cell: cell<_> = {}
9+
cell.value = Some(1)
10+
let s: option<string> = cell.value

0 commit comments

Comments
 (0)