Implement %TypedArray%.prototype.reverse

RESOLVED FIXED in mozilla37

Status

()

Core
JavaScript: Standard Library
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ziyunfei, Assigned: ziyunfei)

Tracking

({dev-doc-complete})

Trunk
mozilla37
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [DocArea=JS])

Attachments

(1 attachment, 3 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8537168 [details] [diff] [review]
bug-1111516-v1.patch
Attachment #8537168 - Flags: review?(evilpies)
(Assignee)

Comment 2

3 years ago
Created attachment 8537655 [details] [diff] [review]
added a missed )
Attachment #8537168 - Attachment is obsolete: true
Attachment #8537168 - Flags: review?(evilpies)
Attachment #8537655 - Flags: review?(evilpies)
(Assignee)

Comment 3

3 years ago
Created attachment 8537839 [details] [diff] [review]
added `if (typeof newGlobal === "function"){...}`
Attachment #8537655 - Attachment is obsolete: true
Attachment #8537655 - Flags: review?(evilpies)
Attachment #8537839 - Flags: review?(evilpies)
Sorry, I will try to review this in 24hours.
Comment on attachment 8537839 [details] [diff] [review]
added `if (typeof newGlobal === "function"){...}`

Review of attachment 8537839 [details] [diff] [review]:
-----------------------------------------------------------------

Looks perfect, can't really think of anything, but adding a few more step annotations.

::: js/src/builtin/TypedArray.js
@@ +175,5 @@
> +    // Step 6.
> +    var middle = std_Math_floor(len / 2);
> +
> +    // Steps 7-8.
> +    // Omit some steps, since there are no holes in typed arrays.

// Especially all the HasProperty/*exists checks always succeed.

@@ +177,5 @@
> +
> +    // Steps 7-8.
> +    // Omit some steps, since there are no holes in typed arrays.
> +    for (var lower = 0, upper, temp; lower !== middle; lower++) {
> +        upper = len - lower - 1;

// a.
var upper =

@@ +178,5 @@
> +    // Steps 7-8.
> +    // Omit some steps, since there are no holes in typed arrays.
> +    for (var lower = 0, upper, temp; lower !== middle; lower++) {
> +        upper = len - lower - 1;
> +        temp = O[lower];

var lowerValue = O[lower];
var upperValue = O[upper];

// We always end up in the j. case
O[lower] = ..
...

::: js/src/tests/ecma_6/TypedArray/reverse.js
@@ +14,5 @@
> +
> +    assertDeepEq(constructor.prototype.reverse.length, 0);
> +
> +    assertDeepEq(new constructor().reverse(), new constructor());
> +    assertDeepEq(new constructor(1000).reverse(), new constructor(1000));

I think 10 is probably enough.
Attachment #8537839 - Flags: review?(evilpies) → review+
(Assignee)

Comment 6

3 years ago
Created attachment 8539002 [details] [diff] [review]
review comment addressed
Attachment #8537839 - Attachment is obsolete: true
Attachment #8539002 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/e3cce6766ed6
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.