-
Notifications
You must be signed in to change notification settings - Fork 36
Event vs. exception? #159
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
Comments
Relatedly, would we consider renaming a section in future spec releases? I do anticipate that we will have new kinds of events in the future, so either we will want to keep the name "event section" or we will want to rename the "exception section" to "event section" in the future. Personally, I think renaming a section would be unnecessarily disruptive, so I would prefer keeping the name "event section." But if we keep the name "event section," it would be weird not to use "event" terminology to describe the contents of that section as well. So I think using "event" throughout the spec is the more forward-looking option. I would be fine with the JS API using "Exception," though, since that is the only kind of event it will be dealing with for now. |
@tlively If we are going to use different terminology for the core spec (event) and the JS API (exception), is the JS API not supposed to catch other kinds of events, even if they are represented with JS exceptions? |
I would use |
The question of events is one of the design points in the Stack Switching efforts. |
@fgmccabe I think the key question here is whether stack switching will extend the event section with new kinds of events. My impression is that all of the designs discussed so far have assumed that there will indeed be new kinds of events added to the event section. Is that your impression as well? |
This is a 'design point'. I.e., don't know at the moment. |
I would call it something like the "tag" or "identifier" section. It's telling the engine to generate some unique tag and associate it with some type. The EH proposal then is using the tags to distinguish exceptions. But you could imagine the GC proposal adding something like |
"Event" is a bit overloaded and might cause confusion with completely unrelated notions of the same name, esp on the Web. Calling them "tags" sounds reasonable to me and perhaps makes clearer the distinction between a tag per se and a tagged value. |
I don't have a strong preference on this, and I'd be fine with the proposal to go with "tags". Concretely, I guess this would mean these changes:
In this case we could also rename If |
I am OK with the name @takikawa If we rename our JS API to |
@takikawa To check if I am understanding you correctly: if the result of a wasm Supposing that understanding is correct, I would change |
@RossTate What is a label type? I am not aware of that concept in wasm but you keep using the term, so I'm wondering what I'm missing. |
The type of labels, e.g. what you check against when you type-check |
@RossTate That "result type" does not represent signatures of params to results in the form of (params) -> (results); it only represents the latter. In the MVP spec term, this was called function type; now given that it is used for more than functions (it is used in blocks for multivalue and also for EH), we can't use that name though. |
For |
The entries in the event section (or the tag section) contain references to signatures, not result types, which only consist of a list of types. So naming it If we think there is a possibility to add other kinds of events in the section, I think it's best to keep the name 'event section' or rename it to 'tag section'. If we think that section as a section for general reference for tags and we are not sure future references of tags can be all represented as JS exceptions, I think it's good to use |
From @RossTate:
Yes, that matches what I was thinking.
Is the idea with your proposal that I was thinking we could potentially accommodate both with the same API. For example, there could be This is why I asked this question earlier:
Perhaps If tags no longer carry an attribute field this wouldn't work of course, so that's a design question that needs to be resolved I think. From @aheejin:
I think it could be extensible if in the future we allow the tag attribute to be set somehow, to distinguish the kind of tag. |
The point of naming it event (and renaming to tag) was to account for their potential use for other things than exception. The stack switching proposal e.g. uses them, and makes use of their ability to have function type. No good reason to abolish that ability. In any case, this issue is about naming, not changing semantics.
All sounds right to me.
The binary format already reserves space for an attribute. In other places (text format, JS API), we can defer introducing it as an explicit component until needed, defaulting to the current meaning when omitted. |
If there is no other objections then, can we proceed with #159 (comment)? |
It was suggested to change the term 'event' to 'tag' to reference the section name and the entries within the section. The suggested reasons were they can be used something other than events in future, and the term event can be confusing with other concepts on the web. Closes WebAssembly#159.
We recently decided to change 'event' to 'tag', and to 'event section' to 'tag section', out of the rationale that the section contains a generalized tag that references a type, which may be used for something other than exceptions, and the name 'event' can be confusing in the web context. See - WebAssembly/exception-handling#159 (comment) - WebAssembly/exception-handling#161
Reflected #159 (comment) in
|
We recently decided to change 'event' to 'tag', and 'event section' to 'tag section', out of the rationale that the section contains a generalized tag that references a type, which may be used for something other than exceptions, and the name 'event' can be confusing in the web context. See - WebAssembly/exception-handling#159 (comment) - WebAssembly/exception-handling#161 Reviewed By: tlively Differential Revision: https://reviews.llvm.org/D104423
We recently decided to change 'event' to 'tag', and to 'event section' to 'tag section', out of the rationale that the section contains a generalized tag that references a type, which may be used for something other than exceptions, and the name 'event' can be confusing in the web context. See - WebAssembly/exception-handling#159 (comment) - WebAssembly/exception-handling#161
We recently decided to change 'event' to 'tag', and 'event section' to 'tag section', out of the rationale that the section contains a generalized tag that references a type, which may be used for something other than exceptions, and the name 'event' can be confusing in the web context. See - WebAssembly/exception-handling#159 (comment) - WebAssembly/exception-handling#161
We recently decided to change 'event' to 'tag', and 'event section' to 'tag section', out of the rationale that the section contains a generalized tag that references a type, which may be used for something other than exceptions, and the name 'event' can be confusing in the web context. See - WebAssembly/exception-handling#159 (comment) - WebAssembly/exception-handling#161
We recently decided to change 'event' to 'tag', and 'event section' to 'tag section', out of the rationale that the section contains a generalized tag that references a type, which may be used for something other than exceptions, and the name 'event' can be confusing in the web context. See - WebAssembly/exception-handling#159 (comment) - WebAssembly/exception-handling#161
See: WebAssembly/exception-handling#159 This change only does the rename where it's observable. This should also be renamed throughout the codebase for consistency and will be done separately. [email protected] Bug: v8:8091 Change-Id: Iec1118194981dfd33be6e30256b6e72d12143e1f Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3021172 Reviewed-by: Andreas Haas <[email protected]> Commit-Queue: Thibaud Michaud <[email protected]> Cr-Commit-Position: refs/heads/master@{#75718}
This reverts commit 0b091e9. Reason for revert: Causes Web Platform Test failures, blocking roll E.g., https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Blink%20Linux/12616/overview Original change's description: > [wasm][eh] Rename Exception to Tag in the JS API > > See: > WebAssembly/exception-handling#159 > > This change only does the rename where it's observable. This should also > be renamed throughout the codebase for consistency and will be done > separately. > > R=[email protected] > > Bug: v8:8091 > Change-Id: Iec1118194981dfd33be6e30256b6e72d12143e1f > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3021172 > Reviewed-by: Andreas Haas <[email protected]> > Commit-Queue: Thibaud Michaud <[email protected]> > Cr-Commit-Position: refs/heads/master@{#75718} Bug: v8:8091 Change-Id: Id2067e1cdc33fa657ef738ef5fafad84057f7209 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3027261 Auto-Submit: Adam Klein <[email protected]> Commit-Queue: Rubber Stamper <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Cr-Commit-Position: refs/heads/master@{#75725}
This is a reland of 0b091e9 Some blink web tests have been temporarily disabled to allow landing changes to the JS API in V8. Original change's description: > [wasm][eh] Rename Exception to Tag in the JS API > > See: > WebAssembly/exception-handling#159 > > This change only does the rename where it's observable. This should also > be renamed throughout the codebase for consistency and will be done > separately. > > [email protected] > > Bug: v8:8091 > Change-Id: Iec1118194981dfd33be6e30256b6e72d12143e1f > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3021172 > Reviewed-by: Andreas Haas <[email protected]> > Commit-Queue: Thibaud Michaud <[email protected]> > Cr-Commit-Position: refs/heads/master@{#75718} Bug: v8:8091 Change-Id: Id5375b5287fff81b8e0096377a55ef63e6d9b985 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3035083 Commit-Queue: Thibaud Michaud <[email protected]> Reviewed-by: Andreas Haas <[email protected]> Cr-Commit-Position: refs/heads/master@{#75785}
The JS API constructor was renamed to "WebAssembly.Tag" to match the spec: WebAssembly/exception-handling#159 Rename "exception" to "tag" throughout the codebase for consistency with the JS API, and to match the spec terminology (e.g. "tag section"). [email protected],[email protected] Bug: v8:11992 Change-Id: I63f9f3101abfeefd49117461bd59c594ca5dab70 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3053583 Reviewed-by: Clemens Backes <[email protected]> Reviewed-by: Nico Hartmann <[email protected]> Commit-Queue: Clemens Backes <[email protected]> Cr-Commit-Position: refs/heads/master@{#75994}
We recently decided to change 'event' to 'tag', and 'event section' to 'tag section', out of the rationale that the section contains a generalized tag that references a type, which may be used for something other than exceptions, and the name 'event' can be confusing in the web context. See - WebAssembly/exception-handling#159 (comment) - WebAssembly/exception-handling#161 Reviewed By: tlively Differential Revision: https://reviews.llvm.org/D104423
In the core spec, we are currently using the term 'event' to represent a tag that can be used to match an exception object in
catch
instructions. An event contains a signature, in which the result type is always void. Two different event can contain the same signature.The term 'event' was suggested years ago out of concern that there will be more kinds of events that will be added to the event section. See https://github.com/WebAssembly/exception-handling/blob/master/proposals/exception-handling/old/Exceptions-v2-Level-1%2BN.md#level-3 for examples of thought then.
Recently JS API addition proposal was suggested in #154 and it uses the term
exception
to represent an exception, which I think fine, but the consistency in the terminology in the core spec and the JS API comes to mind. Also #150 (comment) has several links to previous comments on possible event vs. exception terminology.I am OK with either, but I think we should be consistent on the terminology in the core spec and the JS API spec.
Do we still plan to add other kinds of events to the event section? Are other stack switching or coroutine primitives planning to use the section?
cc @rossberg @ioannad @takikawa @RossTate @tlively
The text was updated successfully, but these errors were encountered: