-
Notifications
You must be signed in to change notification settings - Fork 3
Use internal slot to store the stack trace #14
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
base: main
Are you sure you want to change the base?
Conversation
spec.emu
Outdated
| 1. Let _setter_ be CreateBuiltinFunction(_setterClosure_, 1, "", « »). | ||
| 1. Perform ? DefinePropertyOrThrow(_O_, *"stack"*, PropertyDescriptor { [[Get]]: _getter_, [[Set]]: _setter_, [[Enumerable]]: false, [[Configurable]]: true }). | ||
| 1. If ? IsExtensible(_O_) is *false*, throw a *TypeError* exception. | ||
| 1. Define a new [[ErrorStack]] internal slot on _O_. |
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.
This doesn't match the [[StackTrace]] used by the getter.
I'm actually surprised we don't have a table of internal slots defined by the spec.
| <h1>%ErrorCaptureStackTraceGetter% ( )</h1> | ||
| <p>The value of %ErrorCaptureStackTraceGetter% is a built-in function that takes no arguments. It performs the following steps when called:</p> |
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'm not sure how it works editorially, but I think we need to at least define the observable name of these accessors.
| 1. Perform ? DefinePropertyOrThrow(_O_, *"stack"*, PropertyDescriptor { [[Get]]: _getter_, [[Set]]: _setter_, [[Enumerable]]: false, [[Configurable]]: true }). | ||
| 1. If ? IsExtensible(_O_) is *false*, throw a *TypeError* exception. | ||
| 1. Define a new [[ErrorStack]] internal slot on _O_. | ||
| 1. Perform ? DefinePropertyOrThrow(_O_, *"stack"*, PropertyDescriptor { [[Get]]: %ErrorCaptureStackTraceGetter%, [[Set]]: %ErrorCaptureStackTraceSetter%, [[Enumerable]]: false, [[Configurable]]: true }). |
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 just realized that the accessor is not enumerable where the data property is, any reason to have this difference?
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.
Thank you for catching that, the accessor is not enumerable in the V8 implementation: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/execution/messages.cc;l=1233;drc=6b98c207facca73446d15164ad3f9a62f4b41d11 and the data property is non-enumerable in SpiderMonkey/JSC: https://searchfox.org/firefox-main/rev/48f8a74041be2973d7df8ded3ad5e64c2c00e8dd/js/src/vm/ErrorObject.cpp#1082-1088.
I'll fix the data property in the spec.
Fixes #12