Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the precedence of fast pipe in the presence of function applic… #2078

Merged

Conversation

anmonteiro
Copy link
Member

…ation

@@ -7195,6 +7193,7 @@ let printer = object(self:'self)
makeTup ~uncurried (List.map self#label_x_expression_param params)

method formatFunAppl ~jsxAttrs ~args ~funExpr ~applicationExpr ?(uncurried=false) () =
let funExpr = Reason_ast.processFastPipe funExpr in
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to processFastPipe of funExpr because we only processed applicationExpr in unparseExprRecurse and it may have bailed (e.g. if applicationExpr is an underscore application pattern).

@anmonteiro anmonteiro force-pushed the anmonteiro/fast-pipe-funcall-precedence branch from 357d235 to 513613c Compare July 15, 2018 19:30
@chenglou
Copy link
Member

The be clear: this is a precedence change. It doesn't affect BS side, because x->y(z) was turned into x |. (y z) and then through the ppx, y x z. Now it'll be turned into (x |. y) z which through the ppx becomes (y x) z aka also y x z. Same perf, same type checking (?), same semantics when z is a argument (aka, x->y(~z) still works.

This change is only to accommodate non-bsb-native users who don't get the |. ppx transform (into which -> is desugared, and would still like to use it by redefining it themselves as an infix operator: let (|.) = (a, b) => b(a)*. This won't have the same perf benefits (there might be intermediate functions), doesn't work with labeled arguments and won't type check the same way. But at least it'll work.

* Before this diff, this wasn't possible, since we transform x->y(z) into x |. (y z), whereas they needed (x |. y) z.

cc @bobzhang @IwanKaramazow to confirm!

@chenglou chenglou merged commit cda95d3 into reasonml:master Jul 16, 2018
@anmonteiro anmonteiro deleted the anmonteiro/fast-pipe-funcall-precedence branch July 16, 2018 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants