Skip to content
This repository was archived by the owner on Nov 3, 2021. It is now read-only.

[js-api] Extend with reference types support. #79

Merged
merged 2 commits into from
Mar 24, 2020

Conversation

Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Feb 11, 2020

No description provided.

1. let |initial| be |descriptor|["initial"].
1. If |descriptor|["maximum"] is [=present=], let |maximum| be |descriptor|["maximum"]; otherwise, let |maximum| be empty.
1. If |maximum| is not empty and |maximum| < |initial|, throw a {{RangeError}} exception.
1. Let |type| be the [=table type=] {[=table type|𝗆𝗂𝗇=] n, [=table type|𝗆𝖺𝗑=] |maximum|} [=table type|funcref=].
1. If |value| is missing and |elementType| is {{TableKind/anyfunc}},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens for other types? Isn't the next step ill-defined then?
Either way, I would simply make the default to null independent of the element type, seems cleanest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this didn't end up quite right. However, it's not exactly clear what "right" means in the context of optional any, unfortunately (https://www.w3.org/Bugs/Public/show_bug.cgi?id=23602).

We probably want to support creating an anyref table with the values initialized to undefined. If we defaulted to null in that case, we'd either break that or make new Table({..}, undefined) do something different from new Table({..}). That's pretty surprising behavior in JS, so I don't think we should do that.

That leaves the nullref type to be considered; it looks like I proposed making new Table({..}, undefined) and new Table({..}) both throw there. That's probably not so helpful.

I notice that the Global constructor has the same issues… I think the best solution will be to use DefaultValue().

1. Let |tableaddr| be **this**.\[[Table]].
1. Let |initialSize| be the length of **this**.\[[Values]].
1. Let |store| be the [=surrounding agent=]'s [=associated store=].
1. Let |result| be [=table_grow=](|store|, |tableaddr|, |delta|, [=ref.null=]).
1. Let (<var ignore>limits</var>, |elementType|) be [=table_type=](|tableaddr|).
1. If |value| is missing and |elementType| is {{TableKind/anyfunc}},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

1. If |value| does not have a \[[FunctionAddress]] internal slot, throw a {{TypeError}} exception.
1. Let |ref| be [=ref.func=] |value|.\[[FunctionAddress]].
1. Let (<var ignore>limits</var>, |elementType|) be [=table_type=](|tableaddr|).
1. If |value| is undefined and |elementType| is {{TableKind/anyfunc}},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is less of an issue, because the argument is not optional, so it can't be "missing". OTOH, this means it clashes even more with https://www.w3.org/Bugs/Public/show_bug.cgi?id=23602. I think I'll make this optional as well.

Copy link
Contributor Author

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!

1. let |initial| be |descriptor|["initial"].
1. If |descriptor|["maximum"] is [=present=], let |maximum| be |descriptor|["maximum"]; otherwise, let |maximum| be empty.
1. If |maximum| is not empty and |maximum| &lt; |initial|, throw a {{RangeError}} exception.
1. Let |type| be the [=table type=] {[=table type|𝗆𝗂𝗇=] n, [=table type|𝗆𝖺𝗑=] |maximum|} [=table type|funcref=].
1. If |value| is missing and |elementType| is {{TableKind/anyfunc}},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this didn't end up quite right. However, it's not exactly clear what "right" means in the context of optional any, unfortunately (https://www.w3.org/Bugs/Public/show_bug.cgi?id=23602).

We probably want to support creating an anyref table with the values initialized to undefined. If we defaulted to null in that case, we'd either break that or make new Table({..}, undefined) do something different from new Table({..}). That's pretty surprising behavior in JS, so I don't think we should do that.

That leaves the nullref type to be considered; it looks like I proposed making new Table({..}, undefined) and new Table({..}) both throw there. That's probably not so helpful.

I notice that the Global constructor has the same issues… I think the best solution will be to use DefaultValue().

1. If |value| does not have a \[[FunctionAddress]] internal slot, throw a {{TypeError}} exception.
1. Let |ref| be [=ref.func=] |value|.\[[FunctionAddress]].
1. Let (<var ignore>limits</var>, |elementType|) be [=table_type=](|tableaddr|).
1. If |value| is undefined and |elementType| is {{TableKind/anyfunc}},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is less of an issue, because the argument is not optional, so it can't be "missing". OTOH, this means it clashes even more with https://www.w3.org/Bugs/Public/show_bug.cgi?id=23602. I think I'll make this optional as well.

Copy link
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. The disadvantage of formulating it in terms of DefaultValue is that we will soon (with typed funcrefs) have types that do not have a default value. But we can worry about it then.

@Ms2ger
Copy link
Contributor Author

Ms2ger commented Feb 18, 2020

Re typed function references: will it be possible to put those in globals too?

@rossberg
Copy link
Member

@Ms2ger, yes.

@Ms2ger
Copy link
Contributor Author

Ms2ger commented Feb 18, 2020

Okay, then we need to handle that in (or around) DefaultValue anyway. Let's cross that bridge when we get to it, though.

As far as I'm concerned, this is good to go.

@bzbarsky: just as a fyi, this is the API that prompted my questions earlier.

@Ms2ger
Copy link
Contributor Author

Ms2ger commented Feb 18, 2020

I noticed I didn't use DefaultValue quite right; it returns a wasm value, not a js value. Fixed.

@littledan
Copy link
Contributor

Anything blocking this from landing?

@Ms2ger
Copy link
Contributor Author

Ms2ger commented Mar 24, 2020

As far as I know, the only blocker is finding someone to push the big green button. @rossberg ?

@rossberg rossberg merged commit 2028231 into WebAssembly:master Mar 24, 2020
@rossberg
Copy link
Member

Ah, sorry. Thanks!

@Ms2ger Ms2ger deleted the update-jsapi branch August 11, 2020 15:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants