Fix 1913

[voodoos]

This should fix the short path issue in #1913

OCaml 5.3 introduces an additional Env.add_type in solve_constructor_annotation that should not be added to the graph.

@mikeshulman could you check that it does solve your issue ?

[mikeshulman]

I don't think this solves it for me. I removed the merlin from opam, cloned voodoos/fix-1913 and compiled and installed it as instructed in the merlin readme, and I am still getting the same errors. Does this change make my example work for you?

[voodoos]

Yes, it seems I don't get the Graph.add failures anymore with my test script.

[mikeshulman]

Ah, I had to also remove the opam merlin-lib and install that from the updated version (that wasn't clear to me from the compiling installation instructions in the readme, which only say to dune install -p dot-merlin-reader,merlin). When I do that, indeed the Graph.add errors go away. I do still get a different error though:

let: unknown answer: "exception":"Invalid_argument(\"Types.Transient_expr.set_scope 134217728\")
Raised at Stdlib.invalid_arg in file \"stdlib.ml\", line 30, characters 20-45
Called from Ocaml_typing__Types.Transient_expr.set_scope in file \"src/ocaml/typing/types.ml\", line 642, characters 6-72
Called from Stdlib__List.iter in file \"list.ml\", line 112, characters 12-15
Called from Ocaml_typing__Types.backtrack in file \"src/ocaml/typing/types.ml\", line 937, characters 6-35
Called from Ocaml_typing__Btype.backtrack in file \"src/ocaml/typing/btype.ml\" (inlined), line 640, characters 16-41
Called from Merlin_kernel__Mtyper.type_implementation in file \"src/kernel/mtyper.ml\", line 168, characters 2-23
Called from Merlin_kernel__Mtyper.run.(fun) in file \"src/kernel/mtyper.ml\", line 237, characters 35-78
Called from Merlin_utils__Misc.try_finally in file \"src/utils/misc.ml\", line 45, characters 8-15
Re-raised at Merlin_utils__Misc.try_finally in file \"src/utils/misc.ml\", line 62, characters 10-24
Called from Merlin_kernel__Mpipeline.process in file \"src/kernel/mpipeline.ml\", line 308, characters 22-49
Called from CamlinternalLazy.do_force_block in file \"camlinternalLazy.ml\", line 49, characters 17-27
Re-raised at CamlinternalLazy.do_force_block in file \"camlinternalLazy.ml\", line 56, characters 4-11
Called from CamlinternalLazy.force_lazy_block in file \"camlinternalLazy.ml\" (inlined), line 78, characters 27-67
Called from Merlin_kernel__Mpipeline.timed_lazy in file \"src/kernel/mpipeline.ml\", line 17, characters 11-23
Re-raised at Merlin_kernel__Mpipeline.timed_lazy in file \"src/kernel/mpipeline.ml\", line 23, characters 7-22
Called from CamlinternalLazy.do_force_block in file \"camlinternalLazy.ml\", line 49, characters 17-27
Re-raised at CamlinternalLazy.do_force_block in file \"camlinternalLazy.ml\", line 56, characters 4-11
Called from Merlin_kernel__Mpipeline.typer_result in file \"src/kernel/mpipeline.ml\" (inlined), line 133, characters 21-30
Called from Query_commands.dispatch in file \"src/frontend/query_commands.ml\", line 653, characters 16-47
Called from Merlin_commands__New_commands.run in file \"src/commands/new_commands.ml\", line 98, characters 15-53
Called from Merlin_utils__Std.let_ref in file \"src/utils/std.ml\", line 739, characters 8-12
Re-raised at Merlin_utils__Std.let_ref in file \"src/utils/std.ml\", line 745, characters 4-13
Called from Merlin_utils__Misc.try_finally in file \"src/utils/misc.ml\", line 45, characters 8-15
Re-raised at Merlin_utils__Misc.try_finally in file \"src/utils/misc.ml\", line 62, characters 10-24
Called from Stdlib__Fun.protect in file \"fun.ml\", line 34, characters 8-15
Re-raised at Stdlib__Fun.protect in file \"fun.ml\", line 39, characters 6-52
Called from Merlin_kernel__Mocaml.with_state in file \"src/kernel/mocaml.ml\", line 18, characters 8-38
Re-raised at Merlin_kernel__Mocaml.with_state in file \"src/kernel/mocaml.ml\", line 24, characters 4-15
Called from Dune__exe__New_merlin.run.(fun) in file \"src/frontend/ocamlmerlin/new/new_merlin.ml\", lines 118-119, characters 16-52

but maybe that should be reported as a new issue?

[voodoos]

This one is indeed different. Could you try with my latest commit (b53e767) ? (It's probably only masking another, deeper, issue.)

[mikeshulman]

That does make the error go away!

[voodoos]

@mikeshulman it actually turns out that this is an issue that was fixed recently on the compiler ocaml/ocaml#13808, but we usually don't upgrade Merlin until a point release is made.

I applied the upstream patch to replace my workaround, would you mind testing one last time ?

Thanks a lot for your help !

[mikeshulman]

Yes, that also works! Thank you for fixing it.