I spent a good few hours understanding why the CI tests in my PR to mirage/repr were still failing. It turns out, the bug, is fairly simple to reproduce. Consider the following OCaml code:
let g : int = 1
We have a let-binding that contains a type constraint. Where, in the abstract syntax tree sense, do we attach the constraint? Since OCaml 5.1, we have quite a few options for how this can be represented.
The first is using the Ppat_constraint AST node which allows you to attach a type constraint to any pattern.
| Ppat_constraint of pattern * core_type
This tends to be used for things like function arguments (e.g. fun (x : int) -> x + 1). So it doesn't quite capture what we want for this binding constraint.
On older OCaml compilers, let g : int = 1 also includes a Pexp_constraint on the body of the value binding. This makes sense if we consider a more general case of the syntax where we allow functions.
let g x : int = x + 1
In this example, we no longer parse a Ppat_constraint and instead this is treated as syntactic sugar for:
let g x = (x + 1 : int)
In OCaml 5.1 a new field for value bindings was added allowing type constraints to be tracked directly in the bindings themselves.
and value_binding =
{
pvb_pat: pattern;
pvb_expr: expression;
pvb_constraint: poly_constraint option;
pvb_attributes: attributes;
pvb_loc: Location.t;
}
Our type constraint (: int) can be recorded in the pvb_constraint directly. This is a nice change, bringing the concrete syntax of the language closer to the parsetree. For ppxlib, however, it does create a bit of a headache as we now have to encode the different versions of expressing type constraints into the AST of the compiler before 5.1!
So what was the bug? The author's of repr had added some value bindings to circumvent some "unused value warnings" in OCaml. They choose to write these bindings like:
let (_ : int) = 1
Here, I have simplified the types and the body of the binding. The significant thing to notice is the parentheses surrounding the value binding's pattern. This forces the AST representation to be a single Ppat_constraint node regardless of the current compiler version. No Pexp_constraint and no pvb_constraint.
However, using ppxlib with a compiler less than 5.1 meant that this did not roundtrip cleanly. Instead, the value after passing through ppxlib's migrations was:
let _ : int = 1
After bottoming out at the issue, I discovered we already had a test case for this in ppxlib! Whatsomore, ocamlformat.0.20.0 (which repr was still using) actually transforms instances of let x : t = ... into let (x : t) = ...! This limitation does not seem inherent to the parsetree, the printer or the migrations. But I have asked the (very kind and friendly) previous maintainers about these changes.