Separating constructor parsing from function parsing

Separating constructor parsing from function parsing

Make parsing functions and constructors more readable by separating
the two into disjoint functions.
Committed: https://code.google.com/p/dart/source/detail?r=1172

[hausner, 2011-11-03 21:30:43]

Looks like a big change, but it's mostly moving code and eliminating a few if
statements. Thanks.

[hausner, 2011-11-03 22:30:51]

TBR. I'll address potential comments in a separate checkin.

[regis, 2011-11-03 23:58:22]

LGTM

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

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1536
runtime/vm/parser.cc:1536: if (IsLiteral("class")) {
This is not new code, but I was nevertheless puzzled by how we can see "class"
here. I would add a comment referring to Parser::CheckConstructors and explain
that the default token position in ClassDesc points to the "class" token of its
declaration.

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1551
runtime/vm/parser.cc:1551: // Add implicit parameter for constructor phase.
Should you use "construction phase" here and elsewhere?

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1566
runtime/vm/parser.cc:1566: // are is generated.
Comment parsing error.

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1578
runtime/vm/parser.cc:1578: // to strore the initializer expressions in the
respective instance fields.
strore -> store

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1643
runtime/vm/parser.cc:1643: // and add the implicit super call to the super
construcotr's body
construcotr -> ...

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1690
runtime/vm/parser.cc:1690: // initializer.
Doesn't the above comment fit on one line?

[hausner, 2011-11-04 17:04:55]

Addressed in a separate checkin. Thanks!

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

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1536
runtime/vm/parser.cc:1536: if (IsLiteral("class")) {
On 2011/11/03 23:58:22, regis wrote:
> This is not new code, but I was nevertheless puzzled by how we can see "class"
> here. I would add a comment referring to Parser::CheckConstructors and explain
> that the default token position in ClassDesc points to the "class" token of
its
> declaration.

Done.

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1551
runtime/vm/parser.cc:1551: // Add implicit parameter for constructor phase.
On 2011/11/03 23:58:22, regis wrote:
> Should you use "construction phase" here and elsewhere?

True. Done in all places.

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1566
runtime/vm/parser.cc:1566: // are is generated.
On 2011/11/03 23:58:22, regis wrote:
> Comment parsing error.

Done.

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1578
runtime/vm/parser.cc:1578: // to strore the initializer expressions in the
respective instance fields.
On 2011/11/03 23:58:22, regis wrote:
> strore -> store

Done.

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1643
runtime/vm/parser.cc:1643: // and add the implicit super call to the super
construcotr's body
On 2011/11/03 23:58:22, regis wrote:
> construcotr -> ...

Done.

http://codereview.chromium.org/8417056/diff/1/runtime/vm/parser.cc#newcode1690
runtime/vm/parser.cc:1690: // initializer.
On 2011/11/03 23:58:22, regis wrote:
> Doesn't the above comment fit on one line?

Done.