janestreet/ppx_sexp_conv

`[%sexp_of: t]` expands to code that does not use `t` as a type

Opened this issue · 3 comments

There is an irregularity between %sexp_of and e.g. %compare. Consider:

  let compare (type k v) compare_k compare_v = [%compare: (k * v) list]
  let sexp_of_t (type k v) sexp_of_k sexp_of_v = [%sexp_of: (k * v) list]

This triggers

Warning 34 [unused-type-declaration]: unused type k.

(and the same for v).

On the other hand, omitting the type parameters:

  let compare compare_k compare_v = [%compare: (k * v) list]
  let sexp_of_t sexp_of_k sexp_of_v = [%sexp_of: (k * v) list]

triggers

Error: Unbound type constructor k

on the definition of compare, while sexp_of_t is ok.

This is indeed desirable, and in fact we attempted this change at some point.
However, the change ended up too massive and too disruptive because of how often people abuse this "feature" to derive sexp conversions for types that don't exist, so we gave up.

Perhaps some day we'll try again, if we can come up with a way to make the change incrementally, but I'm afraid that day is not today.

Thanks for the context! This is indeed very tempting to abuse, so not so surprising. I wonder if a way forward would be to add support to ppx_sexp to accept a cookie so that if e.g. --cookie 'ppx_sexp_strict="1"' is passed then the strict interpretation is used. Then projects could add a cookies clause to their dune files to indicate that they conform to the stricter interpretation, and maybe one day the default could be switched.

Yeah, that sounds like a workable approach.