reason
reason copied to clipboard
reasonml
Missing `=>` infix operator escape
This PR applies the same solution that we use for /\* and makes =\> work.
Because we reuse the solution that is already in place, this is still parsed as => in the AST and printed as => in the OCaml side.
fixes #1941
Great improvement, @anmonteiro! One alternative approach would be to swap -> and =>. Thoughts?
One downside to swapping is that it breaks with how other conflicts are handled between syntaxes.
@hcarty what do you mean?
edit: sorry I hadn't seen @jordwalke's comment.
@jordwalke I personally don't mind it, here are my thoughts on the tradeoffs:
-
=\>is uglier (subjective) but feels natural given/\*for example -
=\>could probably just be listed in this comparison table without any further explanation, whereas->would maybe need its own little explanation paragraph- Granted, this is an edge case that wouldn't come up to often, but I'm generally in favor of what feels more natural.
Thoughts?
It might be nice to save -> for Reason's core syntax later on. It's not a valid infix operator currently.
The previously cases of swapping for keywords caused confusion before. Infix operators are not likely to be as confusing when swapped - that's already done for string concatenation for example - but it's still nice to have conversion rules be consistent. Is => used often enough to be worth adding an exception?
@jordwalke do you agree with the above? should we go ahead and keep =\> then?
My original idea of swapping -> and => didn't take into account that people will likely want to implement custom behavior for -> via ppx. So for that reason, we wouldn't want -> to be represented by a super common infix operator like =>(in ocaml). Instead parsing -> into the more obscure |. identifier makes it a better candidate for fancy / opinionated transforms.
The reasons for using such a complicated escaping convention for * and / had more to do with avoiding lexing ambiguity with comments. Normally, we apply swaps or mappings. What would such a solution look like here?
One approach:
ML's => could be printed in Reason as ==>.
ML's ==> could be printed in Reason as ===>.
ML's ===> could be printed in Reason as ====>.
The downside to that approach is that operators are extra long etc (even ones like ==> which don't have an ambiguity must be written as ===> in Reason).
Another approach is that we can swap the operator => and \=>. Reason parses \=> into the AST as \=>. We could make it so \=> gets "swapped" with => where all the other swapping happens. Benefits:
- All the swapping works more or less the same - no lexing changes necessary.
- The leading backslash could be used in many places to represent these compatibility parsings. You never have to question how to escape an operator because it's always the first character.
@jordwalke I really like your 2nd idea. I'll try to prototype something in this PR over the next couple of days.
I think you said in Discord that backslash prefixed operators are not valid in OCaml, which would allow Reason to fix all these compatibility parsings in one go.
A user pointed out that ++ is not currently handled properly when converting from OCaml to Reason (see https://reasonml.chat/t/access-conflicting-infix-operator/805). Could that be captured in this fix as well?
@hcarty the idea I have in mind (and I think it's what Jordan implied) is that \ could be a prefix for all such transformations between OCaml and Reason. Therefore, we'd make it work using \++.
That solves the problem, right?
@jordwalke I think this is ready for another look.
I implemented your suggestion by having any operator preceded with a backslash (\) be the same operator on the OCaml side without the backslash.
This enables things like:
let (\++) = ...
let (\=>) = ...
The way I made it generic for all backslash-prefixed operators is by still having e.g. \=> in the AST but printing to OCaml as =>. This way we don't need to enumerate all the possible operators that need swapping.
The downside of this approach is that converting OCaml code that has e.g. => to Reason will look like let (=>) = ... which is invalid Reason code (but I think that's fine?).
This seems like a nice simplification, with the downside that keeping the \-prefix form in the AST makes conversion to/from OCaml syntax harder.
@hcarty The problem is only about OCaml -> Reason, not the other way around.
Some examples:
# Reason -> Reason
$ echo 'let (\=>) = (a, b) => a + b' | ./_build/install/default/bin/refmt
let (\=>) = (a, b) => a + b;
$ echo 'let (\++) = (a, b) => a + b' | ./_build/install/default/bin/refmt
let (\++) = (a, b) => a + b;
# Reason -> OCaml
$ echo 'let (\=>) = (a, b) => a + b' | ./_build/install/default/bin/refmt --print ml
let (=>) a b = a + b
$ echo 'let (\++) = (a, b) => a + b' | ./_build/install/default/bin/refmt --print ml
let (++) a b = a + b
# OCaml -> Reason (where the problem is)
$ echo 'let (=>) a b = a + b' | ./_build/install/default/bin/refmt --parse ml
let (=>) = (a, b) => a + b;
$ echo 'let (++) a b = a + b' | ./_build/install/default/bin/refmt --parse ml
let (++) = (a, b) => a + b;
@anmonteiro Sorry, I was thinking of Reason -> OCaml through ocamlformat since refmt doesn't keep comments. I don't think ocamlformat is perfect in that regard either, to be fair. And ocamlformat's Reason -> OCaml conversion currently has some pretty tight version constraints.
@hcarty If you refmt Reason -> Ocaml with refmt and then use ocamlformat, would that work in your case?
@IwanKaramazow Unfortunately not. Going .re -> refmt -> ocamlformat translates the syntax but last I checked refmt drops comments when outputting OCaml syntax.
Thanks for the work here @anmonteiro! It'd be nice to get this PR approved and merged, since it blocks interoperability with some fairly basic OCaml infix operators, esp. => and libraries that use them.
A nasty one I just come across is the (//) operator used in Fpath - it might need special treatment on top of this :(
