Implement various missing methods in TypedArray

[andreabergia]

Since most methods were already implemented in NativeArray, I followed this approach:

• when reasonable, I extracted the implementation in NativeArray and moved it to a new class ArrayLikeAbstractOperations;
• otherwise, I copy&pasted the code from NativeArray into NativeTypedArrayView and simplified it.

[p-bakker]

Great stuff!

Could you check whether this covers everything mentioned in #1411?
And also #1307 (or at least for TypedArrays?

And whether it maybe also fixes #699

[andreabergia]

Great stuff!

Thanks 🙂

Could you check whether this covers everything mentioned in #1411? And also #1307 (or at least for TypedArrays?)

For #1411 it implements the "old" methods, i.e. not the ones added by ES2024. Therefore it also does not implement #1307 (because I felt that it's better to do a single implementation, for both normal and typed arrays).
I also have not added BigInt64Array/ BigUint64Array (#1410).

And whether it maybe also fixes #699

It does not. I have only added new methods, not fixed existing ones.

[rbri]

Great!

• 1 for merging this!!!!

[p-bakker]

Besides makes a wack of tests from test262 pass, based on the diff of the test262.priperties file it seem it also breaks a bunch of tests that previously passed.

What's up with those?

[andreabergia]

Besides makes a wack of tests from test262 pass, based on the diff of the test262.priperties file it seem it also breaks a bunch of tests that previously passed.

Thanks for pointing this out, I somehow missed it. There were three categories of failing tests:

• the tests in built-ins/TypedArrayConstructors, which are now failing for BigInt since those kind of typed arrays are not implemented. This is coherent with the previously existing methods (say set or subarray), which were already failing.
• the tests which use detached, such as prototype/copyWithin/coerced-values-end-detached.js - this is something that should be implemented in the test 262 harness and it is not yet. Previously the tests were passing because the invoked method did not exist; now that it does, since the detaching feature does not work, the tests fails. I don't think it's super important.
• the tests for species constructor, which made me realize that my implementation of filter, map, and splice was a bit incorrect. I fixed those. 🙂

[p-bakker]

For the 2nd item,: you means something additional needs to be implemented here? If so, could you create a new case about what needs to be implemented?

[p-bakker]

The state of things in test262.properties seems ok now.

But I do see a recurring theme on failing tests for methods, like:

prototype/values/invoked-as-func.js
    prototype/values/invoked-as-method.js
    prototype/values/length.js
    prototype/values/name.js
    prototype/values/prop-desc.js
    prototype/values/this-is-not-object.js
    prototype/values/this-is-not-typedarray-instance.js

Any chance you could make a follow-up case for fixing whichever of those failing tests could be fixed by some TLC?

[andreabergia]

For the 2nd item,: you means something additional needs to be implemented here? If so, could you create a new case about what needs to be implemented?

Opened #1527

But I do see a recurring theme on failing tests for methods, like:

prototype/values/invoked-as-func.js
    prototype/values/invoked-as-method.js
    prototype/values/length.js
    prototype/values/name.js
    prototype/values/prop-desc.js
    prototype/values/this-is-not-object.js
    prototype/values/this-is-not-typedarray-instance.js

Any chance you could make a follow-up case for fixing whichever of those failing tests could be fixed by some TLC?

Created #1528

[gbrail]

I think this is a good change in general, but it still looks like we fixed many test262 tests that were previously broken, but we still broke test262 tests that previously worked. I understand the issues above about "detached" support, but I'm not excited about merging this if it breaks tests that currently work. Is there anything that we can do here, or do others want to merge this and then fix the broken tests later?

[andreabergia]

I think this is a good change in general, but it still looks like we fixed many test262 tests that were previously broken, but we still broke test262 tests that previously worked. I understand the issues above about "detached" support, but I'm not excited about merging this if it breaks tests that currently work. Is there anything that we can do here, or do others want to merge this and then fix the broken tests later?

I think the reason is clear by looking at the test case code, and not just at the test262.properties file.

---

The first category of tests are the ones about the detached, for example: test/built-ins/TypedArray/prototype/copyWithin/coerced-values-end-detached.js:

testWithTypedArrayConstructors(function(TA) {
  var ta;
  function detachAndReturnIndex(){
      $DETACHBUFFER(ta.buffer);
      return 900;
  }

  var array = [];
  array.length = 10000; // big arrays are more likely to cause a crash if they are accessed after they are freed
  array.fill(7, 0);
  ta = new TA(array);
  assert.throws(TypeError, function(){
    ta.copyWithin(0, 100, {valueOf : detachAndReturnIndex});
  }, "should throw TypeError as array is detached");
});

Previously this test was passing because the function copyWithin did not exist, so trying to invoke it throw an exception. Now, the function exists, but since we don't have the "detached buffer" feature, the tests fails. However, this PR does not really break anything - the fact that the test was passing was, well, a lucky coincidence.

---

The other category of tests now broken are those in the directory built-ins/TypedArrayConstructors, where previously the tests like test/built-ins/TypedArrayConstructors/prototype/lastIndexOf/bigint-inherited.js were passing and now are failing. They are something like this:

testWithTypedArrayConstructors(function(TA) {
  assert.sameValue(TA.prototype.hasOwnProperty("lastIndexOf"), false);
});

This fails because the functions are now defined. The hasOwnProperty should return false, but that was true also for existing functions such as set or toString - which were failing before this PR. All these tests should be fixed all at once, in a separate PR though, because there is a common problem shared between pre-existing and new functions.

[p-bakker]

Does seem like there are a bunch of odd ones that are now seemingly broken

Besides the detached buffer ones for example:

• prototype/map/speciesctor-get-species-custom-ctor-length-throws.js
• within built-ins/TypedArrayConstructors

The detached buffer ones are also strange: if they never passed, why weren't they recorded in test262.properties before? Does that mean something is amiss in our test262Suite?

So I agree this needs to be looked at before merging

[p-bakker]

@andreabergia so our comments overlapped a bit, time-wise

What about tests like prototype/map/species/ctor-get-species-custom-ctor-length-throws.js? Why are those now reported as failing?

[andreabergia]

What about tests like prototype/map/species/ctor-get-species-custom-ctor-length-throws.js? Why are those now reported as failing?

Same as the others - there are three tests like these: map, filter, slice. All of these do something like the following:

testWithTypedArrayConstructors(function(TA) {
  var sample = new TA(2);

  sample.constructor = {};
  sample.constructor[Symbol.species] = function() {
    return new TA();
  };

  assert.throws(TypeError, function() {
    sample.map(function() { return 0; });
  });
});

Before this PR, they are "passing" because the map method does not exist, and so we get a type error. Now they are "really" failing because my implementation of the Symbol.species is not 100% perfect, although it does handle quite a lot of cases.
So, again, the fact that they were passing before this PR is just misleading.

[p-bakker]

@gbrail to answer your earlier question: I think the seemingly breaking tests aren't actual breakages. Instead, due to how those test are authored, they previously returned false positives

So i think this PR is good to go, from a test262 perspective

[rbri]

agree with @p-bakker looks fine for me and another great step forward +1 for merge

and many thanks @andreabergia

[gbrail]

Thanks!

I resolved the conflict and merged this manually. Great progress!