Conversation
- set up productions in reverse order of precedence - split productions that contain operators of different precedence - join productions that have operators of similar precedence - use tokens instead of strings - add test case
Codecov Report
@@ Coverage Diff @@
## master #671 +/- ##
============================================
+ Coverage 71.11% 71.29% +0.18%
- Complexity 2773 2786 +13
============================================
Files 106 106
Lines 9069 9062 -7
Branches 1778 1765 -13
============================================
+ Hits 6449 6461 +12
+ Misses 2205 2194 -11
+ Partials 415 407 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
nickl-
left a comment
There was a problem hiding this comment.
I was also thinking the easiest way to resolve this is to build the hierarchy in the grammar. This does not mean that there are no other way to deal with this.
The comment will not be useful 10 years from now.
| * will do is rearrange them in to a different order. The java code | ||
| * will be compiled, jitted and inlined by c2 so you wont even know | ||
| * what the performance impact is unless you do micro benchmarks. | ||
| */ |
There was a problem hiding this comment.
This comment should only shed light on what the code does, if it is unclear otherwise. It should not be the motivation for the change, that is what the PR and commit messages are for.
|
Also have you considered addressing the other outstanding parser issues while at this? |
|
Please also add unit test for: true && false ? true : true && false ? true : false;Just to be sure. |
Revise comment as requested by reviewer
done |
Are there still items that need to be resolved on this issue? I think at this point you will have to put your own code forward. |
I need to train my editor to do this for me.
Looks good, RTBC |
|
This is RTBC - ready to be committed (merged) Are you waiting for me? |
Fix the problem with operator precedence (beanshell#590)
This should fix the problem with 1+2*3=9 as noted in #590 and probably elsewhere. Simply reverting the patch that broke operator precedence wasn't acceptable, and with no one else stepping up I went ahead and redid this. So basically this RP is almost the same as before but fixes mod/power precedence. I retained the reduction of productions in the unary expression and elimination of one lookahead from the cast expression. Tokens are used instead of strings because a one point this was the strong recommendation for better performance from the javacc documentation. The test case is a work in progress.