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

Use CopyNameAndLength in Snapshot.wrap #86

Merged
merged 1 commit into from
May 4, 2024

Conversation

andreubotella
Copy link
Member

This uses the CopyNameAndLength AO defined in the ShadowRealm proposal to address the unexpected behavior of the length property discovered in https://github.com/tc39/test262/pull/3874/files#r1501253279.

This uses the `CopyNameAndLength` AO defined in the ShadowRealm
proposal to address the unexpected behavior of the `length` property
discovered in https://github.com/tc39/test262/pull/3874/files#r1501253279.
@andreubotella
Copy link
Member Author

While working on the engine262 and test262 changes, I noticed that this runs into a spec assertion: CreateBuiltinFunction already calls SetFunctionName and SetFunctionLength, and both of those functions assert that the function object doesn't already have the "name"/"length" property, so calling them again from CopyNameAndLength wouldn't be allowed.

Neither ShadowRealm's WrappedFunctionCreate nor Function.prototype.bind use CreateBuiltinFunction, instead making a new basic object with a [[Call]] method. I wonder if we should do the same, or maybe make a CreateRawBuiltinFunction that doesn't set the length and name.

@legendecas legendecas merged commit b1d445b into tc39:master May 4, 2024
5 checks passed
@legendecas
Copy link
Member

@andreubotella ah, you are right, https://tc39.es/ecma262/#sec-setfunctionname asserts that the function object has no "name" own property. Setting name twice would run into that assert.

@andreubotella andreubotella deleted the wrap-copy-name-and-length branch May 4, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants