Add Array.findLast and findLastIndex from ES2023

[andreabergia]

Also implement it for the typed arrays variants.
There are no test262 changes because our version of test262 is old and does not contain them yet. 🙂

The tests I've added are basically adaptation of the existing tests for find and findIndex.

Should fix #1241.

[p-bakker]

There are no test262 changes because our version of test262 is old

Maybe rebase and then merge after #1540 is merged then

[gbrail]

Yes indeed -- there are now test262 tests that will run these methods and it looks like many of them will pass. Can you please rebase to the latest to pick up the new test262 updates (don't forget to "git submodule update") and update the properties file? Thanks!

[andreabergia]

Rebased on top of master - now passes some of the new test262 cases (it seems the same set of tests passed by find and findIndex).

[gbrail]

Thanks -- this is great progress.

Now, I see that we can mark a lot of the test262 tests as passing now, but looking in test262.properties under prototype/findLastIndex and prototype/findLast, there are still lots of tests that fail.

Have you been able to go through those and see if we can make more of them pass? Now that we added a new feature, it'd be important to either make all of them pass or understand why some of them still fail (usually because of unimplemented stuff elsewhere in Rhino).

[p-bakker]

@andreabergia just FYI: saw you recently submitted several PRs related to (Typed)Arrays. I've scourged the backlog and tagged all cases related to arrays with the Arrays label

[p-bakker]

To what extent are the testcases for which tests are included in this PR not already covered by test262?

iMHO we do not need to duplicate testcases already covered by test262 with own tests

[rbri]

iMHO we do not need to duplicate testcases already covered by test262 with own tests

At least i find id simpler to debug this extra cases instead of the 262 ones

[gbrail]

I too have found it useful to have a set of simple test cases to get a few feature up and running, and then use test262 to get all the details right later. I don't mind if those kinds of test cases get checked in.

[gbrail]

BTW this one needs one more rebase... Sorry but things are moving relatively fast!

[andreabergia]

Thanks -- this is great progress.

Now, I see that we can mark a lot of the test262 tests as passing now, but looking in test262.properties under prototype/findLastIndex and prototype/findLast, there are still lots of tests that fail.

Have you been able to go through those and see if we can make more of them pass? Now that we added a new feature, it'd be important to either make all of them pass or understand why some of them still fail (usually because of unimplemented stuff elsewhere in Rhino).

It's not obvious from the PR, but if you look at the file you can see that the test cases that don't pass for the new methods findLast/findLastIndex, also don't pass for the "old" methods find/findIndex:

    prototype/findLast/callbackfn-resize-arraybuffer.js
    prototype/findLast/not-a-constructor.js {unsupported: [Reflect.construct]}
    prototype/findLast/predicate-call-this-strict.js strict
    prototype/findLast/resizable-buffer.js
    prototype/findLast/resizable-buffer-grow-mid-iteration.js
    prototype/findLast/resizable-buffer-shrink-mid-iteration.js
    prototype/find/callbackfn-resize-arraybuffer.js
    prototype/find/not-a-constructor.js {unsupported: [Reflect.construct]}
    prototype/find/predicate-call-this-strict.js strict
    prototype/find/resizable-buffer.js
    prototype/find/resizable-buffer-grow-mid-iteration.js
    prototype/find/resizable-buffer-shrink-mid-iteration.js

I haven't really investigated them, honestly.

To what extent are the testcases for which tests are included in this PR not already covered by test262?

I literally copied those for find/findIndex and updated them as necessary. As the others have said, I also find them much easier to debug and to investigate.

[p-bakker]

The quoted failures boils down to:

• issues in our strict mode implementation
• not yet implemented resizable ArrayBuffers
• Not yet implemented Reflect

For all these we have cases

But there also seen to be changes to the test results of Array.prototype.flat/flatMap, which doesn't look correct I'd say

[andreabergia]

But there also seen to be changes to the test results of Array.prototype.flat/flatMap, which doesn't look correct I'd say

That was just an issue with the last rebase 🙂 Hopefully the CI build will be clean now

[p-bakker]

Indeed looks good now

Only question I have left is: why are there more test failing for the same method on TypedArray vs. regular Array? For example:

• name.js
• length.js
• prop-desc.js
• this-is-not-xxxxx.js

If there's a valid reason for those to fail currently, are there already cases to address them? If not, I think we should create those

[andreabergia]

Only question I have left is: why are there more test failing for the same method on TypedArray vs. regular Array? For example:

• name.js
• length.js
• prop-desc.js
• this-is-not-xxxxx.js

If there's a valid reason for those to fail currently, are there already cases to address them? If not, I think we should create those

The common problem that many tests on TypedArray have is that we lack one level of prototype - this returns undefined in rhino but an object in node/jsc:

Object.getPrototypeOf(Int8Array).prototype

The reason is that all the typed array classes should have the same prototype, i.e. the following should evaluate to true, but it does not in Rhino:

Object.getPrototypeOf(Int8Array).prototype !== undefined && 
  Object.getPrototypeOf(Int8Array).prototype == Object.getPrototypeOf(Uint8Array).prototype

I've looked a bit into this and it does not look trivial to fix.

[p-bakker]

Whether it's trivial or not idk, but pretty sure we don't have a case for that yet. Looks like the TypedArray intrinsic/prototype ought to provide stuff like for example the from and of methods. I assume we're lacking those as well?

[andreabergia]

Whether it's trivial or not idk, but pretty sure we don't have a case for that yet. Looks like the TypedArray intrinsic/prototype ought to provide stuff like for example the from and of methods. I assume we're lacking those as well?

Exactly - all the methods, in rhino, are defined on Int8Array.prototype.find instead of that object's prototype (i.e. Object.getPrototypeOf(Int8Array.prototype)) from what I understand and I can see playing with node/jsc. The spec is definitely a bit twisted here. 🙂

I've created #1565

[gbrail]

It looks to me like this is a good PR and that it goes as far as we can without a more thorough rewrite of the native array stuff. Unless there are some objections I'd like to merge it. Thanks for all the work on this!