Make anyfunc an alias of funcref #34
Conversation
|
ping |
| "f64", | ||
| "externref", | ||
| "anyfunc", | ||
| "funcref", |
There was a problem hiding this comment.
You should keep both, and test that new Global({ type: "funcref" }) works.
There was a problem hiding this comment.
How would it work if we keep both? Wouldn't that mean that they are different types? How should I define then what the result of type() is?
My idea was that there is only one type, funcref, but the string anyfunc is accepted as an alias in the constructor. Internally, in the non-observable parts of the spec, only funcref is used. Then type() could remain mostly unchanged, because we don't have to merge the return value for funcref and anyfunc somehow.
There was a problem hiding this comment.
Internally, we only use the "core" types, and the IDL enum here is only a way to expose them to JS. ToValueType then maps both enum values to [=funcref=], and FromValueType only creates the "funcref" value. (Those are both already correct in the PR.) Does that make sense?
There was a problem hiding this comment.
You are saying that this is the set of strings that is allowed to be used in a GlobalType, and not the internal type. Fair enough, I added anyfunc back. PTAL
|
My understanding is that these cases should be handled in spec and tests: Inputs (should support both spellings):
Outputs (should use "funcref"):
|
|
Ref #25 |
gahaas
left a comment
gahaas
left a comment
There was a problem hiding this comment.
I added more tests now. I'm not sure about the tests in module.exports and module.imports that you requested. Please tell me if you meant more that this.
I used the wpt tests as a base, because the tests in this repository seem to be outdated.
| "f64", | ||
| "externref", | ||
| "anyfunc", | ||
| "funcref", |
There was a problem hiding this comment.
How would it work if we keep both? Wouldn't that mean that they are different types? How should I define then what the result of type() is?
My idea was that there is only one type, funcref, but the string anyfunc is accepted as an alias in the constructor. Internally, in the non-observable parts of the spec, only funcref is used. Then type() could remain mostly unchanged, because we don't have to merge the return value for funcref and anyfunc somehow.
Ms2ger
left a comment
Ms2ger
left a comment
There was a problem hiding this comment.
Thanks! It seems like you missed ToTableKind in the spec, but lgtm otherwise.
|
Done. |
| assert_true(type.writable, 'type: writable'); | ||
| assert_true(type.enumerable, 'type: enumerable'); | ||
| assert_true(type.configurable, 'type: configurable'); | ||
| if (expected.type.parameters) { |
There was a problem hiding this comment.
Please do !== undefined here as well
|
@rossberg could you merge? |
3 participants
As discussed in WebAssembly/reference-types#85 (review), this PR turns
anyfuncin the js-api into an alias offuncref. Bothanyfuncandfuncrefare valid strings in the js-api. I renamed most uses ofanyfuncintofuncrefbut kept a test foranyfunc.