reason

reason copied to clipboard

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:

1. All the swapping works more or less the same - no lexing changes necessary.
2. 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?

@anmonteiro I think so, at least for infix operators.

@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.

The PR isn't done yet, and I haven't found the time to squeeze it in.

A nasty one I just come across is the (//) operator used in Fpath - it might need special treatment on top of this :(