search

janestreet / sexplib

Automated S-expression conversion
MIT License
147 stars 27 forks source link

Improve error reporting for incorrect sum tags. #23

Open ejgallego opened 8 years ago

ejgallego commented 8 years ago

Dear sexplib developers,

I am using sexplib for interactive use, and it would be great to improve the error messages for incorrect sum types to include the alternative tags.

A possibility is to extend the function conv_error.ml:unexpected_stag to include an optional list of valid tags. Once that is done ppx_sexp_conv could provide this information.

Do you think this could make sense? I can provide a patch (in the public domain) if you think that would make a useful improvement.

Best regards, E.

yminsky commented 8 years ago

That seems like a nice idea. The current error messages are indeed too obscure on this point.

ejgallego commented 8 years ago

Dear Yaron, thanks for the comments, I'll try to provide a patch!

yminsky commented 8 years ago

It's worth noting that for us to accept a patch, you need to sign the CLA: https://blogs.janestreet.com/core-icla-1_2.pdf

ejgallego commented 8 years ago

Noted, thanks!

aalekseyev commented 4 years ago

This issue is as relevant now as ever. We recently revisited it, and it was mentioned that: 1) This feature has a cost: it increases the code size that comes with producing a nice error message. 2) It has an overlap with sexp_grammar. If sexp grammar is available, then there should be enough information in that to construct a good error message from that without having to keep an extra (third) copy of the list of constructors in the binary.

So the plan is to wait until sexp_grammar is more mature and widespread and then change the error messages to use that.