yojson
yojson copied to clipboard
Remove Tuple & Variant
It has been long time on my mind but now that Yojson 2.x is finally out, we can start looking at the next step, Yojson 3.0 (which I assume should be a fairly minor issue issue, given most consumers probably don't ever deal with `Tuple or `Variant).
The code itself is pretty self-explanatory, it mostly eliminates the TUPLE and VARIANT CPPO variables and then deletes some support code.
There might be more code to be deleted, like start_any_variant/start_any_tuple. I assume these might be used by atd (need to look into it) so I just removed the part that parses non-standard syntax but maybe it can be removed altogether, along with the unit test that covers it.
Closes #104
A bit of research has shown that atdgen generates both start_any_variant with the `Edgy_bracket case as well as start_any_tuple, so the reading code there would most likely need to be adapted.
Is this going to kill ppx_deriving_yojson ? I think it targets the safe fragment so it might expect Tuple and Variant as inputs…
@c-cube I don't think it will break ppx_deriving_yojson too bad, as it encodes tuples as lists and variants as lists with the name of the constructor as first argument, if I remember correctly. Here's the code: https://github.com/ocaml-ppx/ppx_deriving_yojson/blob/61f2d63138ec1efea0a3fb4afa682f40497ec380/src/ppx_deriving_yojson.ml#L115-L148
The biggest (only?) issue will probably be Atd which uses Tuple and Variant, but I believe it would make sense in that case to adopt the same encoding strategy that ppx_deriving_yojson uses. Which has the advantage that the JSON it generates is standard-compliant and thus more interoperable with non-OCaml tools.
The biggest (only?) issue will probably be Atd which uses Tuple and Variant
I don't think it's a big issue, and it's a good move forward.
Atdgen will make its -j-std option the default, and the old default behavior that was supporting the experimental syntax for tuples and variants will be removed.
To see the dependencies on Yojson functions, I used this:
~/atd/atdgen/src $ git grep -oP 'Yojson.[^" ]+' | sort | uniq -c
2 ocaml.ml:Yojson.Safe.t
1 oj_emit.ml:Yojson.End_of_object
5 oj_emit.ml:Yojson.End_of_tuple
1 oj_emit.ml:Yojson.Safe.finish_string
1 oj_emit.ml:Yojson.Safe.init_lexer
2 oj_emit.ml:Yojson.Safe.lexer_state
1 oj_emit.ml:Yojson.Safe.map_ident
1 oj_emit.ml:Yojson.Safe.read_comma
2 oj_emit.ml:Yojson.Safe.read_gt
1 oj_emit.ml:Yojson.Safe.read_ident
1 oj_emit.ml:Yojson.Safe.read_lcurl
2 oj_emit.ml:Yojson.Safe.read_null_if_possible
1 oj_emit.ml:Yojson.Safe.read_object_end
1 oj_emit.ml:Yojson.Safe.read_object_sep
1 oj_emit.ml:Yojson.Safe.read_rbr
16 oj_emit.ml:Yojson.Safe.read_space
1 oj_emit.ml:Yojson.Safe.read_tuple_end2
4 oj_emit.ml:Yojson.Safe.read_tuple_sep2
2 oj_emit.ml:Yojson.Safe.skip_json
1 oj_emit.ml:Yojson.Safe.start_any_tuple
1 oj_emit.ml:Yojson.Safe.start_any_variant
1 oj_emit.ml:Yojson.Safe.to_string
1 oj_emit.ml:Yojson.Safe.write_bool
1 oj_emit.ml:Yojson.Safe.write_float
1 oj_emit.ml:Yojson.Safe.write_float_prec
1 oj_emit.ml:Yojson.Safe.write_int
1 oj_emit.ml:Yojson.Safe.write_json
1 oj_emit.ml:Yojson.Safe.write_null
1 oj_emit.ml:Yojson.Safe.write_std_float
1 oj_emit.ml:Yojson.Safe.write_std_float_prec
1 oj_emit.ml:Yojson.Safe.write_string
1 omelange_emit.ml:Yojson.Safe.to_string
Occurrences such as Yojson.End_of_tuple, Yojson.Safe.read_tuple_end2, and a few others will have to be removed.
See https://github.com/ahrefs/atd/issues/412
@mjambon That's a great list, will be very handy!
I've been looking at the std variants and with the removal of tuple and variant the only non-standard thing seems to be the printing of floats (NaN, +Infinity, -Infinity). I see that atdgen is currently emitting both write_float(_prec) and write_std_float(_prec). Looking at atdgen it seems to be configurable (p.std) which variant it will use.
I'm thinking that as we're breaking compatibility anyway to become more JSON-compliant, might be sensible to bite the bullet and make the std variants the only option, thus failing to encode unrepresentable floats in JSON. WDYT?
This has broken some dependents of yojson: https://github.com/tmcgilchrist/ocaml-gitlab/issues/101