Stop Simplif from causing disagreement between runtime arity and syntatic arity of a function

[ncik-roberts]

A stated goal of the Syntactic Arity RFC (ocaml/RFCs#32) was for runtime arity to match syntactic arity¹. The implementation of syntactic arity in #12236 does not fully implement that goal (unintentionally). For example, even the motivating make_greeter example given in the RFC currently compiles as a function with runtime arity 3, though the spec given in the RFC suggests its runtime arity should match its syntactic arity of 2. This isn't a question of correctness but rather a question of performance: calling a function with runtime arity 2 is optimized at application sites to 2 syntactic arguments.

This PR fixes the implementation to match the RFC.

The piece we missed in #12236: Simplif looks for nested lfunctions and fuses their parameter lists. In order for runtime arity to match syntactic arity, this rewrite shouldn't happen for functions that correspond to fun/function terms that the user wrote.

Functors

Notably, this PR does not change how Simplif's parameter fusion rewrite applies to other constructs that compile to functions, like functors and class functions. The arity of these constructs are out of the scope of the original syntactic arity RFC and PR (though perhaps one day we'll want to follow the RFC's model for these constructs as well). Furthermore, basic experimentation shows that multi-argument functors rely on Simplif's rewrite to be given the "expected" arity; e.g. module Make (X1 : S) (X2 : S) = ... is only given an arity of 2 by virtue of this rewrite and would otherwise be a 1-ary function returning a 1-ary function.

If it's decided that it's not important to preserve this rewrite for functors/class functions, it would simplify this PR to just delete Simplif's rewrite instead of the current approach of excepting fun/function terms.

Review

To review this PR, I'd suggest reading the two commits in order. The first commit adds a regression test demonstrating the discrepancy, and the second commit fixes it and updates the test output.

Footnotes

1. "Function definitions in OCaml are curried by default, in principle accepting one argument at a time and returning a closure accepting the rest. Since most function applications are saturated (that is, they pass the same number of arguments as the function definition expects), the implementation of functions is optimised to make saturated applications fast. [...] However, this optimisation requires knowing how many arguments a function definition expects. Since this information is not currently tracked in the parse tree, it must be guessed later, and sometimes this guess is wrong. [...] N-ary lambdas would make this issue go away, because the number of parameters of a function is known rather than inferred." ↩

[lthls]

I'm not a huge fan of using allocations to test arity.
You can get the runtime arity of a function using a bit of Obj:

let runtime_arity (f: _ -> _) =
  let clos_info = Obj.raw_field (Obj.repr f) 1 in
  let raw_arity =
    Nativeint.(to_int (shift_right clos_info (Sys.word_size - 8)))
  in
  if raw_arity < 0 then (* tupled function *) 1 else raw_arity

Do you think you could rewrite the test to use that instead ?

[ncik-roberts]

@lthls That seems reasonable enough. I've made that change. I force-pushed it so that any reviewer can still confirm that the test changes output as expected due to the code change.

[goldfirere]

This is ready to merge from my perspective. Does this need any more review before pulling the trigger?

[lthls]

The test file ended up more complex than I expected, but I'm fine with it (modulo a comment that I think is slightly wrong).
I have also looked at the PR itself and haven't found anything suspicious.

[lthls]

When reading this sentence, it looks like fun _ _ -> 0 would have nested arity [Curried 2; Curried 1] (for some values of 2 and 1).
But I think what you actually compute is the expected [Curried 2]. Maybe we could replace to a value by to the number of values corresponding to its arity ?

[ncik-roberts]

Thanks for the correction.