dart2js-kernel: Recognize some List constructors.

dart2js-kernel: Recognize some List constructors.

BUG=
R=sigmund@google.com

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

[sra1, 2017-07-24 22:01:24]

sra@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2017-07-25 16:54:45]

lgtm

Note: it might be worth checking whether some language/corelib tests start
passing as a result.

https://codereview.chromium.org/2984183002/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2984183002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:2406: if (function.isExternal &&
function.isFromEnvironmentConstructor) {
nit: consider moving this at the beginning of handleInvokeFactoryConstructor?

https://codereview.chromium.org/2984183002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:2491: if (true) {
(no action required)

In case you were considering alternatives, another style I've seen for these
blocks that are always true is to define a block, but no if:

push(foreign);
// TODO: blah blah
{
  doSomething();
}

https://codereview.chromium.org/2984183002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:2500: // TODO(sra): Pick up type
inference type, which might be more precise,
consider using "TODO(redemption)" (so we can grep for everything we left aside
during the migration, as opposed to TODOs that are for further out in the
future.

[sra1, 2017-07-25 20:08:16]

https://codereview.chromium.org/2984183002/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder_kernel.dart (right):

https://codereview.chromium.org/2984183002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:2406: if (function.isExternal &&
function.isFromEnvironmentConstructor) {
On 2017/07/25 16:54:45, Siggi Cherem (dart-lang) wrote:
> nit: consider moving this at the beginning of handleInvokeFactoryConstructor?

Done.

https://codereview.chromium.org/2984183002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:2491: if (true) {
On 2017/07/25 16:54:45, Siggi Cherem (dart-lang) wrote:
> (no action required)
> 
> In case you were considering alternatives, another style I've seen for these
> blocks that are always true is to define a block, but no if:
> 
> push(foreign);
> // TODO: blah blah
> {
>   doSomething();
> }

Done.

https://codereview.chromium.org/2984183002/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder_kernel.dart:2500: // TODO(sra): Pick up type
inference type, which might be more precise,
On 2017/07/25 16:54:45, Siggi Cherem (dart-lang) wrote:
> consider using "TODO(redemption)" (so we can grep for everything we left aside
> during the migration, as opposed to TODOs that are for further out in the
> future.

Done.

[sra1, 2017-07-25 23:31:30]

Description was changed from

==========
dart2js-kernel: Recognize some List constructors.

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

to

==========
dart2js-kernel: Recognize some List constructors.

BUG=
R=sigmund@google.com

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

[sra1, 2017-07-25 23:31:31]

Committed patchset #4 (id:60001) manually as
af56ba7b19aa40509b8a5a381e7d546bf13e49b9 (presubmit successful).