-
Notifications
You must be signed in to change notification settings - Fork 472
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
Conversation
There was a problem hiding this 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
test/built-ins/Object/defineProperties/typedarray-backed-by-resizable-buffer.js
Outdated
Show resolved
Hide resolved
test/built-ins/Object/defineProperties/typedarray-backed-by-resizable-buffer.js
Outdated
Show resolved
Hide resolved
test/built-ins/TypedArray/prototype/slice/coerced-start-grow.js
Outdated
Show resolved
Hide resolved
The latest commit addresses all the review comments, and additionally simplifies superfluous |
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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) }); |
There was a problem hiding this comment.
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
ead3443
to
3859c2f
Compare
…t from each test file.
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.
3859c2f
to
6854f9a
Compare
Integrates
harness/resizableArrayBufferUtils.js
helper to the rest of the test files in PR #3888.