Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RAB: Integrate helper to rest of the tests #4198

Merged
merged 5 commits into from
Aug 27, 2024

Conversation

ioannad
Copy link
Contributor

@ioannad ioannad commented Aug 15, 2024

Integrates harness/resizableArrayBufferUtils.js helper to the rest of the test files in PR #3888.

@ioannad ioannad requested a review from a team as a code owner August 15, 2024 23:29
Copy link
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things to fix up, then we can finally call this done

@ioannad
Copy link
Contributor Author

ioannad commented Aug 26, 2024

The latest commit addresses all the review comments, and additionally simplifies superfluous ReadElement2 and HasElement2 in TypedArray/out-of-bounds-behaves-like-detached.js,

Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@@ -11,16 +11,14 @@ includes: [compareArray.js, resizableArrayBufferUtils.js]
features: [resizable-arraybuffer]
---*/

function ObjectDefinePropertyHelper(ta, index, value) {
function MayNeedBigInt(ta, value) {
if (ta instanceof BigInt64Array || ta instanceof BigUint64Array) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future work: MayNeedBigInt could go into the helper file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make a followup PR with this change, since this is defined across many files.

@@ -33,7 +31,7 @@ for (let ctor of ctors) {
return 0;
}
};
helper(fixedLength, evil, 8);
Object.defineProperty(fixedLength, evil, { value: MayNeedBigInt(fixedLength, 8) });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much more readable, thank you

@Ms2ger Ms2ger force-pushed the integrate-helper-to-PR3888 branch from ead3443 to 3859c2f Compare August 27, 2024 10:32
@Ms2ger Ms2ger enabled auto-merge (squash) August 27, 2024 10:34
ioannad and others added 5 commits August 27, 2024 17:15
Co-authored-by: Philip Chimento <philip.chimento@gmail.com>
In particular:
- Switching `TypedArray/out-of-bounds-has.js` and `TypedArray/out-of-bounds-get-and-set.js``
  to include ctors of any size, accommodating for BigInt ctors.
- Simplify superfluous helper definitions, renaming to a better description,
  and defined `MayNeedBigInt` also used in other RAB methods.
- Duplicating `defineProperties/typedarray-backed-by-resizable-buffer.js`
  for `defineProperty`.
- Removes previously renamed test.
@Ms2ger Ms2ger force-pushed the integrate-helper-to-PR3888 branch from 3859c2f to 6854f9a Compare August 27, 2024 15:15
@Ms2ger Ms2ger merged commit 3acb672 into tc39:main Aug 27, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants