Syntactic function arity parsing

[ncik-roberts]

This is the parsing half of syntactic function arity. Unlike the upcoming typechecking/translation PR, this PR does not change the semantics of any existing OCaml program. But it does place new requirements on ppxes, so we should wait to merge it until some internal PRs for ppxes land.

@goldfirere for review please.

This PR transplants bits of the upstream PR (ocaml/ocaml#12236) to flambda-backend.

At a high level:

• The OCaml parser now keeps track of function arity, encoding this in the parsetree using the Jane Syntax framework.
• The sanity checks that run on ASTs produced by ppxes now require that functions were encoded using the Jane Syntax framework.
• For now, typechecking drops this arity information. (It constructs another parsetree that doesn't have any arity information and typechecks that.)

How to review

• Review the first two commits (Add some locals tests and Make Jane Syntax treat attributes as inner-to-outer) in isolation. See "Things of note".
• Then look at the entire diff of the PR.

This PR does not share much code with the upstream PR, except for some pieces of the parser.

Things of note

Making Jane Syntax treat attributes as inner-to-outer

One of the early commits ensures the following things hold:

1. Jane Syntax should interpret later attributes before earlier attributes. ("Later" and "earlier" are with respect to the order of attributes in, say, the pexp_attributes list.)
2. When the user writes something like [: :] [@attr], the [@attr] attribute should be placed later in the list than any Jane Syntax attribute.

(1.) did not hold previously, but (2.) did. This PR makes both properties hold. It's important for (1.) to hold because, without it, this PR would be incompatible with the way that ppxlib merges attributes. For example, suppose a ppx replaces [%ppx_extension] with fun y z -> y + z, which is encoded in Jane Syntax as (fun[@jane.erasable._builtin] y -> fun[@jane.erasable._builtin.param] z -> (y + z)[@jane.erasable._builtin.end]). Then,

(* source program *)
(fun x -> [%ppx_extension])

(* encoding as Jane Syntax parsetree when handed to the ppx *)
(fun[@jane.erasable._builtin] x -> [%ppx_extension] [@jane.erasable._builtin.end])

(* result of running ppx, keeping in mind that ppxlib merges attributes by placing outer attributes after
   inner ones *)
(fun[@jane.erasable._builtin] x -> fun[@jane.erasable._builtin][@jane.erasable._builtin.end] y -> fun[@jane.erasable._builtin.param] z -> (y + z)[@jane.erasable._builtin.end])

So, for fun[@jane.erasable._builtin][@jane.erasable._builtin.end] y -> ..., Jane Syntax likewise needs to interpret @jane.erasable._builtin.end before @jane.erasable._builtin, because otherwise it will think the AST is invalid.

alloc_mode on function type constraints

The parsetree encoded in Jane Syntax is different: there's an alloc_mode field of the type constraint. This is because the Jane Street OCaml parser treats this construct specially:

let local_ f x : int -> int -> int = fun y z -> x + y + z

We want this type to be treated by the type-checker as local_ (int -> int -> int). The old parser accomplished this by putting an attribute on the Pexp_constraint node, but given that we're no longer constructing a Pexp_constraint node directly, I instead add a new field to the type_constraint. I add some tests in this PR because we weren't testing this behavior before.

Merge this as-is or wait for typechecking?

TODO @ncik-roberts : I need to check that typedtree is unchanged if we want to merge this.

The main reason I can think to not merge this is that it constructs, and then throws away, a dummy parsetree just for purposes of typechecking. You'd expect this to impact minor heap allocations, and it looks like it does. (For a relevant benchmark, this is 1.9% worse: 15.9 TB vs. 15.6 TB.) But minor heap allocations are cheap, so maybe we want to merge this anyway in the interest of merging smaller, simpler PRs.

[goldfirere]

Instead of accumulating lots and lots of comments in a complete review, I've decided it's best just to send comments every day they accumulate. More to come. (Note to self: stopped in Parser.all_params_as_newtypes.)

[goldfirere]

A few comments to handle, but looks pretty good. Sorry for how long this has taken to get reviewed!

[ncik-roberts]

Just now, I unintentionally pushed to the wrong remote, so my latest force-push backs that out. I haven't responded to the new comments yet.

[ncik-roberts]

I just pushed a change to use a less attribute-heavy encoding. Now, a function has at most 3 Jane Syntax attributes, but usually only 1:

1. The usual outer attribute identifying the AST as a Jane Syntax construct.
2. An attribute on the last Pexp_fun node if its body is function cases.
3. An attribute on the Pexp_constraint or Pexp_coerce node if its type should be interpreted at the local mode.

The less-attribute-heavy encoding was a secondary goal. The primary goal was to avoid putting any attributes on the function body itself and instead keep them on the Pexp_fun nodes. That way, ppxes that transform or transplant the body of a function don't get in the way of Jane Syntax.

[ncik-roberts]

TL;DR: The rebase was messy, and the commit history is much less useful as a result. I recommend just reviewing the overall diff.

The rebase is messy because #1417 introduced the need to support things like:

let f x y (type a : immediate) z = ...

We need to embed this information in the n-ary functions encoding, as the (type a : immediate) shouldn't interrupt a function's arity. There are lots of intermediate commits in this PR, and indeed many that change the attribute encoding, and I didn't want to support (type a : immediate) at every step of the way. Instead of fixing up every intermediate commit to get it compiling and working, I just fixed up one of the latest commits.

In other words, most of the early commits no longer even compile, let alone work.

Here is a brief summary of the changes I made in order to support (type a : immediate):

• I moved the chunk of code that encodes/decodes layout annotations as payloads so I could reuse it for n-ary functions.
• I embedded the layout annotation as a _builtin attribute, but I checked that layouts were enabled (at any maturity) before allowing the (type a : immediate) syntax. The usual layout-maturity check is run to ensure that immediate is only allowed at alpha.

[ncik-roberts]

The most recent pair of commits transplants a commit from upstream that embeds the parameter location in the parsetree. This isn't strictly needed for this PR, but it will simplify the next PR.

[ncik-roberts]

TODO @ncik-roberts : I need to check that typedtree is unchanged if we want to merge this.

I ran a test against the stdlib and observed that the produced typedtree is unchanged, with one exception: the location of a function is now marked as ghost in the typedtree for constructions like let f (type t) x = x. I have reason to believe this change is fine: the function node in let f x = x is marked as ghost already in the upstream compiler, and merlin does not have any trouble with this. (Furthermore, I think it's just correct for the Texp_function in let f (type t) x = x to be marked as ghost — it doesn't appear literally in the source text.)

[goldfirere]

OK. It sounds like this is ready for re-review from me, given the rebase on top of #1417. Is that correct?

[ncik-roberts]

Yes, this is now ready for re-review from you.

[ncik-roberts]

My latest rebase is on top of the Uniqueness PR, which does affect n-ary lambdas because of constructs like let local_ f x : int -> int = .... (We need the type annotation to be interpreted at the provided mode.) Prior to the rebase, the only mode allowed in that position was local_, but now unique and once can be provided there, too.

[goldfirere]

This is all good from my point of view. Thanks!

[ncik-roberts]

The last rebase resolves some trivial conflicts with #1761. I'll go ahead and merge this after some internal testing finishes.