[kernel] Allow local initializers with redirecting constructors.

[kernel] Allow local initializers with redirecting constructors.

Summary:

Previously, we had asserts in the kernel flowgraph builder than assumed
redirecting initializers were not accompanied by any other initializers. This
was to prevent the presence of field initializers alongside them.

Now, we allow (only) local initializers to appear with redirecting initializers
because it's safe and necessary for closure conversion.

Test Plan:

Re-ran pkg/kernel/test/closures_type_vars/suite.dart and
pkg/kernel/test/closures/suite.dart -- all tests pass again.

BUG=
R=jensj@google.com

Committed: https://github.com/dart-lang/sdk/commit/66fe821637f91a6f62dc5c7d5295bfb1213a879d

[sjindel, 2017-08-04 16:46:10]

Description was changed from

==========
[kernel] Allow local initializers with redirecting constructors.

Summary:

Previously, we had asserts in the kernel flowgraph builder than assumed
redirecting initializers were not accompanied by any other initializers. This
was to prevent the presence of field initializers alongside them.

Now, we allow (only) local initializers to appear with redirecting initializers
because it's safe and necessary for closure conversion.

Test Plan:

Re-ran pkg/kernel/test/closures_type_vars/suite.dart and
pkg/kernel/test/closures/suite.dart -- all tests pass again.

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

to

==========
[kernel] Allow local initializers with redirecting constructors.

Summary:

Previously, we had asserts in the kernel flowgraph builder than assumed
redirecting initializers were not accompanied by any other initializers. This
was to prevent the presence of field initializers alongside them.

Now, we allow (only) local initializers to appear with redirecting initializers
because it's safe and necessary for closure conversion.

Test Plan:

Re-ran pkg/kernel/test/closures_type_vars/suite.dart and
pkg/kernel/test/closures/suite.dart -- all tests pass again.

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

[sjindel, 2017-08-04 16:46:10]

sjindel@google.com changed reviewers:
+ dmitryas@google.com

[sjindel, 2017-08-07 09:05:11]

sjindel@google.com changed reviewers:
+ jensj@google.com
- dmitryas@google.com

[jensj, 2017-08-07 09:12:21]

lgtm with small nit.

If you have only run the closure tests you should probably run more tests
though...

https://codereview.chromium.org/2992323002/diff/1/runtime/vm/kernel_binary_fl...
File runtime/vm/kernel_binary_flowgraph.cc (right):

https://codereview.chromium.org/2992323002/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:2825: if (PeekTag() ==
kRedirectingInitializer)
nit: I think the style guide wants braces because
a) its not on one line
b) it has an else case

(I'm not saying I've necessarily adhered to it all the time though I should).

[sjindel, 2017-08-07 09:17:34]

On 2017/08/07 09:12:21, jensj wrote:
> lgtm with small nit.
> 
> If you have only run the closure tests you should probably run more tests
> though...
> 
Sure, are there any others you suggest?

[sjindel, 2017-08-07 09:17:41]

https://codereview.chromium.org/2992323002/diff/1/runtime/vm/kernel_binary_fl...
File runtime/vm/kernel_binary_flowgraph.cc (right):

https://codereview.chromium.org/2992323002/diff/1/runtime/vm/kernel_binary_fl...
runtime/vm/kernel_binary_flowgraph.cc:2825: if (PeekTag() ==
kRedirectingInitializer)
On 2017/08/07 09:12:21, jensj wrote:
> nit: I think the style guide wants braces because
> a) its not on one line
> b) it has an else case
> 
> (I'm not saying I've necessarily adhered to it all the time though I should).

Done.

[jensj, 2017-08-07 09:18:37]

On 2017/08/07 09:17:34, sjindel wrote:
> On 2017/08/07 09:12:21, jensj wrote:
> > lgtm with small nit.
> > 
> > If you have only run the closure tests you should probably run more tests
> > though...
> > 
> Sure, are there any others you suggest?

I have started running "./pkg/front_end/tool/fasta testing fasta_min" though it
takes ~25 minutes...

[jensj, 2017-08-09 08:41:56]

On 2017/08/07 09:18:37, jensj wrote:
> On 2017/08/07 09:17:34, sjindel wrote:
> > On 2017/08/07 09:12:21, jensj wrote:
> > > lgtm with small nit.
> > > 
> > > If you have only run the closure tests you should probably run more tests
> > > though...
> > > 
> > Sure, are there any others you suggest?
> 
> I have started running "./pkg/front_end/tool/fasta testing fasta_min" though
it
> takes ~25 minutes...

(in case the above could be misinterpreted: I haven't started a test-run on this
change; what I meant was that I have switched to using that command to test my
own stuff).

[sjindel, 2017-08-14 12:05:24]

Description was changed from

==========
[kernel] Allow local initializers with redirecting constructors.

Summary:

Previously, we had asserts in the kernel flowgraph builder than assumed
redirecting initializers were not accompanied by any other initializers. This
was to prevent the presence of field initializers alongside them.

Now, we allow (only) local initializers to appear with redirecting initializers
because it's safe and necessary for closure conversion.

Test Plan:

Re-ran pkg/kernel/test/closures_type_vars/suite.dart and
pkg/kernel/test/closures/suite.dart -- all tests pass again.

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

to

==========
[kernel] Allow local initializers with redirecting constructors.

Summary:

Previously, we had asserts in the kernel flowgraph builder than assumed
redirecting initializers were not accompanied by any other initializers. This
was to prevent the presence of field initializers alongside them.

Now, we allow (only) local initializers to appear with redirecting initializers
because it's safe and necessary for closure conversion.

Test Plan:

Re-ran pkg/kernel/test/closures_type_vars/suite.dart and
pkg/kernel/test/closures/suite.dart -- all tests pass again.

BUG=
R=jensj@google.com

Committed:
https://github.com/dart-lang/sdk/commit/66fe821637f91a6f62dc5c7d5295bfb1213a879d
==========

[sjindel, 2017-08-14 12:05:26]

Committed patchset #1 (id:1) manually as
66fe821637f91a6f62dc5c7d5295bfb1213a879d (presubmit successful).