Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Update JS API to better specify opaqueData exception identity #250
base: main
Are you sure you want to change the base?
Update JS API to better specify opaqueData exception identity #250
Changes from all commits
b87068d
af87b72
b6df4c2
38bbc27
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Does this (preexisting) line mean we have the reverse mapping from
opaqueData
(or object) toexternaddr
..?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.
My interpretation of the preexisting line is that the core spec was expected to be updated to say that
func_invoke
(from the core spec) returns theopaqueData
(and is also expected to say whereopaqueData
comes from). There is no need for a reverse mapping becauseopaqueData
is theexternaddr
and is passed directly into this step. I think of this like as basically just being a cast fromopaqueData
(a construct that was to be defined in the core spec) toexternaddr
(a construct that is used in this spec as a key in themap
).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.
Why?
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.
Same reason as above.
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.
Why is the tag address referred to as a "type" here?
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.
We'll fix the tag/type wording in a separate change.
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 seems to assume that there is a catch block to handle it, but of course that does not need to be the case, and it is not when the exception actually escapes to JS.
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.
Indeed. Ideally this text should probably say something like "create an exception and unwind the stack" according to the language of the core spec, and "do whatever the core spec says should be done with exceptions"; i.e. unwind until the exception is caught or propagate it back to the embedder. Is there a particular spot in the core spec that defines that creation?
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.
Maybe what we actually need to do is define some kind of interface in the embedding appendix for that (that appendix at least needs to be updated to say what happens when an invoked function raises/propagates an exception). The question still remains though, since that interface will itself need to say what happens.
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.
#268 is the beginning of what I meant by the above comment; although I noticed that it doesn't say anything about calls from the instance into the embedder, so there may not be anything relevant to the case we're talking about here.