I think we need a little more than this.
For example, section 4.7.3 says the algorithm is expected to move to the core specification, but since the core throw step won't mention the opaque data, this section will probably need to stay here. It wouldn't make sense to say "Invoke the catch block with |payload| and |opaqueData|" but maybe it could say something like "Invoke the catch block with |payload|" and then should say what happens to opaqueData. e.g. if the exception is subsequently rethrown before control returns to JS then the same opaqueData needs to be returned from "call an Exported Function" (item 9 needs to be updated). Likewise for 3.9.5 of "create a host function"; "Throw with ... |opaqueData|" will have to say something like "throw with type and payload, and then return the same opaqueData if the exception propagates back to JS or is rethrown".
Also probably somewhere should say that if the exception is generated by wasm rather than by JS, then opaqueData is a null externref.

And most importantly we should add tests for these cases; e.g. a test where wasm calls a JS function that creates a new WebAssembly.Exception, adds some properties to it and throws it, then lets it propagate through the wasm frame and catches it, and tests that the same properties are on the caught object. (and one that rethrows inside of wasm rather than letting it just propagate).

It wouldn't make sense to say "Invoke the catch block with |payload| and |opaqueData|" but maybe it could say something like "Invoke the catch block with |payload|" and then should say what happens to opaqueData. e.g. if the exception is subsequently rethrown before control returns to JS then the same opaqueData needs to be returned from "call an Exported Function" (item 9 needs to be updated). Likewise for 3.9.5 of "create a host function"; "Throw with ... |opaqueData|" will have to say something like "throw with type and payload, and then return the same opaqueData if the exception propagates back to JS or is rethrown". Also probably somewhere should say that if the exception is generated by wasm rather than by JS, then opaqueData is a null externref.

I'm not sure if I understand the motivation. This sounds like, you want opaqueData to be removed from the Exception return type, and we can't even say things like "throw an exception with tag/payload/opaqueData" at all, and change all these sentences to include only the tag and payload. And we only mention opaqueData in an indirect way like "The same opaqueData is expected to be returned." Is my understanding correct?

If so, I'm not sure what the purpose of this is. Either way we are adding another stuff to be returned (opaqueData) to the core spec, which is unfortunate, but that's what we've decided to do. Why can't we simply include opaqueData in the return type and only mention it in a roundabout way?

Also, does this "removing opaqueData from the return type and mention in a roundabout way" work with this wrapping-unwrapping scheme you described in #218 (comment)?

I think I might be misunderstanding your comments, in which case please let me know.

And most importantly we should add tests for these cases

Will do that. Do you mind if I do that as a separate PR?

Yes, I agree with @dschuff. Stating the existence of opaque data is one piece, but the more important part is to specify that it has identity and how that is preserved.

I think that we can model identity (and opacity) itself by assigning an external address to the payload and store it in map. This has to be converted upon entry and exit from Wasm, like regular values. The other piece is to hand-wave somehow that the core rule for executing rethrow preserves this external address.

FWIW, this also needs to assign identities to Wasm exceptions itself, i.e., Wasm throw. That identity may be observed in JS, after the exception has been caught and rethrown in JS, passes through another layer of Wasm (which could involve rethrowing it as well), and is caught by JS again.

I'm not sure if I understand the motivation. This sounds like, you want opaqueData to be removed from the Exception return type, and we can't even say things like "throw an exception with tag/payload/opaqueData" at all, and change all these sentences to include only the tag and payload. And we only mention opaqueData in an indirect way like "The same opaqueData is expected to be returned." Is my understanding correct?

Basically yes. The core spec definitions for returning from an exported function and invoking a catch block do not mention any opaque data, and this spec is currently written as if it does. e.g. can't say here that this opaque data is handed to us by the function return (at least not in the way that's written here, where it just lists opaqueData as another return value). I think what @rossberg is suggesting is that we allocate an externaddr associated with the payload when we create a new exception (e.g. in 12.2 of "call an Extported Function").
And then I guess when we get a payload from the exported function (12.1), we would find the externaddr associated with that payload.

I tried to make this work using just the extern value cache, but it didn't seem quite right. We basically need to have a map that maps exception payloads (which now need to be somehow unique) to exception objects, so that we can look them up in 12.1. Or alternatively wrap a new exception object around the same payload.
I'm still thinking about how this mapping might work.

