Add a new magic element type for annotating types that have Javascript array behavior.

Add a new magic element kind for annotating types that have Javascript array behavior.
Committed: https://code.google.com/p/dart/source/detail?r=8611

[ngeoffray, 2012-05-30 15:14:03]

This change is intended to make progress on improving performance on array-like
objects in the DOM.

@Stephen: to use this new element just do:

class _Int32ArrayImpl extends _ArrayBufferViewImpl implements Int32Array,
JavaScriptArrayBehavior, List<int> native "*Int32Array" {
...
}

@Kasper, Peter: I haven't changed the real JS array to "implement" that
interface, so the guard check is now very long, ie:

if (typeof obj !== 'string' || typeof obj !== object || obj.constructor !==
Array || !obj.is$JavaScriptArrayBehavior()) bailout();

Patching each instantiated array turns out to be annoying because of our list
literals, and because of how arrays are being handled in multiple places in the
compiler. So I'm not sure it makes sense right now to try to share that
interface. Maybe the bailout check should just invoke a method.

[floitsch, 2012-05-30 19:03:54]

On 2012/05/30 15:14:03, ngeoffray wrote:
> This change is intended to make progress on improving performance on
array-like
> objects in the DOM.
> 
> @Stephen: to use this new element just do:
> 
> class _Int32ArrayImpl extends _ArrayBufferViewImpl implements Int32Array,
> JavaScriptArrayBehavior, List<int> native "*Int32Array" {
> ...
> }
> 
> @Kasper, Peter: I haven't changed the real JS array to "implement" that
> interface, so the guard check is now very long, ie:
> 
> if (typeof obj !== 'string' || typeof obj !== object || obj.constructor !==
> Array || !obj.is$JavaScriptArrayBehavior()) bailout();
> 
> Patching each instantiated array turns out to be annoying because of our list
> literals, and because of how arrays are being handled in multiple places in
the
> compiler. So I'm not sure it makes sense right now to try to share that
> interface. Maybe the bailout check should just invoke a method.
I think that reducing the number of checked types is more interesting. We could
either rely on the type-annotations or use other heuristics.

[sra1, 2012-05-30 20:35:39]

Question: does injecting the name this way make it visible by importing the
library? Should it be a _private name?

"JavaScriptArrayBehavior" is not the best name IMO, since JavaScript arrays do
many things that we don't care about - e.g. automatically changing length when
you assign past the end.

There are several basic properties I can think of that would be useful to the
compiler and I think we should choose a name that makes it clear which set of
properties is being declared:

(1) The indexer checks for an int value i  0 <= i < a.length.
(2) The array indexer can be implemented by the JavaScript indexer for checked
indexes
(3) The length cannot change
(4) The element values are not null
(5) The element values are numbers in some range.
(6) The array is writable

So far we are exploiting the combination (1+2) which is a great place to start.
We should probably be checking (1+2+6) if there is an array assignment - many
DOM array-like types are read-only so have index setters that throw.

(3) should will help with constraint based bounds check elimination like ABCD.
(4+5) should help with how the values are consumed.

I don't particularly like these names but they are more descriptive:

(1+2):   _IndexerIsBoundsCheckThenJavaScriptIndexer
(1+2+6): _IndexerIsBoundsCheckThenJavaScriptIndexerWritable

[ahe, 2012-05-31 08:29:47]

I have been thinking about this. I think the approach makes sense, but I'll like
to discuss a few details in person.

[ngeoffray, 2012-06-06 12:20:45]

PTAL

@Stephen: The interface will only be visible to libraries we decide can have
access to it. So there's no need to mark it private (same applies to our JS
method).

[ngeoffray, 2012-06-11 10:19:13]

Ping

[sra1, 2012-06-12 17:48:37]

lgtm

[ahe, 2012-06-12 18:30:04]

LGTM!

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
File lib/compiler/implementation/compiler.dart (right):

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:116: ClassElement
javaScriptIndexingBehaviorInterface;
How about jsIndexingBehaviorInterface?

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
File lib/compiler/implementation/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
lib/compiler/implementation/lib/js_helper.dart:1079: * visible to anyone, and is
only injected into special lirbaries.
lirbaries -> libraries

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
File lib/compiler/implementation/native_handler.dart (right):

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
lib/compiler/implementation/native_handler.dart:86:
library.define(compiler.javaScriptIndexingBehaviorInterface, compiler);
For the unit tests, it may make sense to test for null here.

[ngeoffray, 2012-06-13 13:02:42]

Thanks Peter and Stephen

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
File lib/compiler/implementation/compiler.dart (right):

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:116: ClassElement
javaScriptIndexingBehaviorInterface;
On 2012/06/12 18:30:04, ahe wrote:
> How about jsIndexingBehaviorInterface?

Done.

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
File lib/compiler/implementation/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
lib/compiler/implementation/lib/js_helper.dart:1079: * visible to anyone, and is
only injected into special lirbaries.
On 2012/06/12 18:30:04, ahe wrote:
> lirbaries -> libraries

Done.

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
File lib/compiler/implementation/native_handler.dart (right):

https://chromiumcodereview.appspot.com/10447088/diff/8001/lib/compiler/implem...
lib/compiler/implementation/native_handler.dart:86:
library.define(compiler.javaScriptIndexingBehaviorInterface, compiler);
On 2012/06/12 18:30:04, ahe wrote:
> For the unit tests, it may make sense to test for null here.

Done.