Chromium Code Reviews
Help | Chromium Project | Sign in
(1106)

Issue 9429015: Added no argument constructor support to external arrays. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 1 month ago by ioseb
Modified:
1 year, 9 months ago
CC:
v8-dev_googlegroups.com
Visibility:
Public.

Description

Added no argument constructor support to external arrays.
ArrayType.call({}), ArrayType.apply({}) and new ArrayType() are all supported now.

Modified existing no arg constructor test to assertDoesNotThrow().
Added number of new different tests.

Original issue: http://code.google.com/p/v8/issues/detail?id=1497

Contributed by ioseb.dzmanashvili@gmail.com

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+53 lines, -16 lines) Lint Patch
M src/d8.cc View 3 chunks +17 lines, -14 lines 0 comments 0 errors Download
M test/mjsunit/external-array.js View 2 chunks +36 lines, -2 lines 0 comments 0 errors Download
Trybot results:
Commit:

Messages

Total messages: 4
ioseb
2 years, 1 month ago #1
ioseb
2 years, 1 month ago #2
rossberg
Ioseb, thanks for this patch. Typed array support in d8 has been vastly improved lately. ...
1 year, 9 months ago #3
ioseb
1 year, 9 months ago #4
Hi Andreas,

You are welcome! Only reason i think it must be supported is that typed arrays
are functions(as was stated in initial issue:
http://code.google.com/p/v8/issues/detail?id=1497) and to my knowledge all JS
functions support these methods, additionally .call() and .apply() methods are
supported in FF. Safari is different though and typed arrays are objects instead
of functions there. But, when i was investigated deeper found several related
issues(details are in this comment:
http://code.google.com/p/v8/issues/detail?id=1497#c5) and gave up with this
patch because i'm not 100% sure how this feature needs to be implemented in
consistent way.

ioseb

On 2012/06/29 15:28:40, rossberg wrote:
> Ioseb, thanks for this patch. Typed array support in d8 has been vastly
improved
> lately. However, I did not incorporate the functionality you implemented in
this
> patch, because it is not covered by the actual typed array spec. Any
particular
> reason why you think this should be supported?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6