-
Notifications
You must be signed in to change notification settings - Fork 903
further improve 'arguments' support #2142
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
Conversation
|
This looks good, but it feels like it's only partway there, but not quite. If you're altering
I am also not quite clear on why we need another subclass of Error handling feels like an area we should come up with a plan to tidy up. At the moment we throw internal errors and record their type based on a name, but all those constructors are per realm, so we should probably come up with a plan to fix that. Since the |
@aardvark179 hehe it is a draft... ;-) will try to finish my changes first, and then i will have a look at your suggestions. As in many other areas we have to go a long way to reach the spec |
|
This looks good to me -- thanks. I'm idly curious whether the "read only arguments" behavior could have been implemented by making the object's properties non-configurable and making the object non-extensible. Are there other JS objects that are supposed to silently ignore attempts to set properties like this one, or is arguments special? |
… a separate ThrowTypeError in Arguments. This also fixes the setup of the ThrowTypeError returned by ScriptRuntime.typeErrorThrower(cx).
All test passing with the code base before the first 'argument' changes (commit 3f4e1a7)
d711c00 to
d40498f
Compare
d40498f to
1425a5b
Compare
at least the property descriptor shows that they are configurable (see the last added test) |
|
Thanks for the progress on this! |
Use the existing ScriptRuntime.typeErrorThrower(cx) instead of having a separate ThrowTypeError in Arguments.
This also fixes the setup of the ThrowTypeError returned by ScriptRuntime.typeErrorThrower(cx).