Update JS API #379
Update JS API #379
Conversation
document/js-api/index.bs
Outdated
| @@ -1149,27 +1143,42 @@ The algorithm <dfn>ToWebAssemblyValue</dfn>(|v|, |type|) coerces a JavaScript va | |||
| 1. Return [=ref.null=] |heaptype|. | |||
| 1. Otherwise, | |||
| 1. Throw a {{TypeError}}. | |||
| 1. If |type| is a subtype of [=ref=] |null| [=heap-type/func=], | |||
| 1. If |heaptype| is a subtype of [=heap-type/func=], | |||
There was a problem hiding this comment.
Should we use the "matches" terminology to match the core spec?
document/js-api/index.bs
Outdated
| 1. Let |objectaddr| be the value of |v|'s \[[ObjectAddress]] internal slot. | ||
| 1. Let |objecttype| be the value of |v|'s \[[ObjectType]] internal slot. | ||
| 1. If |objecttype| is "array", | ||
| 1. If |heaptype| is not a supertype of [=array=], |
There was a problem hiding this comment.
Supertype or subtype, right? Since we want to do implicit downcasts on the boundary. Same for structs below.
We also need to handle the possibility of the downcast failing for structs, arrays, and functions.
There was a problem hiding this comment.
This is the check for the downcast. The heaptype is the target type here, so the actual type must be a subtype of it for that cast to succeed.
Using "matching" terminology now, which may make this somewhat clearer.
document/js-api/index.bs
Outdated
| 1. If |heaptype| is not a supertype of [=struct=], | ||
| 1. Throw a {{TypeError}}. | ||
| 1. Return [=ref.struct=] |objectaddr|. | ||
| 1. If |heaptype| is not a supertype of [=any=], |
There was a problem hiding this comment.
It would be clearer to use (in)equality here.
There was a problem hiding this comment.
Turned around now using matching terminology. Not using inequality mainly for symmetry with the other checks.
|
@tlively, PTAL. |
document/js-api/index.bs
Outdated
| 1. If |objecttype| is "array", | ||
| 1. If [=array=] does not match |heaptype|, | ||
| 1. Throw a {{TypeError}}. | ||
| 1. Return [=ref.array=] |objectaddr|. |
There was a problem hiding this comment.
Don't we still need to do a ref.test to make sure the downcast succeeds when |heaptype| is a specific array type? Same with structs below.
There was a problem hiding this comment.
Well, we do not support concrete array types in the API yet, so this assumes the invariant that heaptype is one of the abstract heap types. Should we make this more powerful?
There was a problem hiding this comment.
Hmm, am I misremembering the result of those discussions? I thought we allowed arbitrary types on the boundary.
There was a problem hiding this comment.
Yes, I think I was correct: #279 (comment)
There was a problem hiding this comment.
Or maybe I misremember -- I assumed concrete types cannot come up because we cannot express them API-side. But did we decide to allow calling into exported Wasm functions with concrete types?
I guess there is nothing else preventing it anyway, so something needs to be fixed here. I'll extend the semantics. However, this will probably require replacing the existing [[ObjectKind]] thingy with a proper call to the embedder interface (to get the precise object type), which I haven't written yet.
There was a problem hiding this comment.
Ah, right, we don't have a way to describe concrete types from the JS API, but yes, I was thinking about the case of calling into an exported function (or setting an exported global, etc) that expects a concrete type.
There was a problem hiding this comment.
Can't we just execute a ref.test like we do for function types?
There was a problem hiding this comment.
Ah yes, you are right, I already added that as part of the funcref proposal.
There was a problem hiding this comment.
Hm, actually I didn't. So that probably is a dangling ref at the moment.
There was a problem hiding this comment.
Okay, I think this is fixed now. I also refactored the function to have less duplication.
document/js-api/index.bs
Outdated
| 1. If |v| is an [=Exported GC Object=], | ||
| 1. Let |objectaddr| be the value of |v|'s \[[ObjectAddress]] internal slot. | ||
| 1. Let |objectkind| be the value of |v|'s \[[ObjectKind]] internal slot. | ||
| 1. If |objecttype| is "array", |
There was a problem hiding this comment.
I think there's a typo here, probably my fault as I think it's in the old revision too (where I forgot to rename to ObjectKind at all spots). Should be |objectkind| instead of |objecttype|?
document/js-api/index.bs
Outdated
| 1. If [=match_valtype=](|type|, [=ref=] |null| [=heap-type/func=]) is false, | ||
| 1. Throw a {{TypeError}}. |
There was a problem hiding this comment.
Is this supposed to use actualtype somewhere? Also, why is the store involved?
There was a problem hiding this comment.
Oops, yes, forgot to update that line after copy & paste. Fixed.
The store is needed to obtain the precise type of a reference value, because it can't be told from the address alone.
Adjust JS API to match core spec: