dart2js cps: Direct access on JS arrays.

dart2js cps: Direct access on JS arrays.

length, forEach, [], and []= are rewritten to direct array access nodes.
There are more methods to specialize, which will be added in separate
CLs.

This CL also contains a class CpsFragment to help build CPS code.

BUG=
R=karlklose@google.com

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

[asgerf, 2015-07-07 14:53:12]

asgerf@google.com changed reviewers:
+ floitsch@google.com, karlklose@google.com

[asgerf, 2015-07-07 14:53:13]

https://codereview.chromium.org/1223813006/diff/20001/tests/language/language...
File tests/language/language_dart2js.status (right):

https://codereview.chromium.org/1223813006/diff/20001/tests/language/language...
tests/language/language_dart2js.status:438: if_null_assignment_static_test/33:
RuntimeError # TypeError: v0.get$a is not a function
This seems to be a pre-existing bug that has surfaced.

It seems to be related to the issue with direct field accesses:
https://codereview.chromium.org/1227543008/

In these cases it seems resolution has detected that a getter call should become
a direct field access (and refuse to emit the getter), but the class world still
can't find a unique field element for it.

[karlklose, 2015-07-08 08:39:24]

LGTM!

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_fragment.dart (right):

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_fragment.dart:18: /// a hole.
Maybe add: "Otherwise, the fragment is 'closed' and a call to put (or any
operation that uses out) will result in an assertion failure in checked mode.

Methods that return a CpsFragment indicate that the fragment is open, those with
a void return type close the fragment."


Then we also could remove the repeated "; no more nodes may be added." in the
comments of methods.

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_fragment.dart:118: Continuation cont = new
Continuation([new Parameter(null)]);
Add type arguments to list literals? (multiple places)

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:895: if ((use.selector.isIndex
|| use.selector.isIndexSet) &&
Merge the two if statements.

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:899: } else if (use is
GetIndex && use.object.definition == list) {
Merge these two conditions?

 if ((use is GetIndex || use is SetIndex) && use.object.definition == list) {
... }

Maybe these two classes should implement an interface like:
  abstract class IndexArrayOperation {
    final Reference<Primitive> object;
  }

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:909: bool
specializeArrayAccess(InvokeMethod node) {
Please document the meaning of the returned value.

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1160: if
(constifyExpression(node)) return;
Maybe make this one if statement and add a comment that we generated specialized
code for that method call.

[asgerf, 2015-07-08 10:28:37]

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/cps_fragment.dart (right):

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_fragment.dart:18: /// a hole.
On 2015/07/08 08:39:24, karlklose wrote:
> Maybe add: "Otherwise, the fragment is 'closed' and a call to put (or any
> operation that uses out) will result in an assertion failure in checked mode.
> 
> Methods that return a CpsFragment indicate that the fragment is open, those
with
> a void return type close the fragment."
> 
> 
> Then we also could remove the repeated "; no more nodes may be added." in the
> comments of methods.

Added comment about "closed".

The return type isn't quite indicative of whether the fragment is closed. The
'append' method leaves it open when appending an open fragment, for example.

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/cps_fragment.dart:118: Continuation cont = new
Continuation([new Parameter(null)]);
On 2015/07/08 08:39:24, karlklose wrote:
> Add type arguments to list literals? (multiple places)

I've added some type arguments in places where it doesn't obscure the code that
much.

There are still places where I left them out because they make the code hard to
read. In these cases, the list is immediately converted into a (typed) list of
references, so it doesn't stop checked-mode from doing its job.

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
File pkg/compiler/lib/src/cps_ir/type_propagation.dart (right):

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:895: if ((use.selector.isIndex
|| use.selector.isIndexSet) &&
On 2015/07/08 08:39:24, karlklose wrote:
> Merge the two if statements.

Done.

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:899: } else if (use is
GetIndex && use.object.definition == list) {
On 2015/07/08 08:39:24, karlklose wrote:
> Merge these two conditions?
> 
>  if ((use is GetIndex || use is SetIndex) && use.object.definition == list) {
> ... }
> 
> Maybe these two classes should implement an interface like:
>   abstract class IndexArrayOperation {
>     final Reference<Primitive> object;
>   }

As you foresaw it creates a static type warning because type promotion cannot
promote the type of 'use' to something with an 'object' getter.

I'm not too keen on adding another "union supertype" (for lack of a better word)
unless there is a strong case for adding it. They are very useful, but it tends
to go badly when there are many of them.

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:909: bool
specializeArrayAccess(InvokeMethod node) {
On 2015/07/08 08:39:24, karlklose wrote:
> Please document the meaning of the returned value.

Done.

https://codereview.chromium.org/1223813006/diff/20001/pkg/compiler/lib/src/cp...
pkg/compiler/lib/src/cps_ir/type_propagation.dart:1160: if
(constifyExpression(node)) return;
On 2015/07/08 08:39:24, karlklose wrote:
> Maybe make this one if statement and add a comment that we generated
specialized
> code for that method call.

I feel it's misleading to use the || operator here.

[asgerf, 2015-07-08 12:33:05]

Committed patchset #5 (id:80001) manually as
895283c21a56ca99736aeaa8fc92e6d4c260bc39 (presubmit successful).