fix: refresh merlin configs after dune build

[rgrinberg]

When dune finishes a build, merlin configuration might not be correct and it's useful to reload it and give users fresh diagnostics. Of course, this requires us throwing away some diagnostic work that isn't necessarily stale, but it's better to be slow than give false positives to the user.

[ulugbekna]

It seems like we're "set"ting the new diagnostics for merlin docs but don't send them? If I add Diagnostics.send diagnostics All to progress_loop, the new diagnostics get sent and user sees updated diagnostics in their files automatically.

That's very cool! I was thinking of this feature, but I imagined we need to update diagnostics only for reverse deps of the module that's being edited, but your solution seems good.

[rgrinberg]

It seems like we're "set"ting the new diagnostics for merlin docs but don't send them? If I add Diagnostics.send diagnostics All to progress_loop, the new diagnostics get sent and user sees updated diagnostics in their files automatically.

Thanks. This is now fixed.

That's very cool! I was thinking of this feature, but I imagined we need to update diagnostics only for reverse deps of the module that's being edited, but your solution seems good.

It turns this is problematic to implement with dune currently. One needs to keep a set of the dependency closure for every module and keep it updated. It's not trivial but if this solution isn't good enough, we'll have to go this way.

[ulugbekna]

If the file being edited is an implementation file and given its interface file exists, would it make sense that we only update diagnostics for its interface file if it's open? But if the impl file being edited doesn't have an interface file, then we update diagnostics to all files.

[rgrinberg]

I'm not sure I understand the optimization. Can you give an example?

[ulugbekna]

I meant to say that, to my understanding, OCaml can avoid compiling reverse dependencies of a compilation unit, if the unit's interface hasn't changed. Consequently, if we could determine that the re-build was triggered by edits to an implementation file, we could only update diagnostics for its interface file and not for other files, which would not anyway have any changes in their diagnostics from merlin because they depend on the unchanged interface file. For example, if there's foo.ml that depends on bar.mli, then edits to bar.ml should only update diagnostics for bar.mli but not foo.ml because nothing changed from foo.ml's perspective.

It's not clear, however, how to determine whether the re-build was triggered by changes to an implementation file. We could

• either rely on dune to tell us what triggered the re-build
• or see what files were edited (olsp received file-edit notifications for) between two builds of a single dune instance (in this case, however, we assume that olsp receives notifications for all file changes, which may not be the case in case of, e.g., dune promotions)

Does this help?

[rgrinberg]

Okay, I understand. Note that we don't need to worry about any of these details since they're all reflected in the dependency graph dune provides us with. We just need a better way to work with it.