Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(117)

Issue 8440014: Two-phase constructors (Closed)

Created:
9 years, 1 month ago by hausner
Modified:
9 years, 1 month ago
Reviewers:
srdjan
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Two-phase constructors This CL fixes the initializer evaluation order and brings the VM fully up to spec. Constructors are executed in two phases: the initializer phase and the constructor body phase. An additional implicit parameter tells the constructor code whether to execute the initializer list or the body (or both). If the super initializer call is not at the end of the initializer list, the super constructor has to be called twice. The arguments to the super initializer have to be evaluated and stored in temporary local variables. At the beginning of the constructor block, a second implicit super call is inserted. If the super initializer call happens to be at the end of the list, none of this madness has to happen, except for the extra implicit parameter to the constructor. Committed: https://code.google.com/p/dart/source/detail?r=1102

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 8

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+286 lines, -99 lines) Patch
M runtime/lib/isolate.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/code_generator_ia32.cc View 1 2 1 chunk +9 lines, -2 lines 0 comments Download
M runtime/vm/dart_entry.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M runtime/vm/object.h View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M runtime/vm/parser.h View 1 2 3 chunks +5 lines, -3 lines 0 comments Download
M runtime/vm/parser.cc View 1 2 24 chunks +265 lines, -89 lines 0 comments Download
M tests/language/language.status View 1 2 2 chunks +0 lines, -4 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
hausner
9 years, 1 month ago (2011-11-01 23:27:04 UTC) #1
srdjan
http://codereview.chromium.org/8440014/diff/7001/runtime/vm/parser.cc File runtime/vm/parser.cc (right): http://codereview.chromium.org/8440014/diff/7001/runtime/vm/parser.cc#newcode1582 runtime/vm/parser.cc:1582: if (func.IsConstructor()) { The if (func.IsConstructor()) is very long. ...
9 years, 1 month ago (2011-11-02 00:08:13 UTC) #2
srdjan
LGTM as well.
9 years, 1 month ago (2011-11-02 00:08:24 UTC) #3
hausner
9 years, 1 month ago (2011-11-02 00:26:40 UTC) #4
Thank you!

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

http://codereview.chromium.org/8440014/diff/7001/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:1582: if (func.IsConstructor()) {
On 2011/11/02 00:08:13, srdjan wrote:
> The if (func.IsConstructor()) is very long. Can you factor it out to a
separate
> method?

I am planning to separate the compilation of constructors and regular functions
in a subsequent change list.

http://codereview.chromium.org/8440014/diff/7001/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:1593: // constuctor's initializer list get compiled.
On 2011/11/02 00:08:13, srdjan wrote:
> constructor's

Done.

http://codereview.chromium.org/8440014/diff/7001/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:1685: if (func.IsConstructor()) {
On 2011/11/02 00:08:13, srdjan wrote:
> Also a very  long if.

see above.

http://codereview.chromium.org/8440014/diff/7001/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:5981: GrowableArray<const Object*>
arg_values(arguments->length() + 2);
On 2011/11/02 00:08:13, srdjan wrote:
> Please add comment about + 2

Done.

Powered by Google App Engine
This is Rietveld 408576698