Can't go-to-definition in generated file

[Timmmm]

Hi, I am very new to OCaml but I've been working on the Sail compiler which is written in OCaml using VSCode and ocaml-lsp. Generally the LSP works very well, however there is an issue with the generated file _build/default/src/lib/ast.ml. Ocaml-lsp seems to know at least something about the types in it, because you can hover them and it shows their types. For example in src/lib/ast_util.ml on this line:

  | Nexp_if (i1, t1, e1), Nexp_if (i2, t2, e2) ->

Nexp_if is defined in _build/default/src/lib/ast.ml. If I hover i1 it says the type is n_constraint which is correct! However for some reason I cannot ctrl-click / go-to-definition on Nexp_if.

If I run

dune ocaml merlin dump-config src/lib

Then it doesn't have (S .../_build/default/src/lib) anywhere, though it does have (B .../_build/default/src/lib/.libsail.objs/byte).

Any idea why this isn't working or how to make it work?

[voodoos]

Hi @Timmmm , thanks for your report !

This is a long-standing issue: Merlin is told by Dune where to find sources and build files. In a dune project the sources are copied inside the _build directory but these are indeed not sources Merlin should consider (we don't want to jump inside the _build directory, since files here should not be modified by the user). So the current configuration is actually correct.

However, it is frequent that users are interested in generated files, so the current behavior is not satisfying. There is a simple workaround however: you can set the generated file to be promoted to the source files. This will make it visible to Merlin.

For example, you can have the output of a rule promoted (but removed by dune clean) with: (mode (promote (until-clean)))

(more detail in the documentation)

[nojb]

these are indeed not sources Merlin should consider (we don't want to jump inside the _build directory, since files here should not be modified by the user). So the current configuration is actually correct.

The files in the _build directory are actually set to be read-only by Dune if I remember correctly. Being able to jump to those files woud still be useful. What would be needed is maybe some visual help at the editor level to warn the user that the file in question is generated.

[Timmmm]

There is a simple workaround however: you can set the generated file to be promoted to the source files.

If I'm understanding this correctly, that will mean the generated file is in src instead of _build? And then I'd have to add it to .gitignore? Bit odd but I suppose that might work. I'll try it.

we don't want to jump inside the _build directory, since files here should not be modified by the user

Yeah that doesn't really follow - as you say my purpose for jumping to the file is not to modify it. I'll try promote anyway.

[Timmmm]

Hmm that didn't seem to work. I added (mode (promote... as follows:

(rule
 (target ast.ml)
 (deps
  (:ast ast.lem))
 (action
  (progn
   (run lem -ocaml %{ast})))
 (mode (promote (until-clean))))   <-- added this line

But now it just doesn't even try to run lem. Plus there are a lot of files that only exist in the build directory. I really think it would be better if ocaml-lsp/Merlin behaved like other LSP servers and just jumped to the file if they know where it is. That's clearly better than refusing to do so because the user might try to edit it (which as @nojb isn't too much of a concern anyway because the files are read-only; you get an error as soon as you try to save it).

[Timmmm]

Ah I eventually found that it works for dune build but not dune build --release! That seems like a very strange design. Also I can't use dune build because it seems like it sets warnings-as-errors (sensible) and Sail has some warnings.

[Timmmm]

Ok so dune build builds src/lib/ast.ml and then fails with some warnings. If I then do dune build --release the build completes and I have src/lib/ast.ml.

However I don't think this is a good solution. Dune doesn't mark src/lib/ast.ml as read-only and it's in the src/ directory, which means users are far more likely to accidentally edit this file than one in _build. If that's the reason for this then I don't understand the logic.

[voodoos]

As I said, I don't think the current situation is satisfying.

• One option would be for Dune to tell all _build directories with sources but we probably want a new directive to prevent Merlin from incorrectly jumping to the _build dir.
• Another option would be for Dune to provide a more granular information, such as the locations of generated sources, copied files etc.

Until someone tackles that issue, I think the workaround is the best option. I didn't know that the release profile skipped the promotion, maybe it's only for "until-clean" promotions, maybe you can try removing that.

Alternatively, you could configure your flag selection per library or globally: https://dune.readthedocs.io/en/stable/concepts/ocaml-flags.html

This way you would be able to build in dev mode.

[Timmmm]

we probably want a new directive to prevent Merlin from incorrectly jumping to the _build dir.

I still don't quite understand this. Why is it incorrect to jump to the _build dir? Every other go-to-definition system I've used is quite happy to do so.

I didn't know that the release profile skipped the promotion, maybe it's only for "until-clean" promotions, maybe you can try removing that.

Unfortunately that made no difference. I did fix the warnings in dune build so I can now just use that for development at least, but we can't have promote in the code permanently because it breaks dune build --release. With --release it's not just that it doesn't do the promotion; it doesn't run the rule at all so you get compilation errors. Maybe I'll open a bug in the Dune repo...

[voodoos]

Sorry, I edited your message instead of answering it 🤦

I still don't quite understand this. Why is it incorrect to jump to the _build dir? Every other go-to-definition system I've used is quite happy to do so.

We must be careful not to jump to a copy of a source file in the _build that actually comes from the source directories. Merlin's current configuration makes no guarantee on which folder will be searched first. Reviewing how this process works in Merlin and adding build dirs at the end of the source path might be enough if someone want to give it a try.

Regardless of the solution that is chosen, this feature requires changing the configuration Dune sends to Merlin.

Unfortunately that made no difference. I did fix the warnings in dune build so I can now just use that for development at least, but we can't have promote in the code permanently because it breaks dune build --release. With --release it's not just that it doesn't do the promotion; it doesn't run the rule at all so you get compilation errors. Maybe I'll open a bug in the Dune repo...

That's most surprising, I think you should open an issue on Dune, if it is expected behavior they will at least be able to explain it 🙂