Fix assertion in the parser about operator precedence.

Fix assertion in the parser about operator precedence.

R=kmillikin@google.com

Committed: https://code.google.com/p/dart/source/detail?r=27625

[Florian Schneider, 2013-09-18 15:09:08]

@Lasse: Please run the VM debug tests before landing next time.

https://codereview.chromium.org/24078019/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/24078019/diff/1/runtime/vm/parser.cc#newcode1890
runtime/vm/parser.cc:1890: (CurrentToken() == Token::kNE));
Could the ASSERT below even be removed since this assertion already asserts
valid super-operators?

[Kevin Millikin (Google), 2013-09-18 15:13:21]

https://codereview.chromium.org/24078019/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/24078019/diff/1/runtime/vm/parser.cc#newcode1900
runtime/vm/parser.cc:1900: ASSERT(Token::Precedence(op) >=
Token::Precedence(Token::kEQ));
This is OK to fix the tests.

It's kind of annoying to assert anything about precedence or to rely
specifically on a particular operator's precedence.

I probably find that annoying because overriding operators has basically nothing
at all to do with precedence.

[Kevin Millikin (Google), 2013-09-18 15:13:27]

lgtm

[Florian Schneider, 2013-09-18 15:17:04]

Committed patchset #1 manually as r27625 (presubmit successful).

[hausner, 2013-09-18 15:52:36]

lgtm

https://codereview.chromium.org/24078019/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/24078019/diff/1/runtime/vm/parser.cc#newcode1890
runtime/vm/parser.cc:1890: (CurrentToken() == Token::kNE));
On 2013/09/18 15:09:09, Florian Schneider wrote:
> Could the ASSERT below even be removed since this assertion already asserts
> valid super-operators?

If somebody decides tomorrow that the conditional operator should be
overloadable then this code breaks. That would be caught by the assertion below.

https://codereview.chromium.org/24078019/diff/1/runtime/vm/parser.cc#newcode1900
runtime/vm/parser.cc:1900: ASSERT(Token::Precedence(op) >=
Token::Precedence(Token::kEQ));
We are blindly calling ParseBinaryExpr() here, so we better make sure that the
operator can be handled by that function. That's what this assert is about.
Maybe you find it a bit less annoying now.


On 2013/09/18 15:13:21, kmillikin wrote:
> This is OK to fix the tests.
> 
> It's kind of annoying to assert anything about precedence or to rely
> specifically on a particular operator's precedence.
> 
> I probably find that annoying because overriding operators has basically
nothing
> at all to do with precedence.