Update JS API for exnref
This change updates exception object allocation, initialization, and construction for exnref; as well as dealing with exception propagation from invoking exported functions; throwing exceptions from host functions into wasm; and wrapping and unwrapping JS exceptions as they propagate into and out of wasm.
It depends on the embedding interfaces added in #268.
I don't see this in this PR or the overview, but is it intended for exn to be passable to/from JS through params/results, or be like v128 and cause TypeError's?
I don't see this in this PR or the overview, but is it intended for
exnto be passable to/from JS through params/results, or be likev128and cause TypeError's?
When Andreas updated the core spec he also did a minimal update to this spec that made it the latter (see e.g. here or in the getters/setters for table elements and globals).
In this PR when exceptions are thrown or propagate from wasm into JS, they end up wrapped in a WebAssembly.Exception object (that's line 1023 in this patch) just as they are in phase 3 exceptions. I'm not sure what a concrete use case would be for passing/returning but it seems slightly weird and asymmetric that you can get any exception instance out to JS by throwing it, but not by returning it.
It is intentional. I can't find the original discussion right now, but being able to pass exception packages as first-class values to JS would have nasty implications, such as:
- Instead of being able to just do reference counting on exnrefs internally, it would require engines to do either cross host-wasm GC and hence effectively create a dependency on GC again, or rely on host code correctly participating in RC, which could easily corrupt the Wasm engine when that's done wrong.
- The host could pass it back as an externref, which implies that exnref becomes a subset of externref, which in turn means that GC/RC for exnrefs would have to include externrefs, with very unclear implications for non-JS embedding.
- For JS embeddings, it would likely expose or constrain implementation details of the handling of the special JavaScript exception tag, because JS code could compare the identity of a thrown JS value and an exnref representing its catch.
Conceptually, its better to think of and exnref as not the exception instance being thrown, but a "pair" of an exception instance that's been caught plus abstract context information for rethrowing it that a Wasm engine might need internally. As such it is a different kind of object from the exception instance and entirely Wasm-internal.
Okay, that was my understanding as well, but wanted to make sure. SpiderMonkey throws a TypeError in ToJSValue.
I think this PR is now reasonably correct and self-contained according to the description in OP. I think it correctly (although may not yet completely) implements what we discussed in #282. (I had forgotten about that thread, we can move further discussion about the desired value semantics back there). So I think it probably makes sense to finish this review and land it, to keep PRs of a reasonable size.
But this algorithm is invoked from another one that already describes Wasm semantics, not host semantics. So we're morally "inside Wasm". So what I'd suggest is to say something like "execute the Wasm instructions (ref.exn |address|) (throw_ref). And with my other comment there is only one place left where this is invoked, so I'd just inline that step there and remove this algorithm.
This is done too, but since the lines it was attached to are now gone, github won't let me reply...
I'm going to go ahead and land this so I can rebase #269 and do some more cleanups. @aheejin let me know if you have more comments, we can update as needed.