Inlined record fields for mixed blocks

[ncik-roberts]

For any reviewer

The key change is in ocaml/typing/types.mli. This is where I extend Record_inlined to additionally track the constructor representation, which may be mixed. The design choice here really guides the rest of this PR.

Usefully, changing this constructor forces me to visit all places that inspect it. It's crucial that these use sites now distinguish between Constructor_uniform_value and Constructor_mixed, as this choice informs e.g. what primitives should be used to project from the record in question.

For typing/translation reviewer — I suggest @ccasin

Congratulations! You have the trickiest pieces of code to review:

• ocaml/typing/typedecl.ml
• ocaml/typing/typeopt.ml

typeopt.ml really just is an instance of "visit use site of Record_inlined and update it to handle mixed constructors", but it's the trickiest instance, as it pushes me to factor out the shared projection code between tupled constructor args and inlined record constructor args.

typedecl.ml is not complicated, but I call it out because it's not a straightforward instance of "chase compiler errors". This is the place that actually starts allowing for mixed inlined records.

For tests review — I suggest @ccasin

Most of the tests are straightforward extensions/adaptions of existing tests for mixed constructors and mixed records.

The novel thing about inlined records are mutable updates. See layout-specific test_*_alpha.ml for examples of that. (Note: we should move these tests to a file that doesn't have "alpha" in the name. This was missed in #2664. I'll do that in a follow-up.)

You can diff the output for variants with the output for records to judge whether the answers are correct:

# floats
$ diff \
  <(sed -n '/Test 17, construction:/,/Test 17.1, construction:/p' ocaml/testsuite/tests/typing-layouts-float64/unboxed_floats_beta.reference) \
  <(sed -n '/Test 17.1, construction:/,/Test 18/p' ocaml/testsuite/tests/typing-layouts-float64/unboxed_floats_beta.reference)

# nativeints, int32s, int64s
for FILE in ocaml/testsuite/tests/typing-layouts-word/unboxed_nativeints_alpha.reference ocaml/testsuite/tests/typing-layouts-bits32/unboxed_bits32s_alpha.reference ocaml/testsuite/tests/typing-layouts-bits64/unboxed_bits64s_alpha.reference; do
  diff \
    <(sed -n '/Test mixed record construction:/,/Test mixed variant construction:/p' $FILE) \
    <(sed -n '/Test mixed variant construction:/,/Test 18/p' $FILE)
done

For flambda2 reviewer

All the changes here are straightforward instances of chasing type-checker errors due to the change to Record_inlined. We'll want to support extensible variants at some point, but given that they are rejected by the type-checker for now, I defer this work for later and raise on patterns like:

Record_inlined (_, Constructor_mixed _, Variant_extensible)

[mshinwell]

@ncik-roberts is this ready to be merged after rebase?

[ncik-roberts]

Ah, I meant to merge this, yes. Thanks for the ping. It's now ready to merge.

CI is not going to pass because #2737 appears to have broken it. I would like to merge this before 5.1.1minus-19 is tagged, ideally.

[mshinwell]

@ncik-roberts if you rebase again CI should work

[ncik-roberts]

There's something funny going on here with float32s, GC roots, and maybe also runtime5. I'll have to look next week. Here's a small repro that segfaults (apparently unrelated to this PR, but this PR's tests manage to trigger it):

module Float32_u = Stdlib_beta.Float32_u

type t =
  | Mutable_str of { mutable x : string }
  | Float32 of float32#

let[@inline always] go x y =
  let f =
    match x with
    | Float32 f32 ->
      (fun[@inline never] () ->
         match y with
         | Mutable_str _ -> f32
         | Float32 _ -> assert false)
    | Mutable_str _ -> assert false
  in
  f ()

let () =
  let x = Float32 #4.s in
  let y = Mutable_str { x = "s" } in
  let _ = go x y in
  ()

Build with runtime 5 and -I +stdlib_beta stdlib_beta.cmxa -extension layouts_alpha -extension small_numbers. (It segfaults with and without O3.)

[mshinwell]

I'm looking at this segfault.

[mshinwell]

The segfault is fixed by https://github.com/ocaml-flambda/flambda-backend/pull/2763

[ncik-roberts]

I've rebased now that #2763 is merged into main, and will merge this PR if CI passes.