-
Notifications
You must be signed in to change notification settings - Fork 36
Clarify that stack traces are not required #216
Conversation
| tag, the associated data fields cannot be read. | ||
|
|
||
| The `Exception` constructor can take an optional `ExceptionOptions` argument, | ||
| which can optionally contain `traceStack` entry. When `traceStack` is `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.
This is described as "optional," but the associated WebIDL doesn't use optional boolean, or (boolean | undefined)? This wording could be dropped.
| entry, `traceStack` is considered `false` by default. | ||
| JavaScript VMs are permitted to attach a stack trace string to `Exception.stack` | ||
| field, as in JavaScript's `Error` class. `traceStack` serves as a request to the | ||
| WebAssembly engine to attach a stack trace; it is not necessary to honour if `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.
While it is a request because there can be engines that don't have the stack trace capability, I'm not sure if the current text is unclear, or if I prefer to explicitly say "it is not necessary to honour", which is something we certainly don't encourage.
I'm actually not very sure why you find the current text not clear.
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.
While it is a request because there can be engines that don't have the stack trace capability, I'm not sure if the current text is unclear, or if I prefer to explicitly say "it is not necessary to honour", which is something we certainly don't encourage.
Well, if anything, the ECMAScript spec makes no demands on engines creating/exposing stack traces, i.e. Error stack is non-standard, nor do other specs directly demand it, such as DOMException, so isn't it a bit weird that the Wasm WG mandates it?
I'm actually not very sure why you find the current text not clear.
I'm hoping that it more explicitly explains the expected behaviour. Unless one is taking an extremely strict view of the wording, the current text doesn't make it immediately obvious as to how an engine is permitted to behave when given this parameter.
Although, of course, feel free to reject the changes if you don't believe that it is a net positive.
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.
Hmm, I see, I think that makes sense. By the way can you wrap the text to 80 cols? It's not a rule kept universally in wasm spec repo, but this file conforms to it, so I think it's better to be consistent.
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.
Yes, the immediate commit should have wrapped it, can you confirm?
(Although for future reference, which tool do you use, as it may differ?)
aheejin
left a comment
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.
Thanks!
Fixes #215.
This should indicate that an engine is still conforming if it does not attach a stack trace, even if
traceStackis requested by JavaScript code (by setting it totrue).