Repository navigation
Inlining binds an argument before a read of an immutable record field #9223
Description
With #9177, inlining keeps the arguments of a call in call order: when the function body uses an argument with side effects after a later argument, that argument is bound to a variable unless the later one is safe to read after it. A record field read never counts as safe, even when the field is immutable:
type p = {x: int}
type m = {mutable y: int}
@val external eff: unit => int = "eff"
let minus = (a, b) => b - a
let f = (p: p) => minus(eff(), p.x)
let g = (m: m) => minus(eff(), m.y)function f(p) {
let a = eff();
return p.x - a | 0;
}
function g(m) {
let a = eff();
return m.y - a | 0;
}g needs the binding, since eff() may assign m.y. f does not: p.x cannot change, and before #9177 f compiled to return p.x - eff() | 0;.
Lambda does not record whether a field is mutable. Translcore builds a field read as Pfield (pos, Fld_record {name}) (compiler/ml/translcore.ml), although the label's lbl_mut is known there, so Lam_analysis.delay_safe treats every Fld_record read as a read of state that may change. Recording the mutability in Fld_record and Fld_record_inline would let delay_safe accept reads of immutable fields and keep the previous output for them.
Raised in the review of #9177.
Activity
mununki commentedon Nov 13, 2026
Implementing this would bring back a miscompilation, so I'm closing it.
A field declared immutable can still change. Record coercion lets a record with a mutable field be seen through a type where the field is immutable (#9086 rejects only the other direction):
eff()runs beforep.xis read, so this prints-8, as master does:12.3.1 compiles
ftoreturn p.x - eff() | 0;and prints-9. Treating the read of an immutable field as safe to move gives that output again.The declaration of
pdoes not tell whether another type, in any module, sees the field as mutable, so I see no sound way to do this for a record that comes from elsewhere. A JavaScript object bound as a record can change as well.Pattern matching already makes this assumption, and gets the same kind of wrong result. With
randviewfrom above:gcompiles toreturn next()(r.x);on master and 12.3.1 and returns2; it should return1. The field of a destructured record is bound as an alias when the field is immutable, and an alias may be read where it is used.let x = view.xinstead of the pattern gives1with #9296.The pattern case needs an issue of its own.