Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1754593003:
Implement TypedArray(typedarray) constructor (Closed)
|
Created:
4 years, 9 months ago by Dan Ehrenberg Modified:
4 years, 9 months ago Reviewers:
adamk CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionImplement TypedArray(typedarray) constructor
The ES2016 draft spec defines a sort of fast path for constructing
a TypedArray based on another TypedArray. This patch implements that
alternative path in TypedArray construction. It is verified by
test262 tests, which now pass. This patch also has a slight cleanup
of TypedArray code by using a macro for TypedArray type checks, as
is done for other types.
This patch includes a minor spec violation: In the same-type case, the
spec indicates that the underlying ArrayBuffer should be copied until
the end, and this is fixed up by making the [[ArrayLength]] shorter.
This is observable with the buffer getter. This patch just copies the
used part of the underlying ArrayBuffer.
R=adamk
BUG=v8:4726
LOG=Y
Committed: https://crrev.com/2fa1c88442d83865f611efb890642d1eb643f320
Cr-Commit-Position: refs/heads/master@{#34443}
Patch Set 1 #Patch Set 2 : fix comment and test262 #
Total comments: 4
Patch Set 3 : Remove outdated comment #
Total comments: 2
Patch Set 4 : rebase #Patch Set 5 : Do SetPrototype only when needed #Patch Set 6 : use the right equality #
Messages
Total messages: 38 (19 generated)
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1754593003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1754593003/1
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1754593003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1754593003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_mac_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/16255)
lgtm once TODO is gone https://codereview.chromium.org/1754593003/diff/20001/src/js/typedarray.js File src/js/typedarray.js (right): https://codereview.chromium.org/1754593003/diff/20001/src/js/typedarray.js#ne... src/js/typedarray.js:259: %InternalSetPrototype(data, prototype); Looks like this is needed to handle ObjectCreate() with an internalSlotsList? Makes me sad, but does seem to match the spec behavior. https://codereview.chromium.org/1754593003/diff/20001/src/js/typedarray.js#ne... src/js/typedarray.js:280: // TODO(littledan): If arg1 is a TypedArray, follow the constructor Seems like this CL takes care of this TODO?
Description was changed from ========== Implement TypedArray(typedarray) constructor The ES2016 draft spec defines a sort of fast path for constructing a TypedArray based on another TypedArray. This patch implements that alternative path in TypedArray construction. It is verified by test262 tests, which now pass. This patch also has a slight cleanup of TypedArray code by using a macro for TypedArray type checks, as is done for other types. R=adamk BUG=v8:4726 LOG=Y ========== to ========== Implement TypedArray(typedarray) constructor The ES2016 draft spec defines a sort of fast path for constructing a TypedArray based on another TypedArray. This patch implements that alternative path in TypedArray construction. It is verified by test262 tests, which now pass. This patch also has a slight cleanup of TypedArray code by using a macro for TypedArray type checks, as is done for other types. This patch includes a minor spec violation: In the same-type case, the spec indicates that the underlying ArrayBuffer should be copied until the end, and this is fixed up by making the [[ArrayLength]] shorter. This is observable with the buffer getter. This patch just copies the used part of the underlying ArrayBuffer. R=adamk BUG=v8:4726 LOG=Y ==========
The CQ bit was checked by littledan@chromium.org
https://codereview.chromium.org/1754593003/diff/20001/src/js/typedarray.js File src/js/typedarray.js (right): https://codereview.chromium.org/1754593003/diff/20001/src/js/typedarray.js#ne... src/js/typedarray.js:259: %InternalSetPrototype(data, prototype); On 2016/03/01 at 18:49:46, adamk wrote: > Looks like this is needed to handle ObjectCreate() with an internalSlotsList? Makes me sad, but does seem to match the spec behavior. Yes, I'm sad too. https://codereview.chromium.org/1754593003/diff/20001/src/js/typedarray.js#ne... src/js/typedarray.js:280: // TODO(littledan): If arg1 is a TypedArray, follow the constructor On 2016/03/01 at 18:49:46, adamk wrote: > Seems like this CL takes care of this TODO? Removed it
The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org Link to the patchset: https://codereview.chromium.org/1754593003/#ps40001 (title: "Remove outdated comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1754593003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1754593003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/1...) v8_linux_dbg_ng on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/2180) v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...) v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...) v8_mac_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/16314)
https://codereview.chromium.org/1754593003/diff/40001/src/js/typedarray.js File src/js/typedarray.js (right): https://codereview.chromium.org/1754593003/diff/40001/src/js/typedarray.js#ne... src/js/typedarray.js:259: %InternalSetPrototype(data, prototype); Could we skip this in the common case where bufferConstructor === GlobalArrayBuffer? That'd save us the runtime call and some actual work.
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1754593003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1754593003/80001
https://codereview.chromium.org/1754593003/diff/40001/src/js/typedarray.js File src/js/typedarray.js (right): https://codereview.chromium.org/1754593003/diff/40001/src/js/typedarray.js#ne... src/js/typedarray.js:259: %InternalSetPrototype(data, prototype); On 2016/03/01 at 19:36:36, adamk wrote: > Could we skip this in the common case where bufferConstructor === GlobalArrayBuffer? That'd save us the runtime call and some actual work. Good idea, done.
The CQ bit was unchecked by littledan@chromium.org
The CQ bit was checked by littledan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org Link to the patchset: https://codereview.chromium.org/1754593003/#ps80001 (title: "Do SetPrototype only when needed")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1754593003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1754593003/80001
The CQ bit was unchecked by littledan@chromium.org
The CQ bit was checked by littledan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org Link to the patchset: https://codereview.chromium.org/1754593003/#ps100001 (title: "use the right equality")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1754593003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1754593003/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...) v8_linux_rel_ng on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/2250)
The CQ bit was checked by littledan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1754593003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1754593003/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by littledan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1754593003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1754593003/100001
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Implement TypedArray(typedarray) constructor The ES2016 draft spec defines a sort of fast path for constructing a TypedArray based on another TypedArray. This patch implements that alternative path in TypedArray construction. It is verified by test262 tests, which now pass. This patch also has a slight cleanup of TypedArray code by using a macro for TypedArray type checks, as is done for other types. This patch includes a minor spec violation: In the same-type case, the spec indicates that the underlying ArrayBuffer should be copied until the end, and this is fixed up by making the [[ArrayLength]] shorter. This is observable with the buffer getter. This patch just copies the used part of the underlying ArrayBuffer. R=adamk BUG=v8:4726 LOG=Y ========== to ========== Implement TypedArray(typedarray) constructor The ES2016 draft spec defines a sort of fast path for constructing a TypedArray based on another TypedArray. This patch implements that alternative path in TypedArray construction. It is verified by test262 tests, which now pass. This patch also has a slight cleanup of TypedArray code by using a macro for TypedArray type checks, as is done for other types. This patch includes a minor spec violation: In the same-type case, the spec indicates that the underlying ArrayBuffer should be copied until the end, and this is fixed up by making the [[ArrayLength]] shorter. This is observable with the buffer getter. This patch just copies the used part of the underlying ArrayBuffer. R=adamk BUG=v8:4726 LOG=Y Committed: https://crrev.com/2fa1c88442d83865f611efb890642d1eb643f320 Cr-Commit-Position: refs/heads/master@{#34443} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/2fa1c88442d83865f611efb890642d1eb643f320 Cr-Commit-Position: refs/heads/master@{#34443} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
