Support type parameters and classes as expression in the vm.

Support type parameters and classes as expression in the vm.
Turn many compile-time errors into runtime errors per latest spec.
Triage affected tests.

R=hausner@google.com

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

[regis, 2013-07-08 17:09:40]

I may still have to adjust status files for dart2js and friends before
submitting.

Thanks,
Regis

[hausner, 2013-07-08 19:51:15]

LGTM with comments.

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

https://codereview.chromium.org/18801007/diff/1/runtime/vm/parser.cc#oldcode7375
runtime/vm/parser.cc:7375: if (!IsAssignableExpr(expr)) {
It looks like you eliminated all uses of this function so it should be removed.
It does handle cases that don't seem to be handled in CreateAssignmentNode,
though, eg. setter super calls.

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

https://codereview.chromium.org/18801007/diff/1/runtime/vm/parser.cc#newcode7201
runtime/vm/parser.cc:7201: ErrorMsg(left_pos, "expression is not assignable");
By moving the compiler error message here, you unfortunately have to make the
message quite vague. I liked the old "left hand side .... not assignable" a bit
better.

https://codereview.chromium.org/18801007/diff/1/runtime/vm/parser.cc#newcode7281
runtime/vm/parser.cc:7281: Token::IsIdentifier(CurrentToken()) ?
CurrentLiteral() : NULL;
It seems to me that in this expression:

o.f = expr;

your expr_ident is o. If the left hand side turns out not to be assignable, the
the noSuchMethodError that is thrown reports that there is no "o" instead of
"f".

[regis, 2013-07-08 23:04:15]

Thanks!

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

https://codereview.chromium.org/18801007/diff/1/runtime/vm/parser.cc#oldcode7375
runtime/vm/parser.cc:7375: if (!IsAssignableExpr(expr)) {
On 2013/07/08 19:51:15, hausner wrote:
> It looks like you eliminated all uses of this function so it should be
removed.
> It does handle cases that don't seem to be handled in CreateAssignmentNode,
> though, eg. setter super calls.

Thanks. Removed.
Shouldn't some setter super call test fail if I removed too much?

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

https://codereview.chromium.org/18801007/diff/1/runtime/vm/parser.cc#newcode7201
runtime/vm/parser.cc:7201: ErrorMsg(left_pos, "expression is not assignable");
On 2013/07/08 19:51:15, hausner wrote:
> By moving the compiler error message here, you unfortunately have to make the
> message quite vague. I liked the old "left hand side .... not assignable" a
bit
> better.

True. Note that many compile time errors are actually gone.

https://codereview.chromium.org/18801007/diff/1/runtime/vm/parser.cc#newcode7281
runtime/vm/parser.cc:7281: Token::IsIdentifier(CurrentToken()) ?
CurrentLiteral() : NULL;
On 2013/07/08 19:51:15, hausner wrote:
> It seems to me that in this expression:
> 
> o.f = expr;
> 
> your expr_ident is o. If the left hand side turns out not to be assignable,
the
> the noSuchMethodError that is thrown reports that there is no "o" instead of
> "f".

The name expr_ident is only used for LiteralNode, LoadLocalNode, and
LoadStaticFieldNode. See CreateAssignmentNode.

Do you have an example of wrong output?

[hausner, 2013-07-08 23:38:07]

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

https://codereview.chromium.org/18801007/diff/1/runtime/vm/parser.cc#oldcode7375
runtime/vm/parser.cc:7375: if (!IsAssignableExpr(expr)) {
I don't know whether we have a test for this. I would think so.

On 2013/07/08 23:04:15, regis wrote:
> On 2013/07/08 19:51:15, hausner wrote:
> > It looks like you eliminated all uses of this function so it should be
> removed.
> > It does handle cases that don't seem to be handled in CreateAssignmentNode,
> > though, eg. setter super calls.
> 
> Thanks. Removed.
> Shouldn't some setter super call test fail if I removed too much?

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

https://codereview.chromium.org/18801007/diff/1/runtime/vm/parser.cc#newcode7281
runtime/vm/parser.cc:7281: Token::IsIdentifier(CurrentToken()) ?
CurrentLiteral() : NULL;
No, I did not run your code. I just don't understand where and how the case I
mentioned is handled now.

On 2013/07/08 23:04:15, regis wrote:
> On 2013/07/08 19:51:15, hausner wrote:
> > It seems to me that in this expression:
> > 
> > o.f = expr;
> > 
> > your expr_ident is o. If the left hand side turns out not to be assignable,
> the
> > the noSuchMethodError that is thrown reports that there is no "o" instead of
> > "f".
> 
> The name expr_ident is only used for LiteralNode, LoadLocalNode, and
> LoadStaticFieldNode. See CreateAssignmentNode.
> 
> Do you have an example of wrong output?

[regis, 2013-07-09 00:13:54]

Matthias, let's chat in person tomorrow.
I still have a bunch of dart2dart (now passing) tests to triage before being
able to submit.

Thanks,
Regis

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

https://codereview.chromium.org/18801007/diff/1/runtime/vm/parser.cc#newcode7281
runtime/vm/parser.cc:7281: Token::IsIdentifier(CurrentToken()) ?
CurrentLiteral() : NULL;
On 2013/07/08 23:38:07, hausner wrote:
> No, I did not run your code. I just don't understand where and how the case I
> mentioned is handled now.

It all depends on the nature of o and f in your case. There are many cases I did
not touch (e.g. no instance field f in an instance o).

> 
> On 2013/07/08 23:04:15, regis wrote:
> > On 2013/07/08 19:51:15, hausner wrote:
> > > It seems to me that in this expression:
> > > 
> > > o.f = expr;
> > > 
> > > your expr_ident is o. If the left hand side turns out not to be
assignable,
> > the
> > > the noSuchMethodError that is thrown reports that there is no "o" instead
of
> > > "f".
> > 
> > The name expr_ident is only used for LiteralNode, LoadLocalNode, and
> > LoadStaticFieldNode. See CreateAssignmentNode.
> > 
> > Do you have an example of wrong output?
>

[regis, 2013-07-09 19:36:08]

Committed patchset #3 manually as r24861 (presubmit successful).