Skip to content

Make anyfunc an alias of funcref #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rossberg merged 8 commits into from
Nov 18, 2021
Merged

Make anyfunc an alias of funcref #34

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cbc8527
Make anyfunc an alias of funcref
gahaas Oct 29, 2021
5d432ee
rename most anyfunc to funcref
gahaas Oct 29, 2021
e411acb
address comments
gahaas Oct 29, 2021
6402a30
Add more tests
gahaas Nov 15, 2021
9aa66d5
Add more tests
gahaas Nov 15, 2021
fa0897b
Add 'anyfunc' back to ValueType
gahaas Nov 15, 2021
9a9a143
Handle ToTableKind as well
gahaas Nov 17, 2021
eb42622
Also change import tests
gahaas Nov 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions document/js-api/index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,7 @@ enum ValueType {
"f32",
"f64",
"externref",
"anyfunc",
"funcref",
Comment on lines 953 to +954

@Ms2ger Ms2ger Nov 8, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should keep both, and test that new Global({ type: "funcref" }) works.

@gahaas gahaas Nov 15, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 Ms2ger Nov 15, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

@gahaas gahaas Nov 15, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

};
</pre>

Expand Down Expand Up @@ -1002,6 +1002,7 @@ which can be simultaneously referenced by multiple {{Instance}} objects. Each
1. If |s| equals "i64", return [=i64=].
1. If |s| equals "f32", return [=f32=].
1. If |s| equals "f64", return [=f64=].
1. If |s| equals "funcref", return [=funcref=].
1. If |s| equals "anyfunc", return [=funcref=].
1. If |s| equals "externref", return [=externref=].
1. Assert: This step is not reached.
Expand All @@ -1015,7 +1016,7 @@ The algorithm <dfn abstract-op>FromValueType</dfn>(|s|) performs the following s
1. If |s| equals [=i64=], return "{{ValueType/i64}}".
1. If |s| equals [=f32=], return "{{ValueType/f32}}".
1. If |s| equals [=f64=], return "{{ValueType/f64}}".
1. If |s| equals [=funcref=], return "{{ValueType/anyfunc}}".
1. If |s| equals [=funcref=], return "{{ValueType/funcref}}".
1. If |s| equals [=externref=], return "{{ValueType/externref}}".
1. Assert: This step is not reached.

Expand Down
4 changes: 2 additions & 2 deletions proposals/js-types/Overview.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ All Wasm types can be defined by a simple grammar. This grammar could be mapped

```TypeScript
type ValueType = "i32" | "i64" | "f32" | "f64"
type ElemType = "anyfunc"
type ElemType = "funcref"
type GlobalType = {value: ValueType, mutable: boolean}
type MemoryType = {limits: Limits}
type TableType = {limits: Limits, element: ElemType}
Expand Down Expand Up @@ -200,7 +200,7 @@ function print(...args) {
for (let x of args) console.log(x + "\n")
}

let table = new Table({element: "anyfunc", minimum: 10});
let table = new Table({element: "funcref", minimum: 10});

let print_i32 = new WebAssembly.Function({parameters: ["i32"], results: []}, print);
table.set(0, print_i32);
Expand Down
6 changes: 6 additions & 0 deletions test/js-api/table/constructor.any.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ test(() => {
assert_Table(table, { "length": 5 });
}, "Basic (non-zero)");

test(() => {
const argument = { "element": "funcref", "initial": 5 };
const table = new WebAssembly.Table(argument);
assert_Table(table, { "length": 5 });
}, "Basic with 'funcref'");

test(() => {
const argument = { "element": "anyfunc", "initial": 0 };
const table = new WebAssembly.Table(argument, {});
Expand Down