If so, I'm not sure what the purpose of this is. Either way we are adding another stuff to be returned (opaqueData) to the core spec, which is unfortunate, but that's what we've decided to do. Why can't we simply include opaqueData in the return type and only mention it in a roundabout way?

We have to say somewhere that "something" is associated with an exception as it goes into and out of wasm to/from JS. This spec currently says that something is opaqueData and assumes that the core spec/wasm knows what that is, and knows that the opaqueData that goes into wasm in in section 4.7.3 is the same one that comes out in 12.2. But I don't think we can just include opaqueData in the return type without saying somewhere where it comes from.

Also, does this "removing opaqueData from the return type and mention in a roundabout way" work with this wrapping-unwrapping scheme you described in #218 (comment)?

I think the wrapping/unwrapping I described is still the behavior we want, we just need way to describe that behavior here without relying on the core spec.

And most importantly we should add tests for these cases

Will do that. Do you mind if I do that as a separate PR?

Yes, separate PR is fine with me.

Thanks for the comments! But more questions 😅:

@rossberg

I think that we can model identity (and opacity) itself by assigning an external address to the payload and store it in map. This has to be converted upon entry and exit from Wasm, like regular values. The other piece is to hand-wave somehow that the core rule for executing rethrow preserves this external address.

You mean we can treat a payload as a unique extern object, and create a mapping of map[externaddr] = payload? Then how do we associate this payload with the exception object (or opaqueData) itself? Do we need a separate map[externaddr] = exception object? If so, how can we link the relationship between the payload and the exception object?

FWIW, this also needs to assign identities to Wasm exceptions itself, i.e., Wasm throw. That identity may be observed in JS, after the exception has been caught and rethrown in JS, passes through another layer of Wasm (which could involve rethrowing it as well), and is caught by JS again.

As I asked above, would we also need map[externaddr] = exception object? I'm not sure why we would need the identity of both the payload and exception object, and how to specify their linked relationship.

@dschuff

I think what @rossberg is suggesting is that we allocate an externaddr associated with the payload when we create a new exception (e.g. in 12.2 of "call an Extported Function"). And then I guess when we get a payload from the exported function (12.1), we would find the externaddr associated with that payload.

Can we have the reverse mapping, meaning given the payload, can we find its externaddr? Don't we only make map[externaddr] = payload?

I tried to make this work using just the extern value cache, but it didn't seem quite right. We basically need to have a map that maps exception payloads (which now need to be somehow unique) to exception objects, so that we can look them up in 12.1.

You mean, we can't make this work only with map[externaddr] = payload, so we also would need map[payload] = exception object? Here given that we treat the payload as an object, can we use an object as a key to a map?

Yes, I think that's basically what I meant. In the current spec we are extracting the externaddr exception object from opaqueData, (meaning the core machinery is tracking that identity for us). I think @rossberg meant that we can store the externaddr for the JS exception object in map; but what we are missing is a way to get it when an exception propagates from core wasm in 12 of "call an exported function". Given that map only has addresses (e.g. externaddr, funcaddr, etc) as keys, map[payload] doesn't quite make sense directly in the current version, because payload in this context can be a wasm primitive type.

Currently we aren't using the map at all because the externaddr for the JS exception object is already getting converted on entry an exit from wasm (i.e. it goes into opaqueData directly via ToWebAssemblyValue in 3.9.4 of "create a host function"). We could maybe just keep that mechanism, but just add some text saying what opaqueData is for and referring to it in the "core rules for rethrow" part, e.g. maybe 3.9.4 stays the same, but insert a step between 3.9.4 and 3.9.5, something like "opaqueData` must be tracked by the core mechanism and associated with the exception as it propagates. If the exception is caught and rethrown, its value stays the same."
That version essentially puts the identity hand-waving right inline with the part that covers exceptions entering wasm. Alternatively it could be factored out and puts somewhere else (and 3.9.5 would just refer to that part and say "opaqueData is one of these things".

I looked a little harder at the current version and realized that we are already storing the exception's identity (in the form of the JS exception object) in the map (the insertion into the map is hidden inside ToWebAssemblyValue when called from 3.9.4 of "create a host function", and the retrieval is in "retrieving an extern value " in 12.2 of "call an exported function". So I think we are a little closer than I realized. I took a shot at mentioning the identity preservation on rethrow in #250
WDYT?

Will close this given that this is not relevant anymore.