Flambda1 region deletion and locals fixes

[stedolan]

The original impetus for this patch was to squash an old TODO in Flambda1's treatment of local regions. Flambda deletes unused regions, both as an optimisation in itself and because they inhibit other optimisations. Originally, it conservatively assumed that any region containing an application was used, since it didn't track whether applications may allocate on their caller's region.

Nowadays, that information is tracked in the ap_mode field, so we can make the optimisation more precise by marking a region used only for those applications which do allocate on their caller's region. This is the first commit.

---

However, this commit causes a testsuite failure. When Flambda1 generates a partial application closure (that is, expands a partial application f a to a closure fun b -> f a b), the ap_mode field on the generated application (f a b) was incorrect. Previously, this didn't matter (ap_mode was only used to control generation of the indirect call stubs caml_applyN, but the application generated here is never indirect), but with the new region-deletion logic it starts to matter.

So, the second commit here fixes the partial appliction modes, copying the correct logic from Closure. Since modes are available from two different sources (from the closure and from the application site), it also adds an assertion that these modes are compatible, which is ensured by typechecking.

---

However, that commit caused the compiler to fail to build, due to the assertion above firing on a use of Obj.magic in boot/menhir/menhirLib.ml that looked like this:

Obj.magic discard env

Note that this is (Obj.magic discard) env, not Obj.magic (discard env) - the magic is being applied to the function rather than to its result. Since discard is a two-argument function, this is a partial application that can get the modes wrong. This turns out to be unsafe: the following example crashes in the debug runtime (and variants corrupt memory / segfault):

module M : sig
  val f : int -> string -> int * string
end = struct
  let[@inline never] pair a b =
    a, b

  let f x =
    Obj.magic pair x
end

let g = (Sys.opaque_identity M.f) 42
let () = Gc.compact ()

This code is completely safe without stack allocation, but with stack allocation turned on the compiler stack-allocates the partial application yet claims it is heap allocated via Obj.magic.

So, since the assertion seems to find real issues, I left it in. This will cause some compilation failures. I patched Menhir manually (and added a horrific sed script to do so on future menhir upgrades), but there may be other occurrences of this issue. The fix is to parenthesise calls to Obj.magic (or better, to give explicit type annotations indicating which types you plan to magic to and from)

[lpw25]

    Obj.magic pair x

Stephen and I discussed this general pattern. We could try to warn about it in the compiler, since there is an existing warning which has to go out of its way to not trigger here. Probably though the best option is to force all Obj.magic to write their to and from types as constraints -- which should be easy to enforce in ppx_js_style. Note that this issue isn't specific to locals -- when we have functions with arity it will be even worse.

[mshinwell]

@lukemaurer is going to review the Flambda 1 parts of this PR.

[mshinwell]

As a note, we've merged #1008 which is the Flambda 2 version of the new check.

[stedolan]

ping @mshinwell

[mshinwell]

@lukemaurer has read this