Missing exnref type check for JS API Table constructor?

[thibaudmichaud]

The Table constructor uses ToValueType and ToWebAssemblyValue, which both assert that the type argument is not exnref, but the Table constructor itself does not check this:

The Table(descriptor, value) constructor, when invoked, performs the following steps:

• Let elementType be ToValueType(descriptor["element"]).
• If elementType is not a reftype,
  • Throw a TypeError exception.
• (... more rules that don't check exnref ...)
• Otherwise,
  • Let ref be ? ToWebAssemblyValue(value, elementType).

Should ToValueType accept exnref, and the second step be:

• If elementType is exnref,
  • Throw a TypeError exception.

?

[aheejin]

Yeah I think that makes sense. I think we had discussions on why we wouldn't want to add exnref to ToWebAssemblyValue and ToJSValue, but I don't see why we shouldn't add it to ToValueType and separately check it. cc @dschuff

[rossberg]

It looks like there are several inconsistencies around ValueType and ToValueType:

1. The ValueType enum end the ToValueType function allow v128 but not exnref.
2. The Global constructor checks for v128 and exnref, although ToValueType does not currently return the latter.
3. The Table constructor only checks for v128 (implicitly).
4. The Tag constructor doesn't check for either (which probably is intentional — we want to prevent values but not the types as such).
5. The Exception constructor checks for both v128 and exnref, although ToValueType does not return the latter.
6. The Global.get/set methods check against both v128 and exnref, although the latter cannot occur.
7. The Table.get/set methods check only against exnref, although it can't occur.
8. The Tabel.grow method does not check for exnref.

The intention presumably was that exnref is handled like v128, in which case (1), (3), (8) are bugs, i.e., the immediate fix would be:

• Add exnref to ValueType and ToValueType.
• Have the Table constructor check against exnref, as suggested.
• Have the Tabel.grow method check against exnref.

However, I would strongly suggest a refactoring here: Instead of having these checks duplicated in so many places, why not simply move them inside ToJSValue and ToWasmValue and throw there? Then they are in a canonical place, and there is no risk of forgetting them again in the presence of the next extension, be it new functions or new types.