First implementation of top-level fields without initializers.

First implementation of top-level fields without initializers.
Committed: https://code.google.com/p/dart/source/detail?r=2346

[ahe, 2011-12-12 14:56:55]

LGTM if you address the comments below.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.dart
File frog/leg/scanner/listener.dart (right):

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:465: List<Identifier> names = <Identifier>[];
Please use Link or LinkBuilder.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:469: Element fields = new
PartialFieldListElement(beginToken, endToken);
FieldList -> Fields?

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:471: // Push in source order.
Normally the order is reversed, isn't it?

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1037: => variables.parseNode(canceler, logger);
I prefer using { return ... } when it doesn't fit on one line.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1039: Type computeType(Compiler compiler, Type
types)
This method doesn't belong in this library.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1040: => variables.computeType(compiler, types);
Ditto for { return ... }.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1061: Type computeType(Compiler compiler, Type
types) {
This method doesn't belong in this library.

http://codereview.chromium.org/8924001/diff/9001/tests/language/language.status
File tests/language/language.status (right):

http://codereview.chromium.org/8924001/diff/9001/tests/language/language.stat...
tests/language/language.status:324: StaticTopLevelTest/02: Fail
Why are these failing?

[karlklose, 2011-12-12 15:09:47]

LGTM, if you address Peter's comments.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/elements/elements.dart
File frog/leg/elements/elements.dart (right):

http://codereview.chromium.org/8924001/diff/9001/frog/leg/elements/elements.d...
frog/leg/elements/elements.dart:34: static final ElementKind VARIABLE_LIST =
const ElementKind('variable_list');
VARIABLE_LIST -> FIELD_LIST?

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.dart
File frog/leg/scanner/listener.dart (right):

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1045: class PartialFieldListElement extends
Element {
Can you add a short comment on what this element represents?

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1048: Node node;
Or perhaps adding a more precise type for the Node would make it more obvious?

[ngeoffray, 2011-12-12 15:52:08]

Thank you for the comments! PTAL.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/elements/elements.dart
File frog/leg/elements/elements.dart (right):

http://codereview.chromium.org/8924001/diff/9001/frog/leg/elements/elements.d...
frog/leg/elements/elements.dart:34: static final ElementKind VARIABLE_LIST =
const ElementKind('variable_list');
On 2011/12/12 15:09:48, karlklose wrote:
> VARIABLE_LIST -> FIELD_LIST?

Done.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.dart
File frog/leg/scanner/listener.dart (right):

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:465: List<Identifier> names = <Identifier>[];
On 2011/12/12 14:56:57, ahe wrote:
> Please use Link or LinkBuilder.

Done.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:469: Element fields = new
PartialFieldListElement(beginToken, endToken);
On 2011/12/12 14:56:57, ahe wrote:
> FieldList -> Fields?

As discussed, maintained FieldList according to the names in the grammar.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:471: // Push in source order.
On 2011/12/12 14:56:57, ahe wrote:
> Normally the order is reversed, isn't it.

Yes.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1037: => variables.parseNode(canceler, logger);
On 2011/12/12 14:56:57, ahe wrote:
> I prefer using { return ... } when it doesn't fit on one line.

Done.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1039: Type computeType(Compiler compiler, Type
types)
On 2011/12/12 14:56:57, ahe wrote:
> This method doesn't belong in this library.

Done.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1040: => variables.computeType(compiler, types);
On 2011/12/12 14:56:57, ahe wrote:
> Ditto for { return ... }.

Done.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1045: class PartialFieldListElement extends
Element {
On 2011/12/12 15:09:48, karlklose wrote:
> Can you add a short comment on what this element represents?

Done.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1048: Node node;
On 2011/12/12 15:09:48, karlklose wrote:
> Or perhaps adding a more precise type for the Node would make it more obvious?


Done.

http://codereview.chromium.org/8924001/diff/9001/frog/leg/scanner/listener.da...
frog/leg/scanner/listener.dart:1061: Type computeType(Compiler compiler, Type
types) {
On 2011/12/12 14:56:57, ahe wrote:
> This method doesn't belong in this library.

Done.

http://codereview.chromium.org/8924001/diff/9001/tests/language/language.status
File tests/language/language.status (right):

http://codereview.chromium.org/8924001/diff/9001/tests/language/language.stat...
tests/language/language.status:324: StaticTopLevelTest/02: Fail
On 2011/12/12 14:56:57, ahe wrote:
> Why are these failing?

We're now parsing more, and things like "static final" as top-level is not
diagnosed as a compile time error yet.

[karlklose, 2011-12-12 17:02:09]

LGTM.

http://codereview.chromium.org/8924001/diff/10003/frog/leg/elements/elements....
File frog/leg/elements/elements.dart (right):

http://codereview.chromium.org/8924001/diff/10003/frog/leg/elements/elements....
frog/leg/elements/elements.dart:98: Node parseNode(Canceler canceler, Logger
logger) {
Node -> VariableDefinitions.

http://codereview.chromium.org/8924001/diff/10003/frog/leg/scanner/listener.dart
File frog/leg/scanner/listener.dart (right):

http://codereview.chromium.org/8924001/diff/10003/frog/leg/scanner/listener.d...
frog/leg/scanner/listener.dart:470: 
Is this empty line on purpose?

[ngeoffray, 2011-12-12 17:17:58]

Thanks Karl!

http://codereview.chromium.org/8924001/diff/10003/frog/leg/elements/elements....
File frog/leg/elements/elements.dart (right):

http://codereview.chromium.org/8924001/diff/10003/frog/leg/elements/elements....
frog/leg/elements/elements.dart:98: Node parseNode(Canceler canceler, Logger
logger) {
On 2011/12/12 17:02:09, karlklose wrote:
> Node -> VariableDefinitions.

Done.

http://codereview.chromium.org/8924001/diff/10003/frog/leg/scanner/listener.dart
File frog/leg/scanner/listener.dart (right):

http://codereview.chromium.org/8924001/diff/10003/frog/leg/scanner/listener.d...
frog/leg/scanner/listener.dart:470: 
On 2011/12/12 17:02:09, karlklose wrote:
> Is this empty line on purpose?

No. Done.

[ahe, 2011-12-12 18:54:09]

Still LGTM!

http://codereview.chromium.org/8924001/diff/10003/frog/leg/scanner/listener.dart
File frog/leg/scanner/listener.dart (right):

http://codereview.chromium.org/8924001/diff/10003/frog/leg/scanner/listener.d...
frog/leg/scanner/listener.dart:1027: class PartialFieldElement extends
VariableElement {
This seems unrelated to the parser.

[ngeoffray, 2011-12-13 08:00:14]

http://codereview.chromium.org/8924001/diff/10003/frog/leg/scanner/listener.dart
File frog/leg/scanner/listener.dart (right):

http://codereview.chromium.org/8924001/diff/10003/frog/leg/scanner/listener.d...
frog/leg/scanner/listener.dart:1027: class PartialFieldElement extends
VariableElement {
On 2011/12/12 18:54:10, ahe wrote:
> This seems unrelated to the parser.

Good point. Code removed in http://codereview.chromium.org//8930004.