Expose transferable constructor to Int8List

Expose transferable constructor to Int8List

Committed: https://code.google.com/p/dart/source/detail?r=15598

[cshapiro, 2012-11-28 22:28:01]

https://codereview.chromium.org/11414211/diff/1/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/1/runtime/lib/byte_array.cc#new...
runtime/lib/byte_array.cc:46: static void* AllocateExternalBytes(intptr_t len,
intptr_t alignment = 16) {
Maybe stick this constant into the base class of the external byte arrays.

https://codereview.chromium.org/11414211/diff/1/runtime/lib/byte_array.dart
File runtime/lib/byte_array.dart (right):

https://codereview.chromium.org/11414211/diff/1/runtime/lib/byte_array.dart#n...
runtime/lib/byte_array.dart:410: static _Int8Array _newTransferrable(int length)
native "Int8Array_newTransferrable";
line break here...

[srdjan, 2012-11-28 23:30:26]

DBC

https://codereview.chromium.org/11414211/diff/1/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/1/runtime/lib/byte_array.cc#new...
runtime/lib/byte_array.cc:48: intptr_t slop = alignment+pointersize;
please add spaces around +, - and everywhere else in new code.

https://codereview.chromium.org/11414211/diff/1/runtime/lib/byte_array.cc#new...
runtime/lib/byte_array.cc:49: void* bytes = malloc(len+slop);
why not intptr_t* ptr = reinterpret_cast<intptr_t*>(malloc(...)), getting rid of
"bytes* void"? Why is ptr intptr_t instead of intptr_t* ?

https://codereview.chromium.org/11414211/diff/1/runtime/lib/byte_array.cc#new...
runtime/lib/byte_array.cc:65: intptr_t ptr = reinterpret_cast<intptr_t>(bytes);
ditto

[cshapiro, 2012-11-29 21:28:17]

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:45: #define EXTERNAL_ALIGNMENT 16
Macros like this are strongly discouraged by the style guide.  

How about making this a static member of ExternalByteArrayData instead?

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:47: void FinalizeExternalBytes(void* ptr) {
Any reason for this routine?  Why not just pass OS::AlignedFree as the callback?

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:297: void* bytes =
OS::AlignedAllocate(len*sizeof(int8_t), EXTERNAL_ALIGNMENT);
I think there are some opportunities for improvement in this code.  First, we
can make OS::AlignedAllocate a template specialized on the type.  This would
internally multiply len by the allocation size and return the correct pointer
type.  This has the added benefit of eliminating the cast on line 298.

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:560: float foo;  // quiet cpplint.
cpplint does not want you using sizeof(<typename>) in general.  The stdint types
above should also be flagged but cpplint is not smart enough to catch that.

I think we have a better fix for this problem by tweaking AlignedAllocate.  If
that was not the case, a "// NOLINT" comment at the end of the offending line is
the right way to muffle cpplint.

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_android.cc
File runtime/vm/os_android.cc (right):

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_android.cc#n...
runtime/vm/os_android.cc:13: #include <malloc.h>
This should be kept sorted.  Move it to line 9.

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_android.cc#n...
runtime/vm/os_android.cc:75: void* p = memalign(alignment, size);
I would follow the example of gettimeofday on line 63

  void* result = memalign(alignment, size)
  if (result == NULL) {
    UNREACHABLE();
  }
  return result;

as we are generally not prepared to handle malloc failures

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_linux.cc#new...
runtime/vm/os_linux.cc:13: #include <malloc.h>
ditto

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_linux.cc#new...
runtime/vm/os_linux.cc:75: void* p = memalign(alignment, size);
ditto

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_win.cc
File runtime/vm/os_win.cc (right):

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_win.cc#newcode8
runtime/vm/os_win.cc:8: #include <malloc.h>
move to line 7

[Cutch, 2012-11-29 23:53:17]

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:45: #define EXTERNAL_ALIGNMENT 16
On 2012/11/29 21:28:17, cshapiro wrote:
> Macros like this are strongly discouraged by the style guide.  
> 
> How about making this a static member of ExternalByteArrayData instead?

Done.

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:47: void FinalizeExternalBytes(void* ptr) {
On 2012/11/29 21:28:17, cshapiro wrote:
> Any reason for this routine?  Why not just pass OS::AlignedFree as the
callback?

Done.

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:297: void* bytes =
OS::AlignedAllocate(len*sizeof(int8_t), EXTERNAL_ALIGNMENT);
On 2012/11/29 21:28:17, cshapiro wrote:
> I think there are some opportunities for improvement in this code.  First, we
> can make OS::AlignedAllocate a template specialized on the type.  This would
> internally multiply len by the allocation size and return the correct pointer
> type.  This has the added benefit of eliminating the cast on line 298.

Done.

https://codereview.chromium.org/11414211/diff/6007/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:560: float foo;  // quiet cpplint.
On 2012/11/29 21:28:17, cshapiro wrote:
> cpplint does not want you using sizeof(<typename>) in general.  The stdint
types
> above should also be flagged but cpplint is not smart enough to catch that.
> 
> I think we have a better fix for this problem by tweaking AlignedAllocate.  If
> that was not the case, a "// NOLINT" comment at the end of the offending line
is
> the right way to muffle cpplint.

Done.

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_android.cc
File runtime/vm/os_android.cc (right):

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_android.cc#n...
runtime/vm/os_android.cc:13: #include <malloc.h>
On 2012/11/29 21:28:17, cshapiro wrote:
> This should be kept sorted.  Move it to line 9.

Done.

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_android.cc#n...
runtime/vm/os_android.cc:75: void* p = memalign(alignment, size);
On 2012/11/29 21:28:17, cshapiro wrote:
> I would follow the example of gettimeofday on line 63
> 
>   void* result = memalign(alignment, size)
>   if (result == NULL) {
>     UNREACHABLE();
>   }
>   return result;
> 
> as we are generally not prepared to handle malloc failures

Done.

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_linux.cc#new...
runtime/vm/os_linux.cc:13: #include <malloc.h>
On 2012/11/29 21:28:17, cshapiro wrote:
> ditto

Done.

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_linux.cc#new...
runtime/vm/os_linux.cc:75: void* p = memalign(alignment, size);
On 2012/11/29 21:28:17, cshapiro wrote:
> ditto

Done.

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_win.cc
File runtime/vm/os_win.cc (right):

https://codereview.chromium.org/11414211/diff/6007/runtime/vm/os_win.cc#newcode8
runtime/vm/os_win.cc:8: #include <malloc.h>
On 2012/11/29 21:28:17, cshapiro wrote:
> move to line 7

Done.

[cshapiro, 2012-11-30 00:23:25]

https://codereview.chromium.org/11414211/diff/2003/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:47: template<typename T>
Sorry, this is not what I meant.  You should make OS::AlignedAllocate a
template.

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_android.cc
File runtime/vm/os_android.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_android.cc#n...
runtime/vm/os_android.cc:78: return NULL;
This return is not needed.

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_linux.cc#new...
runtime/vm/os_linux.cc:78: return NULL;
ditto

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_macos.cc
File runtime/vm/os_macos.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_macos.cc#new...
runtime/vm/os_macos.cc:80: return NULL;
ditto

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_win.cc
File runtime/vm/os_win.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_win.cc#newco...
runtime/vm/os_win.cc:113: return NULL;
ditto

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/raw_object.h#ne...
runtime/vm/raw_object.h:1350: static const int Alignment = 16;
kAlignment

[Cutch, 2012-11-30 04:09:54]

Everything is fixed except the location of the template. See my note.

https://codereview.chromium.org/11414211/diff/2003/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:47: template<typename T>
If OS::AlignedAllocate is a template where should the OS specific
implementations go? In the header or as some other OS:: method?

Why not keep OS::AlignedAllocate as is and add a template method to
OS::AllocateAlignedArray<T>(intptr_t n, intptr_t alignment); ?
On 2012/11/30 00:23:25, cshapiro wrote:
> Sorry, this is not what I meant.  You should make OS::AlignedAllocate a
> template.

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_android.cc
File runtime/vm/os_android.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_android.cc#n...
runtime/vm/os_android.cc:78: return NULL;
On 2012/11/30 00:23:25, cshapiro wrote:
> This return is not needed.

Done.

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_android.cc#n...
runtime/vm/os_android.cc:78: return NULL;
I was copying the structure of OS::GetCurrentTimeMicros- looks like that doesn't
need the return statement either.
On 2012/11/30 00:23:25, cshapiro wrote:
> This return is not needed.

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_linux.cc#new...
runtime/vm/os_linux.cc:78: return NULL;
On 2012/11/30 00:23:25, cshapiro wrote:
> ditto

Done.

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_macos.cc
File runtime/vm/os_macos.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_macos.cc#new...
runtime/vm/os_macos.cc:80: return NULL;
On 2012/11/30 00:23:25, cshapiro wrote:
> ditto

Done.

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_win.cc
File runtime/vm/os_win.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/os_win.cc#newco...
runtime/vm/os_win.cc:113: return NULL;
On 2012/11/30 00:23:25, cshapiro wrote:
> ditto

Done.

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/vm/raw_object.h#ne...
runtime/vm/raw_object.h:1350: static const int Alignment = 16;
On 2012/11/30 00:23:25, cshapiro wrote:
> kAlignment

Done.

[cshapiro, 2012-11-30 18:02:15]

almost there, just minor stuff

https://codereview.chromium.org/11414211/diff/2003/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/2003/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:47: template<typename T>
Adding the OS::AllocateAlignedArray<T> template sound good, thanks.

https://codereview.chromium.org/11414211/diff/7011/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/7011/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:49: intptr_t size = n*sizeof(T); // NOLINT
This should go away once we have the aforementioned template.  In the future,
always put 2 spaces between the semicolon and the comment.  cpplint will not
catch that since warnings are suppressed by the nolint

https://codereview.chromium.org/11414211/diff/7011/runtime/lib/byte_array.dart
File runtime/lib/byte_array.dart (right):

https://codereview.chromium.org/11414211/diff/7011/runtime/lib/byte_array.dar...
runtime/lib/byte_array.dart:790: static _Uint32Array _newTransferrable(int
length) native
minor, but, the prevailing style seems to be to wrap the native along with the
method name (keeping them on the same line)

https://codereview.chromium.org/11414211/diff/7011/runtime/vm/os_test.cc
File runtime/vm/os_test.cc (right):

https://codereview.chromium.org/11414211/diff/7011/runtime/vm/os_test.cc#newc...
runtime/vm/os_test.cc:56: 
2 spaces between definitions

[Cutch, 2012-11-30 18:21:43]

https://codereview.chromium.org/11414211/diff/7011/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/7011/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:49: intptr_t size = n*sizeof(T); // NOLINT
On 2012/11/30 18:02:16, cshapiro wrote:
> This should go away once we have the aforementioned template.  In the future,
> always put 2 spaces between the semicolon and the comment.  cpplint will not
> catch that since warnings are suppressed by the nolint

Done.

https://codereview.chromium.org/11414211/diff/7011/runtime/lib/byte_array.dart
File runtime/lib/byte_array.dart (right):

https://codereview.chromium.org/11414211/diff/7011/runtime/lib/byte_array.dar...
runtime/lib/byte_array.dart:790: static _Uint32Array _newTransferrable(int
length) native
On 2012/11/30 18:02:16, cshapiro wrote:
> minor, but, the prevailing style seems to be to wrap the native along with the
> method name (keeping them on the same line)

Done.

[cshapiro, 2012-11-30 18:40:26]

https://codereview.chromium.org/11414211/diff/24/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/24/runtime/lib/byte_array.cc#ne...
runtime/lib/byte_array.cc:292: int8_t* bytes =
OS::AllocateAlignedArray<int8_t>(len,
newline after the open paren and 4 space indent

  int8_t* bytes = OS::AllocateAlignedArray<int8_t>(
      len,
      ExternalByteArrayData<int8_t>::kAlignment);
      
See

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

[cshapiro, 2012-11-30 18:56:53]

sorry, missed this

https://codereview.chromium.org/11414211/diff/24/runtime/vm/os.h
File runtime/vm/os.h (right):

https://codereview.chromium.org/11414211/diff/24/runtime/vm/os.h#newcode45
runtime/vm/os.h:45: template<typename T>
If you are going to live with the NOLINT why not just

template<typename T>
static T* AllocateAlignedArray(intptr_t n, intptr_t alignment) {
  intptr_t size = n * sizeof(T);  // NOLINT
  return reinterpret_cast<T*>(OS::AlignedAllocate(size, alignment));
}

or avoid it

template<typename T>
static T* AllocateAlignedArray(intptr_t n, intptr_t alignment) {
  T* result = reinterpret_cast<T*>(OS::AlignedAllocate(n * sizeof(*result),
                                                       alignment));
  return result;
}

Either way, please add spaces around *

[Cutch, 2012-11-30 19:27:10]

https://codereview.chromium.org/11414211/diff/24/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/24/runtime/lib/byte_array.cc#ne...
runtime/lib/byte_array.cc:292: int8_t* bytes =
OS::AllocateAlignedArray<int8_t>(len,
On 2012/11/30 18:40:26, cshapiro wrote:
> newline after the open paren and 4 space indent
> 
>   int8_t* bytes = OS::AllocateAlignedArray<int8_t>(
>       len,
>       ExternalByteArrayData<int8_t>::kAlignment);
>       
> See
> 
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

Done.

https://codereview.chromium.org/11414211/diff/24/runtime/vm/os.h
File runtime/vm/os.h (right):

https://codereview.chromium.org/11414211/diff/24/runtime/vm/os.h#newcode45
runtime/vm/os.h:45: template<typename T>
On 2012/11/30 18:56:53, cshapiro wrote:
> If you are going to live with the NOLINT why not just
> 
> template<typename T>
> static T* AllocateAlignedArray(intptr_t n, intptr_t alignment) {
>   intptr_t size = n * sizeof(T);  // NOLINT
>   return reinterpret_cast<T*>(OS::AlignedAllocate(size, alignment));
> }
> 
> or avoid it
> 
> template<typename T>
> static T* AllocateAlignedArray(intptr_t n, intptr_t alignment) {
>   T* result = reinterpret_cast<T*>(OS::AlignedAllocate(n * sizeof(*result),
>                                                        alignment));
>   return result;
> }
> 
> Either way, please add spaces around *
> 

Done.

[cshapiro, 2012-11-30 19:30:39]

lgtm

ship it!

[Cutch, 2012-11-30 20:34:49]

On 2012/11/30 19:30:39, cshapiro wrote:
> lgtm
> 
> ship it!

Test configurations: none_vm_debug_ia32 none_vm_release_ia32 none_vm_debug_x64
none_vm_release_x64
[24:06 | 100% | +24033 | -    0]

[siva, 2012-11-30 22:15:53]

DBC

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:394: uint16_t* bytes =
OS::AllocateAlignedArray<uint16_t>(
The name 'byte' here is not very readable,
why not call it 'external_array' or 'ext_array'

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h
File runtime/vm/os.h (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h#newcode50
runtime/vm/os.h:50: }
This function here looks pretty ugly and is not strictly
an OS api.

I would move it out from here to byte_array.cc as a static
function in that file.

[cshapiro, 2012-11-30 22:18:11]

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:394: uint16_t* bytes =
OS::AllocateAlignedArray<uint16_t>(
The object.h class calls this value data

:-)

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h
File runtime/vm/os.h (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h#newcode50
runtime/vm/os.h:50: }
It was my idea to stuff this in here.  I would have preferred the template
interface be the only interface to allocating.  I suppose we could make the
aligned stuff below private and have this be the interface.  (Otherwise, you
have to cast all over the place.)

[siva, 2012-11-30 22:33:21]

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:394: uint16_t* bytes =
OS::AllocateAlignedArray<uint16_t>(
Sure data is fine too.

On 2012/11/30 22:18:11, cshapiro wrote:
> The object.h class calls this value data
> 
> :-)

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h
File runtime/vm/os.h (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h#newcode50
runtime/vm/os.h:50: }
The only use of this currently seems to be in byte_array.cc

why not wait for more use cases before deciding which way to go.

It would be nice if the API was similar to malloc (we have code which
does cast the result of malloc based on the type).

On 2012/11/30 22:18:11, cshapiro wrote:
> It was my idea to stuff this in here.  I would have preferred the template
> interface be the only interface to allocating.  I suppose we could make the
> aligned stuff below private and have this be the interface.  (Otherwise, you
> have to cast all over the place.)

[Cutch, 2012-11-30 22:34:27]

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h
File runtime/vm/os.h (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h#newcode50
runtime/vm/os.h:50: }
This is how the code was originally structured. The ArrayAllocation method was
private to byte_array.cc
On 2012/11/30 22:33:21, siva wrote:
> The only use of this currently seems to be in byte_array.cc
> 
> why not wait for more use cases before deciding which way to go.
> 
> It would be nice if the API was similar to malloc (we have code which
> does cast the result of malloc based on the type).
> 
> On 2012/11/30 22:18:11, cshapiro wrote:
> > It was my idea to stuff this in here.  I would have preferred the template
> > interface be the only interface to allocating.  I suppose we could make the
> > aligned stuff below private and have this be the interface.  (Otherwise, you
> > have to cast all over the place.)
>

[cshapiro, 2012-11-30 22:35:53]

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h
File runtime/vm/os.h (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/vm/os.h#newcode50
runtime/vm/os.h:50: }
We also have code that uses new and delete which handles types in a less clumsy
fashion than malloc.  That is used in places where memory that is truly an array
is allocated.

The interface we have is close to memalign (which provides the additional
alignment parameter not present in malloc).

[Ivan Posva, 2012-12-02 14:17:55]

Also please fix your setup to include reviews@dartlang.org in the CC line.

-Ivan

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc
File runtime/lib/byte_array.cc (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc#...
runtime/lib/byte_array.cc:288: DEFINE_NATIVE_ENTRY(Int8Array_newTransferrable,
1) {
The spelling transferable is generally preferred, especially since the DOM APIs
only use on r:
http://dev.w3.org/html5/spec/common-dom-interfaces.html#transferable-objects

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.dart
File runtime/lib/byte_array.dart (right):

https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.dar...
runtime/lib/byte_array.dart:10: /* patch */ factory Int8List.transferrable(int
length) {
ditto

[Cutch, 2012-12-02 19:14:54]

https://codereview.chromium.org/11434077

On 2012/12/02 14:17:55, Ivan Posva wrote:
> Also please fix your setup to include mailto:reviews@dartlang.org in the CC
line.
> 
> -Ivan
> 
> https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc
> File runtime/lib/byte_array.cc (right):
> 
>
https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.cc#...
> runtime/lib/byte_array.cc:288: DEFINE_NATIVE_ENTRY(Int8Array_newTransferrable,
> 1) {
> The spelling transferable is generally preferred, especially since the DOM
APIs
> only use on r:
> http://dev.w3.org/html5/spec/common-dom-interfaces.html#transferable-objects
> 
> https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.dart
> File runtime/lib/byte_array.dart (right):
> 
>
https://codereview.chromium.org/11414211/diff/5024/runtime/lib/byte_array.dar...
> runtime/lib/byte_array.dart:10: /* patch */ factory Int8List.transferrable(int
> length) {
> ditto