Merlin generates ASTs with bad locations

[lpw25]

Given the following file:

let () = (Foo.Bar;;

merlin's parser will generate a parsetree with non-ghost sibling locations that overlap:

$ ocamlmerlin single dump -protocol sexp -what parsetree -filename ~/test/merlin.ml < ~/test/merlin.ml
((assoc) (class . "return") (value . "[
  structure_item (merlin.ml[1,0+0]..[1,0+17])
    Pstr_value Nonrec
    [
      <def>
        pattern (merlin.ml[1,0+4]..[1,0+6])
          Ppat_construct \"()\" (merlin.ml[1,0+4]..[1,0+6])
          None
        expression (merlin.ml[1,0+9]..[1,0+17])
          Pexp_open Fresh
          module_expr (merlin.ml[1,0+10]..[1,0+17])
            Pmod_ident \"Foo.Bar\" (merlin.ml[1,0+10]..[1,0+17])
          expression (merlin.ml[1,0+10]..[1,0+17])
            Pexp_construct \"()\" (merlin.ml[1,0+17]..[1,0+17])
            None
    ]
]


") (notifications) (timing (assoc) (clock . 53) (cpu . 3) (query . 0) (pp . 0) (reader . 3) (ppx . 0) (typer . 0) (error . 0)))

Note the locations on the expression and module expression for the local open.

These locations now blow-up when they pass through ppxlib -- because ppxlib ironically tries to protect merlin from the bad locations produced by ppxes -- and so merlin stops working on these files.

[let-def]

It seems that even correct code is generating overlapping locations, both with Merlin and OCaml parsers:

let () = (Foo.Bar.());;

[
  structure_item (test.ml[1,0+0]..[1,0+21])
    Pstr_value Nonrec
    [
      <def>
        pattern (test.ml[1,0+4]..[1,0+6])
          Ppat_construct "()" (test.ml[1,0+4]..[1,0+6])
          None
        expression (test.ml[1,0+9]..[1,0+21])
          Pexp_open Fresh
          module_expr (test.ml[1,0+10]..[1,0+17])
            Pmod_ident "Foo.Bar" (test.ml[1,0+10]..[1,0+17])
          expression (test.ml[1,0+10]..[1,0+20])
            Pexp_construct "()" (test.ml[1,0+18]..[1,0+20])
            None
    ]
]

[let-def]

Does ppxlib reject this?

[trefis]

It seems that even correct code is generating overlapping locations, both with Merlin and OCaml parsers:

That's a known issue yes, there are a bunch of escape hatchs builtin ppxlib's check (cf. https://github.com/ocaml-ppx/ppxlib/blob/master/src/location_check.ml#L370 ) until they get fixed upstream (preliminary PR: https://github.com/ocaml/ocaml/pull/8987 )

[let-def]

So in the short term it is probably better to add this case to ppxlib.

[trefis]

FTR: the upstream PR has finally be merged. So 4.12 merlin might be a bit better when it comes to locations (but I'm still not sure what will happen in the presence of recovery).