ocaml / merlin

Context sensitive completion for OCaml in Vim and Emacs
https://ocaml.github.io/merlin/
MIT License
1.57k stars 233 forks source link

Issue with "<:sexp_of< >>" #131

Closed roshanjames closed 10 years ago

roshanjames commented 10 years ago
      let printer x = Sexp.to_string (<:sexp_of< Jane.t t >>x) in

For the above snippet I get the following error/warning from merlin: "Warning 20: this argument will not be used by the function."

I believe its because does not understand the "<:sexp_of< >>" syntax extension correctly.

trefis commented 10 years ago

Actually, when the parser encounters a Camlp4 quotation (i.e. <: ... >) it generates a "dummy node" (equivalent to what you'd have with Obj.magic ()). So we indeed have no informations on that chunk of the code later on, but we feel that hat kind of construction doesn't appear often enough to be a real problem.

I will let @def-lkb explain in more details (either here or on #133 ) why we don't want to deal with camlp4.

roshanjames commented 10 years ago

FWIW, it is pretty common in our code base. Esp to do with "sexp_of" like things.

trefis commented 10 years ago

Hopefully it will disappear with 4.02 :)

roshanjames commented 10 years ago

A few comments: (1) Two more cases of the same. There is also:

a) <:test_eq<>> b) <:test_result<>>

(2) If you infer the type of <: ... > snippets to be a fresh ['a], I expect that the type checker should be more forgiving about the error that I originally posted. I expect the type checker should subsequently unify the ['a] with a function type and not complain that [x] will be unused.

(3) It is true that ppx might make this problem obsolete. But realistically we have to live with the current macros for the next few years. Certainly till Jan 2015 :).

Finally, can you keep then issue open while we are still discussing it.

roshanjames commented 10 years ago

Any updates about this?

trefis commented 10 years ago

If you infer the type of <: ... > snippets to be a fresh ['a], I expect that the type checker should be more forgiving about the error that I originally posted. I expect the type checker should subsequently unify the ['a] with a function type and not complain that [x] will be unused.

Indeed; I misread your initial snippet. I'll try and have a look at that tomorrow.

It is true that ppx might make this problem obsolete. But realistically we have to live with the current macros for the next few years.

I find the current approach "OK". We have no semantic information about what's inside the quotation (so: no error reporting), but I think that's ok. My intuition is that their scope is very limited (i.e. they should be really small pieces of code), and that "the compiler" will tell you about the errors merlin fails to mention.

roshanjames commented 10 years ago

You could approach this problem as I suggested in #133, but I understand that the approach there requires work to get into a reliable form. Another more immediate fix is to simply support a small set of these syntaxes which can be enabled by an appropriate EXT. Currently there are three of these that know off and are mentioned in this thread.

It would be valuable to have very reliable merlin errors and its worth pushing to see if we can achieve this. FWIW, I think we have made really good progress in this direction already.

trefis commented 10 years ago

On the specific problem of the warning 20, I believe it is fixed since 7aa603f (which preceeds the release of merlin 1.5).