Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2096873002:
Amends the TypedArray constructor to use the path for primitives for all (Closed)
|
Created:
4 years, 5 months ago by bakkot Modified:
4 years, 5 months ago Reviewers:
Dan Ehrenberg 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. |
DescriptionAmends the TypedArray constructor to use the path for primitives for all
types of primitives, not just undefined, booleans, numbers, and strings.
(The missing cases were null and Symbol.) This is required by the
specification, and there are test262 tests which we were failing due to
this bug.
BUG=v8:5124
Committed: https://crrev.com/f788bd9cce19815cba746e47bb65abfe25c16208
Committed: https://crrev.com/f772c22cd1c492aa0235a8e6012d0386146d2eb2
Cr-Original-Commit-Position: refs/heads/master@{#37234}
Cr-Commit-Position: refs/heads/master@{#37407}
Patch Set 1 #
Total comments: 2
Patch Set 2 : refactor 'primitive' to 'not object' #Patch Set 3 : actual is object test #Patch Set 4 : rebase #
Messages
Total messages: 31 (12 generated)
bakkot@google.com changed reviewers: + littledan@chromium.org
https://codereview.chromium.org/2096873002/diff/1/src/js/typedarray.js File src/js/typedarray.js (right): https://codereview.chromium.org/2096873002/diff/1/src/js/typedarray.js#newcod... src/js/typedarray.js:267: IS_SYMBOL(arg1) || IS_NULL(arg1)) { This will become outdated next time a new primitive type is added (e.g., SIMD.js). Maybe instead rephrase this code to put this as the "else" case, and the current else case would be if (IS_OBJECT(arg1)) ?
https://codereview.chromium.org/2096873002/diff/1/src/js/typedarray.js File src/js/typedarray.js (right): https://codereview.chromium.org/2096873002/diff/1/src/js/typedarray.js#newcod... src/js/typedarray.js:267: IS_SYMBOL(arg1) || IS_NULL(arg1)) { On 2016/06/23 22:03:33, Dan Ehrenberg wrote: > This will become outdated next time a new primitive type is added (e.g., > SIMD.js). Maybe instead rephrase this code to put this as the "else" case, and > the current else case would be if (IS_OBJECT(arg1)) ? Done.
lgtm
The CQ bit was checked by bakkot@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2096873002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux_arm64_rel_ng on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_arm64_rel_ng_triggered on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng_trigg...)
The CQ bit was checked by bakkot@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2096873002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by bakkot@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2096873002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Amends the TypedArray constructor to use the path for primitives for all types of primitives, not just undefined, booleans, numbers, and strings. (The missing cases were null and Symbol.) This is required by the specification, and there are test262 tests which we were failing due to this bug. BUG=v8:5124 ========== to ========== Amends the TypedArray constructor to use the path for primitives for all types of primitives, not just undefined, booleans, numbers, and strings. (The missing cases were null and Symbol.) This is required by the specification, and there are test262 tests which we were failing due to this bug. BUG=v8:5124 Committed: https://crrev.com/f788bd9cce19815cba746e47bb65abfe25c16208 Cr-Commit-Position: refs/heads/master@{#37234} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/f788bd9cce19815cba746e47bb65abfe25c16208 Cr-Commit-Position: refs/heads/master@{#37234}
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2091693004/ by machenbach@chromium.org. The reason for reverting is: [Sheriff] Breaks layout tests. Please rebase upstream if intended: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/bui....
The CQ bit was checked by bakkot@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
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 bakkot@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from littledan@chromium.org Link to the patchset: https://codereview.chromium.org/2096873002/#ps60001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Amends the TypedArray constructor to use the path for primitives for all types of primitives, not just undefined, booleans, numbers, and strings. (The missing cases were null and Symbol.) This is required by the specification, and there are test262 tests which we were failing due to this bug. BUG=v8:5124 Committed: https://crrev.com/f788bd9cce19815cba746e47bb65abfe25c16208 Cr-Commit-Position: refs/heads/master@{#37234} ========== to ========== Amends the TypedArray constructor to use the path for primitives for all types of primitives, not just undefined, booleans, numbers, and strings. (The missing cases were null and Symbol.) This is required by the specification, and there are test262 tests which we were failing due to this bug. BUG=v8:5124 Committed: https://crrev.com/f788bd9cce19815cba746e47bb65abfe25c16208 Committed: https://crrev.com/f772c22cd1c492aa0235a8e6012d0386146d2eb2 Cr-Original-Commit-Position: refs/heads/master@{#37234} Cr-Commit-Position: refs/heads/master@{#37407} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/f772c22cd1c492aa0235a8e6012d0386146d2eb2 Cr-Commit-Position: refs/heads/master@{#37407}
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2120763002/ by hablich@chromium.org. The reason for reverting is: Speculative revert to unblock roll https://codereview.chromium.org/2114113002/. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
