classes implementing TypedData must not implement ByteBuffer

[rakudrama]

import 'dart:typeddata';

main() {
  var a = new Int8List.fromList([0,1,2,3,4,5,6,7]);
  var b = new Int32List.view(a); // Should be a.buffer
  print(b);
}

The above program appears to work on VM based implementations:

$ sdk/bin/dart bug5v.dart
[50462976, 117835012]
$ sdk/bin/dart --checked bug5v.dart
[50462976, 117835012]

It does not work under dart2js:

$ sdk/bin/dart2js bug5v.dart --out=o.js && out/ReleaseIA32/d8 o.js
[0, 1, 2, 3, 4, 5, 6, 7]
$ sdk/bin/dart2js bug5v.dart --checked --out=o.js && out/ReleaseIA32/d8 o.js
o.js:676: type 'Int8Array' is not a subtype of type 'ByteBuffer'
  throw $.wrapException(ex);
          ^
type 'Int8Array' is not a subtype of type 'ByteBuffer'
    at $.throwExpression (o.js:676:11)
    at $.interceptedTypeCheck (o.js:772:5)
    at $.main (o.js:514:5)
    at o.js:1303:7

It is impossible to make the JavaScript Int8Array type implement ByteBuffer efficiently.

The VM implementation should be changed so that classes implementing typed data (e.g _Int8Array) never implement ByteBuffer. We have plenty of evidence that it is too confusing for the VM / Dartium to be more permissive than what is possible in JavaScript.

[kasperl]

cc @madsager.

[iposva-google]

I am contesting the "plenty of evidence" based on the fact that the dart2js implementation of typeddata in dart:html landed a couple of weeks ago. Thus I am changing the Milestone to Later until we see whether this is REALLY a problem.

---

cc @a-siva.
Removed this from the M5 milestone.
Added this to the Later milestone.

[iposva-google]

s/couple of weeks/couple of days/

[kasperl]

Bear in mind that Stephen is describing a fairly general issue (the VM being more permissive than dart2js) and that it isn't just something we've seen with typed data.

[vsmenon]

I've seen this problem bite us a couple times on tests the Dart team itself has written.

E.g:
This was a fix to a test that only worked on the VM:

https://chromiumcodereview.appspot.com/13244003/diff/1/tests/standalone/typed_data_test.dart

This was an inadvertent break introduced in another test (and later fixed). Again, worked in the VM:

https://codereview.chromium.org/13845015/diff/1/tests/html/postmessage_structured_test.dart

[a-siva]

Looking at the interface specification in typed_data.dart it states
abstract class Int8List implements List<int>, TypedData {
  ...
};
This doesn't seem to imply that Int8List should not implement the ByteBuffer
interface.

Are you suggesting a new language feature stating
abstract class Int8List implements List<int>, TypedData 'and does not implement ByteBuffer' {
  ....
};

[vsmenon]

I don't think anyone wants that language feature. :-)

I think there are two separate questions here:

(1) Is there a problem?

We have some evidence that it's easy to write code that confuses one type for the other (see CLs).

(2) If so, how should we solve it?

We could change the VM implementation as sra suggests. Or we could try to catch this with static tooling. (E.g., in a statically typed language, the type checker would have rejected the examples in those CLs.)

[madsager]

I think a real issue here is that for programming web applications we are using dart:typed_data as our typed arrays. Typed arrays have a specification:

http://www.khronos.org/registry/typedarray/specs/latest/

In that specification, the ArrayBuffer (ByteBuffer) and ArrayBufferView (TypedData) types are different types and it seems pretty clear that the intention is not to have objects implement both. Some browser APIs depends on objects only having one type or the other.

The main example is postMessage. postMessage sends a structured clone of its argument to the other end. The behavior of typed arrays under structured cloning are given in the typed array specification:

http://www.khronos.org/registry/typedarray/specs/latest/#­9

ArrayBuffer and ArrayBufferView are treated separately and in the current web platform there are no objects that implement both. Because some of the VM typed_data objects implement both, we have to choose which of the structured cloning steps to take. In the current Dartium implementation we are treating everything as ByteBuffers if transferred. That loses the 'view' property of what you are posting which will be confusing to web programmers:

Float32List a = new Float32List(size);
postMessage(a);

We don't know what to do here and we are going to send the Float32Array to the other side as a ByteBuffer (which will actually be a Uint8List as well because you cannot allocate ByteBuffers on their own with typed_data). The type-mismatch of what you send and what you will receive seems unfortunate for this API.

I do think that having the split between the types would be the right thing to do. I understand the implications that this will introduce an extra indirection in the implementation but I do think that the mismatch with the web platform implemented in all modern browsers is unfortunate.

[iposva-google]

Removed Priority-Medium label.
Added Priority-Unassigned label.

[anders-sandholm]

Issue #12365 has been merged into this issue.

[lrhn]

Looking at the interface specification in typed_data.dart it states
abstract class Int8List implements List<int>, TypedData {
...
};
This doesn't seem to imply that Int8List should not implement the ByteBuffer
interface.

It more than implies it. An object should not implement a public type that it isn't documented as implementing. It also shouldn't expose public methods that it isn't documented as having.
This is a bug, and it should be fixed. We don't want users to start to depend on the incorrect behavior.

[rakudrama]

Marked this as blocking #12688.

[rakudrama]

Marked this as blocking #10097.

[lrhn]

Please increase priority. This is repeatedly tripping up users.

---

cc @iposva-google.

[DartBot]

This comment was originally written by @bgourlie

---

A priority bump would be much appreciated.

[justinfagnani]

This is now causing an issue in JS interop where we'd like to transfer, not proxy, typed data and have problems determining how to present typed data to JS.

[lrhn]

Removed Priority-Unassigned label.
Added Priority-High label.

[lrhn]

Issue #15698 has been merged into this issue.

[lrhn]

Set owner to @lrhn.
Added Started label.