Made assert a keyword.

Made assert a keyword.

BUG=dartbug.com/5053
TEST=

Kasper deployed this as https://codereview.chromium.org//11113009

[aam-me, 2012-09-30 20:21:24]

Peter, please, review when you have a chance.

Thanks!

[ahe, 2012-10-10 08:44:13]

https://codereview.chromium.org/11014010/diff/1/lib/compiler/implementation/c...
File lib/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11014010/diff/1/lib/compiler/implementation/c...
lib/compiler/implementation/compiler.dart:374: assertMethod =
jsHelperLibrary.find(const SourceString('Assert'));
I think I would prefer calling this "assertHelper". This way we follow the
naming convention that methods start with a lowercase letter.

https://codereview.chromium.org/11014010/diff/1/lib/compiler/implementation/r...
File lib/compiler/implementation/resolver.dart (right):

https://codereview.chromium.org/11014010/diff/1/lib/compiler/implementation/r...
lib/compiler/implementation/resolver.dart:1279: if (node.receiver === null) {
Not sure this check is needed.

https://codereview.chromium.org/11014010/diff/1/lib/compiler/implementation/r...
lib/compiler/implementation/resolver.dart:1283: if
(isExpressionStatementExpression(node) &&
I don't think this check is needed. This must be the assert method. Then I think
you can get rid of isExpressionStatementExpresssion method.

https://codereview.chromium.org/11014010/diff/1/lib/compiler/implementation/s...
File lib/compiler/implementation/scanner/parser.dart (right):

https://codereview.chromium.org/11014010/diff/1/lib/compiler/implementation/s...
lib/compiler/implementation/scanner/parser.dart:2055: token =
parseParenthesizedExpression(token);
Rather than parsing a parenthesized expression here, I would prefer to use
parseArguments. This makes the Send node look like it always did. We can report
an error in the resolver if the number of arguments is incorrect.

[aam-me, 2012-10-11 05:35:25]

Peter, thank you for the comments.
Please take another look when you have a chance!

https://chromiumcodereview.appspot.com/11014010/diff/1/lib/compiler/implement...
File lib/compiler/implementation/compiler.dart (right):

https://chromiumcodereview.appspot.com/11014010/diff/1/lib/compiler/implement...
lib/compiler/implementation/compiler.dart:374: assertMethod =
jsHelperLibrary.find(const SourceString('Assert'));
On 2012/10/10 08:44:13, ahe wrote:
> I think I would prefer calling this "assertHelper". This way we follow the
> naming convention that methods start with a lowercase letter.

Done.

https://chromiumcodereview.appspot.com/11014010/diff/1/lib/compiler/implement...
File lib/compiler/implementation/resolver.dart (right):

https://chromiumcodereview.appspot.com/11014010/diff/1/lib/compiler/implement...
lib/compiler/implementation/resolver.dart:1279: if (node.receiver === null) {
On 2012/10/10 08:44:13, ahe wrote:
> Not sure this check is needed.

If I understand correctly, this check is used to skip complexities of resolving
"a.b(args)" when we have just "a(args)". There are few places in listener where
Send node is created with null receiver.

https://chromiumcodereview.appspot.com/11014010/diff/1/lib/compiler/implement...
File lib/compiler/implementation/scanner/parser.dart (right):

https://chromiumcodereview.appspot.com/11014010/diff/1/lib/compiler/implement...
lib/compiler/implementation/scanner/parser.dart:2055: token =
parseParenthesizedExpression(token);
On 2012/10/10 08:44:13, ahe wrote:
> Rather than parsing a parenthesized expression here, I would prefer to use
> parseArguments. This makes the Send node look like it always did. We can
report
> an error in the resolver if the number of arguments is incorrect.

Done.

[ahe, 2012-10-11 05:44:08]

Just a quick answer. I'll take a closer look later when I'm at the office.

https://chromiumcodereview.appspot.com/11014010/diff/1/lib/compiler/implement...
File lib/compiler/implementation/resolver.dart (right):

https://chromiumcodereview.appspot.com/11014010/diff/1/lib/compiler/implement...
lib/compiler/implementation/resolver.dart:1279: if (node.receiver === null) {
On 2012/10/11 05:35:25, aam wrote:
> On 2012/10/10 08:44:13, ahe wrote:
> > Not sure this check is needed.
> 
> If I understand correctly, this check is used to skip complexities of
resolving
> "a.b(args)" when we have just "a(args)". There are few places in listener
where
> Send node is created with null receiver.

Yes. I'm being stupid :-)

[ahe, 2012-10-11 07:19:21]

LGTM!

https://chromiumcodereview.appspot.com/11014010/diff/6002/lib/compiler/implem...
File lib/compiler/implementation/universe/universe.dart (right):

https://chromiumcodereview.appspot.com/11014010/diff/6002/lib/compiler/implem...
lib/compiler/implementation/universe/universe.dart:205: bool isAssert() { return
(isCall() && name.stringValue === "assert"); }
Use => shorthand or add newline after {.

https://chromiumcodereview.appspot.com/11014010/diff/6002/lib/compiler/implem...
File lib/compiler/implementation/warnings.dart (right):

https://chromiumcodereview.appspot.com/11014010/diff/6002/lib/compiler/implem...
lib/compiler/implementation/warnings.dart:307: "Wrong number of arguments for
assert. Should be 1, but given #{1}.");
for -> to?

[aam-me, 2012-10-11 12:26:05]

Thank you for the corrections, Peter!

https://chromiumcodereview.appspot.com/11014010/diff/6002/lib/compiler/implem...
File lib/compiler/implementation/universe/universe.dart (right):

https://chromiumcodereview.appspot.com/11014010/diff/6002/lib/compiler/implem...
lib/compiler/implementation/universe/universe.dart:205: bool isAssert() { return
(isCall() && name.stringValue === "assert"); }
On 2012/10/11 07:19:21, ahe wrote:
> Use => shorthand or add newline after {.

Done.

https://chromiumcodereview.appspot.com/11014010/diff/6002/lib/compiler/implem...
File lib/compiler/implementation/warnings.dart (right):

https://chromiumcodereview.appspot.com/11014010/diff/6002/lib/compiler/implem...
lib/compiler/implementation/warnings.dart:307: "Wrong number of arguments for
assert. Should be 1, but given #{1}.");
Thanks! Done.

[kasperl, 2012-10-12 10:41:02]

I hope it's okay that I'm trying to land this change as is here:
https://codereview.chromium.org/11113009/.

Maybe I'm a bit too impatient, but it's just great to see more of the M1 issues
being resolved! Hopefully I can help fix any remaining status file issues in the
process.

Cheers,
Kasper