Tom S [:evilpie] Reporter
	Description • 10 years ago

This is specified at:
https://people.mozilla.org/~jorendorff/es6-draft.html#sec-%typedarray%.prototype.lastindexof
https://people.mozilla.org/~jorendorff/es6-draft.html#sec-%typedarray%.prototype.indexof
You can look at Bug #1078975 to get a general idea how this works.

So what you have to do is:
- Take a look at Array.js and copy the code for indexof and lastindexof to TypedArray.js
- Add the code for "This function is not generic" like it's done in TypedArrayFind.
- Add a reference to the functions in TypedArray.cpp in the TypedArrayObject::protoFunctions array. You can take "find" as a reference again.
- Add the function to the array in test_xrayToJS.xul, like it's done for Bug 1078975.
- Write some tests

	Hubert B Manilla
	Comment 1 • 10 years ago

i would love to work on this!

	Till Schneidereit [:till]
	Comment 2 • 10 years ago

(In reply to Hubert B Manilla from comment #1)
> i would love to work on this!

Excellent, please jump right in!

If you have any questions, ask here or join us in the #jsapi channel on irc.mozilla.org.

	Tom S [:evilpie] Reporter
	Comment 4 • 10 years ago

Comment on attachment 8535999 [details] [diff] [review]
bug-1107601-v1.patch

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

::: js/src/builtin/TypedArray.js
@@ +92,5 @@
> +    if (len === 0)
> +        return -1;
> +
> +    // Steps 7-8.
> +    var n =  ToInteger(fromIndex);

One space to many.

::: js/src/tests/ecma_6/TypedArray/indexOf_lastIndexOf.js
@@ +1,1 @@
> +const constructors = [

Please rename this file to indexOf-and-lastIndexOf.js.

I really like your tests \o/

@@ +31,5 @@
> +    assertEq(new constructor([1, 2, 1, 2, 1]).indexOf(1, -2), 4);
> +
> +    // throws if `this` isn't a TypedArray
> +    var nonTypedArrays = [undefined, null, 1, false, "", Symbol(), [], {}, /./,
> +                         /* new Proxy(new constructor(), {}) // this should throw */

Mhm yeah interesting. Reminds me of bug 1096753, but ecma apparently decided that this is okay at least for Array.isArray. We should probably at least have a bug for this.

@@ +40,5 @@
> +        }, TypeError, "Assert that indexOf fails if this value is not a TypedArray");
> +    });
> +
> +    // test that this.length is never called
> +    assertEq(Object.defineProperty(new Int8Array([0, 1, 2, 3, 5]), "length", {

new constructor

@@ +83,5 @@
> +        get() {
> +            throw new Error("length accessor called");
> +        }
> +    }).lastIndexOf(1), 1);
> +}

I would like to see some tests for Float32Array/Float64Array that look for different floats. Especially correct NaN handling.

	Till Schneidereit [:till]
	Comment 5 • 10 years ago

(In reply to Tom Schuster [:evilpie] from comment #4)
> > +    // throws if `this` isn't a TypedArray
> > +    var nonTypedArrays = [undefined, null, 1, false, "", Symbol(), [], {}, /./,
> > +                         /* new Proxy(new constructor(), {}) // this should throw */
> 
> Mhm yeah interesting. Reminds me of bug 1096753, but ecma apparently decided
> that this is okay at least for Array.isArray. We should probably at least
> have a bug for this.

I think the situation is somewhat different for typed arrays, maybe. They give the developer certain guarantees such as "assigning (a value of the right type) to an index within the range cannot throw", same for accessing indices. Crucially, in this case we're the developer, and we make use of just those guarantees in the implementation. If we let proxies through here, we lose them.

I'd argue that people who want to have generic list methods apply to all of their instances, no matter what their implementation strategy might be, should just use the Array methods.

	Tom S [:evilpie] Reporter
	Comment 7 • 10 years ago

Comment on attachment 8536019 [details] [diff] [review]
review comment addressed

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

Thank you! You already have quite a lot of experience, so please open your own bugs (e.g. there are still a lot of TypedArray methods without a bug) instead of using mentored bugs in the future.

	ziyunfei Assignee
	Comment 8 • 10 years ago

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=215a035fbb83

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/d70b753290cc

	Nigel Babu [:nigelb]
	Comment 10 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/d70b753290cc

	Florian Scholz (Open Web Docs)
	Comment 11 • 9 years ago

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/indexOf
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/lastIndexOf
https://developer.mozilla.org/en-US/Firefox/Releases/37