-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fix parenthesization for binary operators #189
Fix parenthesization for binary operators #189
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick fix!
Only thing is that there's already shared/src/test/diff/codegen/Parentheses.mls
, so better move your test cases there.
Also, let's add this one:
fun (--) minusminus(a, b) = a - b
//│ fun (--) minusminus: (Int, Int) -> Int
:js
1 -- (3 -- 5)
//│ Int
//│ // Prelude
//│ class TypingUnit3 {}
//│ const typing_unit3 = new TypingUnit3;
//│ // Query 1
//│ res = minusminus(1, minusminus(3, 5));
//│ // End of generated code
//│ res
//│ = 3
It doesn't have a problem because custom operators are desugared to plain methods. It was not clear to me that this is why your condition JSBinary.operators.contains(op)
is sufficient in JSBackend.scala
. Better add a comment to that effect there!
Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>
fun (-+-) complex(a, b) = a - 2*b | ||
//│ fun (-+-) complex: (Int, Int) -> Int | ||
|
||
:p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test is not clean. Please remove this and finally get this PR merged...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, I did it myself.
A small modification that shouldn't impact other code, fixes part of #65