Add support for running constructors with new

Add support for running constructors with new

BUG=
R=dmitryas@google.com

Committed: https://github.com/dart-lang/sdk/commit/43cf49a7179ce6f45467ed0366258207136ce381

[zhivkag, 2017-07-24 14:33:03]

zhivkag@google.com changed reviewers:
+ dmitryas@google.com, kmillikin@google.com

[zhivkag, 2017-07-24 14:33:04]

This CL adds support for creating new objects with new and further initializing
the object by executing the body of the constructor.

[Dmitry Stefantsov, 2017-07-25 11:45:07]

I have some questions.  Please, find them below.

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
File pkg/kernel/lib/interpreter/interpreter.dart (right):

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:667: constructor, ctrEnv, new
NewSK(continuation, newObject));
From the specification I get it that the second argument to [NewSK] constructor
should be a label, not a value.  Is it so?

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:972: return new
ForwardConfiguration(bodyCont, ctrEnv);
Shouldn't we initialize fields first?

[zhivkag, 2017-07-25 13:17:03]

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
File pkg/kernel/lib/interpreter/interpreter.dart (right):

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:667: constructor, ctrEnv, new
NewSK(continuation, newObject));
On 2017/07/25 11:45:07, Dmitry Stefantsov wrote:
> From the specification I get it that the second argument to [NewSK]
constructor
> should be a label, not a value.  Is it so?

Done.

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:972: return new
ForwardConfiguration(bodyCont, ctrEnv);
On 2017/07/25 11:45:07, Dmitry Stefantsov wrote:
> Shouldn't we initialize fields first?

Indeed, we should initialize the fields first (hence the todo remark above the
return). I was planning to do this in a separate CL just to keep the reviews
smaller. Would you prefer to have the change in this CL instead?

[Dmitry Stefantsov, 2017-07-25 13:21:10]

LGTM

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
File pkg/kernel/lib/interpreter/interpreter.dart (right):

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:972: return new
ForwardConfiguration(bodyCont, ctrEnv);
On 2017/07/25 13:17:02, zhivkag wrote:
> On 2017/07/25 11:45:07, Dmitry Stefantsov wrote:
> > Shouldn't we initialize fields first?
> 
> Indeed, we should initialize the fields first (hence the todo remark above the
> return). I was planning to do this in a separate CL just to keep the reviews
> smaller. Would you prefer to have the change in this CL instead? 

No, another CL is fine.

As a side note, it would be nice to have TODOs in their usual form: in caps and
with an assignee.

[zhivkag, 2017-07-25 15:23:54]

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
File pkg/kernel/lib/interpreter/interpreter.dart (right):

https://codereview.chromium.org/2984903002/diff/40001/pkg/kernel/lib/interpre...
pkg/kernel/lib/interpreter/interpreter.dart:972: return new
ForwardConfiguration(bodyCont, ctrEnv);
On 2017/07/25 13:21:09, Dmitry Stefantsov wrote:
> On 2017/07/25 13:17:02, zhivkag wrote:
> > On 2017/07/25 11:45:07, Dmitry Stefantsov wrote:
> > > Shouldn't we initialize fields first?
> > 
> > Indeed, we should initialize the fields first (hence the todo remark above
the
> > return). I was planning to do this in a separate CL just to keep the reviews
> > smaller. Would you prefer to have the change in this CL instead? 
> 
> No, another CL is fine.
> 
> As a side note, it would be nice to have TODOs in their usual form: in caps
and
> with an assignee.

That's a good point, I have updated the TODOs in the code to follow the
mentioned convention.

[zhivkag, 2017-07-25 15:24:24]

Description was changed from

==========
Add support for running constructors with new

BUG=
==========

to

==========
Add support for running constructors with new

BUG=
R=dmitryas@google.com

Committed:
https://github.com/dart-lang/sdk/commit/43cf49a7179ce6f45467ed0366258207136ce381
==========

[zhivkag, 2017-07-25 15:24:25]

Committed patchset #5 (id:80001) manually as
43cf49a7179ce6f45467ed0366258207136ce381 (presubmit successful).