Skip to content

fix(built-ins): modify IsTypedArrayFixedLength implementation to match spec#5262

Merged
jedel1043 merged 9 commits intoboa-dev:mainfrom
AlvinThorn008:main
Mar 29, 2026
Merged

fix(built-ins): modify IsTypedArrayFixedLength implementation to match spec#5262
jedel1043 merged 9 commits intoboa-dev:mainfrom
AlvinThorn008:main

Conversation

@AlvinThorn008
Copy link
Copy Markdown
Contributor

This Pull Request fixes/closes #5256.

As required by ecma262, Object.preventExtensions may be applied to typed arrays without error if the IsTypedArrayFixedLength is not false. Prior to this commit, this check was incorrectly implemented via ArrayBuffer::is_fixed_len which does not/can not include an AUTO length check. This discrepancy lies in that the requirements of a fixed-length TypedArray slightly differ from a fixed-length arraybuffer.

It adds and replaces the checks as specified by sec-istypedarrayfixedlength. Now, typed_array_exotic_prevent_extensions should

  • return Ok(false) when the TypedArray is auto length or when the backing ArrayBuffer is resizable.
  • failing the above, call ordinary_prevent_extensions if the backing array is a SharedArrayBuffer.

This PR also adds a test to ensure what the title of this issue says. In addition, test262's test/staging/built-ins/preventExtensions should now pass completely.

… by GSABs

As required by ecma262, `Object.preventExtensions` may be applied to typed arrays without error if the `IsTypedArrayFixedLength` is not false. Prior to this commit, this check was incorrectly implemented via `ArrayBuffer::is_fixed_len` which does not/can not include an AUTO length check. This discrepancy lies in that the requirements of a fixed-length `TypedArray` slightly differ from a fixed-length arraybuffer.
@AlvinThorn008 AlvinThorn008 requested a review from a team as a code owner March 26, 2026 15:31
@github-actions github-actions bot added the Waiting On Review Waiting on reviews from the maintainers label Mar 26, 2026
@github-actions github-actions bot added this to the v1.0.0 milestone Mar 26, 2026
@github-actions github-actions bot added C-Tests Issues and PRs related to the tests. C-Builtins PRs and Issues related to builtins/intrinsics labels Mar 26, 2026
Copy link
Copy Markdown
Contributor

@zhuzhu81998 zhuzhu81998 left a comment

Choose a reason for hiding this comment

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

I think it would be much nicer if you implement the IsTypedArrayFixedLength operation as a function, similar to:

/// Abstract operation `IsValidIntegerIndex ( O, index )`.
///
/// Returns `true` if the index is valid, or `false` otherwise.
///
/// More information:
/// - [ECMAScript reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-isvalidintegerindex
pub(crate) fn is_valid_integer_index(obj: &JsObject, index: f64) -> JsResult<bool> {
let inner = obj.downcast_ref::<TypedArray>().js_expect(
"integer indexed exotic method should only be callable from TypedArray objects",
)?;
let buf = inner.viewed_array_buffer();
let buf = buf.as_buffer();
// 1. If IsDetachedBuffer(O.[[ViewedArrayBuffer]]) is true, return false.
// 4. Let taRecord be MakeTypedArrayWithBufferWitnessRecord(O, unordered).
// 5. NOTE: Bounds checking is not a synchronizing operation when O's backing buffer is a growable SharedArrayBuffer.
let Some(buf_len) = buf.bytes(Ordering::Relaxed).map(|s| s.len()) else {
return Ok(false);
};
Ok(inner.validate_index(index, buf_len).is_some())
}

Note that you should put the spec as comment.

@AlvinThorn008
Copy link
Copy Markdown
Contributor Author

@zhuzhu81998
That is true. I debated whether it was necessary but it's certainly more in line with the codebase's style. I'll refactor it in a couple minutes.

Thanks

…e function for use in `typed_array_exotic_prevent_extensions`.
@AlvinThorn008
Copy link
Copy Markdown
Contributor Author

I've copied the format of the function you linked. Everything seems to be working as before.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 26, 2026

Test262 conformance changes

Test result main count PR count difference
Total 52,963 52,963 0
Passed 50,732 50,735 +3
Ignored 1,426 1,426 0
Failed 805 802 -3
Panics 0 0 0
Conformance 95.79% 95.79% +0.01%
Fixed tests (3):
test/staging/built-ins/Reflect/preventExtensions/preventExtensions-variable-length-typed-arrays.js (previously Failed)
test/staging/built-ins/Object/preventExtensions/preventExtensions-variable-length-typed-arrays.js (previously Failed)
test/staging/built-ins/Object/seal/seal-variable-length-typed-arrays.js (previously Failed)

Tested main commit: cd1a386965922e796d730c056baa45561cc29011
Tested PR commit: 2f2787c72f95908cf7377f3b7bb128727e23e299
Compare commits: cd1a386...2f2787c

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 59.68%. Comparing base (6ddc2b4) to head (2f2787c).
⚠️ Report is 925 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/builtins/typed_array/object.rs 85.71% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5262       +/-   ##
===========================================
+ Coverage   47.24%   59.68%   +12.44%     
===========================================
  Files         476      589      +113     
  Lines       46892    63477    +16585     
===========================================
+ Hits        22154    37889    +15735     
- Misses      24738    25588      +850     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks great!

@AlvinThorn008
Copy link
Copy Markdown
Contributor Author

Looks great!

Wonderful! Is there anything else that need to be done to merge this PR?

@zhuzhu81998
Copy link
Copy Markdown
Contributor

Looks great!

Wonderful! Is there anything else that need to be done to merge this PR?

maybe change the PR title quickly? it is more about implementing IsTypedArrayFixedLength correctly.

@AlvinThorn008 AlvinThorn008 changed the title fix(built-ins): permit preventExtensions on fixed-length typed arrays backed by GSABs fix(built-ins): modify IsTypedArrayFixedLength implementation to match spec Mar 28, 2026
@jedel1043 jedel1043 enabled auto-merge March 29, 2026 15:36
@jedel1043 jedel1043 added this pull request to the merge queue Mar 29, 2026
Merged via the queue into boa-dev:main with commit f5e88de Mar 29, 2026
22 checks passed
@github-actions github-actions bot removed the Waiting On Review Waiting on reviews from the maintainers label Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Builtins PRs and Issues related to builtins/intrinsics C-Tests Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix: permit preventExtensions on fixed length TypedArrays backed by GSABs

3 participants