Skip to content

Editorial: narrow some AO return types and parameter types#3826

Merged
ljharb merged 1 commit intomainfrom
narrowings
Apr 29, 2026
Merged

Editorial: narrow some AO return types and parameter types#3826
ljharb merged 1 commit intomainfrom
narrowings

Conversation

@michaelficarra
Copy link
Copy Markdown
Member

No description provided.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 9, 2026

The rendered spec preview for this PR is available as a single page at https://tc39.es/ecma262/pr/3826 and as multiple pages at https://tc39.es/ecma262/pr/3826/multipage .

Comment thread spec.html Outdated
Comment thread spec.html
Comment thread spec.html Outdated
Comment thread spec.html Outdated
Copy link
Copy Markdown
Member

@linusg linusg left a comment

Choose a reason for hiding this comment

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

Excellent stuff, some comments

Comment thread spec.html
Comment thread spec.html Outdated
Comment thread spec.html Outdated
Comment thread spec.html
@michaelficarra michaelficarra added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Apr 27, 2026
@ljharb ljharb merged commit c3fb529 into main Apr 29, 2026
10 checks passed
@ljharb ljharb deleted the narrowings branch April 29, 2026 06:03
Comment thread spec.html
1. Let _isArray_ be ? IsArray(_value_).
1. If _isArray_ is *true*, return ? SerializeJSONArray(_state_, _value_).
1. If _isArray_ is *true*, then
1. Assert: _value_ is an Array. _value_ is not a Proxy.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add this assert on engine262 breaks the following test262 tests:

  • staging/sm/RegExp/match-trace.js
  • built-ins/JSON/stringify/value-array-proxy.js
  • built-ins/JSON/stringify/value-array-abrupt.js

Is this correct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, we actually suspect this is incorrect and will be sending a PR to correct it soon.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See #3842.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editorial change ready to merge Editors believe this PR needs no further reviews, and is ready to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants