Implement %TypedArray%.prototype.slice

👁 Image	ziyunfei Reporter
	Description • 11 years ago

No description provided.

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 1 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 2 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 3 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 4 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Updated • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 5 • 11 years ago

Spec, for reference: https://people.mozilla.org/~jorendorff/es6-draft.html#sec-%typedarray%.prototype.slice

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 6 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 7 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 8 • 11 years ago

/r/3883 - Bug 1121935 - Part 1: add intrinsic for retrieving original builtin constructors. r=waldo /r/3885 - Bug 1121935: Implement SpeciesConstructor stub for now /r/3887 - Bug 1121935: Implement %TypedArray%.slice() /r/3889 - Bug 1121935: Add tests for %TypedArray%.slice() Pull down these commits: hg pull review -r f1cb7cdc725e6475b01a1bc84487c198e09e6eed

👁 Image	Tom S. (please needinfo tschuster)
	Updated • 11 years ago

👁 Image	Tom S. (please needinfo tschuster)
	Updated • 11 years ago

👁 Image	Tom S. (please needinfo tschuster)
	Updated • 11 years ago

👁 Image	Tom S. (please needinfo tschuster)
	Updated • 11 years ago

👁 Image	Tom S. (please needinfo tschuster)
	Updated • 11 years ago

👁 Image	Tom S. (please needinfo tschuster)
	Comment 9 • 11 years ago

Comment on attachment 8564485 [details] MozReview Request: bz://1121935/djc https://reviewboard.mozilla.org/r/3881/#review3079 ::: js/src/builtin/TypedArray.js (Diff revision 1) > +function TypedArraySlice(start, end = undefined) { end = undefined is wrong. The spec says: "The length property of the slice method is 2." ::: js/src/tests/ecma_6/TypedArray/slice.js (Diff revision 1) > + assertDeepEq(constructor.prototype.slice.length, 1); Would be 2 after the change above. ::: js/src/tests/ecma_6/TypedArray/slice.js (Diff revision 1) > + I would like to see a few tests where start and/or end are outside of the bounds. Like: new constructor([1, 2]).slice(-3) // [1, 2] new constructor([1, 2]).slice(0, -3) // [] ? I think ::: js/src/builtin/TypedArray.js (Diff revision 1) > + // Steps 18a-e. We usually do 18.a-e. ::: js/src/tests/ecma_6/TypedArray/slice.js (Diff revision 1) > + }, TypeError, "Assert that reverse fails if this value is not a TypedArray"); Nit: s/reverse/slice/ ::: js/src/tests/ecma_6/TypedArray/slice.js (Diff revision 1) > + We should have a test where we test some of the SpeciesConstructor bits that we have already implemented. var x = new constructor; x.constructor = undefined; slice.call({constructor: 123}); should work x.constructor = "not a constructor"; .. slice .. x.constructor = Math.sin; // also not a constructor .. slice .. should throw

👁 Image	Tom S. (please needinfo tschuster)
	Comment 10 • 11 years ago

Looks really good, I would just like to see tests for a few more edge cases.

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 11 • 11 years ago

Comment on attachment 8564485 [details] MozReview Request: bz://1121935/djc /r/3883 - Bug 1121935 - Part 1: add intrinsic for retrieving original builtin constructors. r=waldo /r/3885 - Bug 1121935: Implement SpeciesConstructor stub for now /r/3887 - Bug 1121935: Implement %TypedArray%.slice() /r/3889 - Bug 1121935: Add tests for %TypedArray%.slice() Pull down these commits: hg pull review -r 3e4ca896fc349bbef9a81bece1a89ff5cb3b935e

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 12 • 11 years ago

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ddebbe02dc59

👁 Image	Tom S. (please needinfo tschuster)
	Comment 13 • 11 years ago

Comment on attachment 8564485 [details] MozReview Request: bz://1121935/djc https://reviewboard.mozilla.org/r/3881/#review3087 Ship It! ::: js/src/tests/ecma_6/TypedArray/slice.js (Diff revision 2) > + undefConstructor.slice(123); It would be good to actually make sure something sensible happens. (You can fix this before landing, no new review required) var undefConstructor = new constructor(2); undefinedConstructor.constructor = undefined; assertEq(undefConstructor.slice(1), new constructor(1))

👁 Image	Tom S. (please needinfo tschuster)
	Comment 14 • 11 years ago

I think you have to wait on bug 1121936, because that bug contains the patch for GetBuiltinConstructor, which is still waiting for review.

👁 Image	Till Schneidereit [:till]
	Comment 15 • 11 years ago

Moved over from bug 1121936.

👁 Image	Till Schneidereit [:till]
	Updated • 11 years ago

👁 Image	Tom S. (please needinfo tschuster)
	Comment 16 • 11 years ago

Landed this! Thanks for all your effort, and I am sorry this was a little bit more difficult to get organized. https://hg.mozilla.org/integration/mozilla-inbound/rev/b5ec2e74a50c https://hg.mozilla.org/integration/mozilla-inbound/rev/ae1809cd39e3

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 17 • 11 years ago

No worries, I think this was my smoothest patch cycle yet. Thanks for all the support!

👁 Image	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/ae1809cd39e3 https://hg.mozilla.org/mozilla-central/rev/b5ec2e74a50c

👁 Image	Florian Scholz (Open Web Docs)
	Comment 19 • 11 years ago

https://developer.mozilla.org/en-US/Firefox/Releases/38#JavaScript https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/slice

👁 Image	Alice0775 White
	Updated • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 20 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 21 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 22 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 23 • 11 years ago

👁 Image	Dirkjan Ochtman (:djc) Assignee
	Comment 24 • 11 years ago

👁 Image	Lars T Hansen [:lth]
	Updated • 10 years ago