[js-api] Unclear spec for `-0` in `ToWebAssemblyValue`?

[Liedtke]

The algorithm ToWebAssemblyValue has the following specification for i31:

7.4. Else if v is a Number and v is equal to ? ToInt32(v) and ℝ(v) < 2^30 and ℝ(v) ⩾ -2^30,

What does "v is equal to ToInt32(v)" exactly mean? ToInt32(-0) is +0. Is +0 equal to -0? JS would say yes (-0 === +0), however, it is an important property that if a parameter is typed externref or anyref, ToWebAssemblyValue() should not have any observable side effects on JS values (especially when passing it back to JS).

The ECMAScript spec seems to explicitly list -0 in many cases (for clarity?), maybe this algorithm should do the same?

[bvisness]

I imagine that it would be equality as defined for Number, which is defined as typical floating point equality, meaning -0 would equal +0.

[rossberg]

No, this should not equate -0 and +0, since it's only supposed to include values that are in the image of the ToJSValue for an i31. The correct check should be using SameValue.

[bvisness]

Well, we explicitly implemented the opposite based on the above reading of the spec, but I'm sure we'd be able to change it.

[rossberg]

Yeah, that would also be my literal interpretation of what's written. I think the text should be fixed, but maybe other folks have a different opinion?

[eqrion]

I'm fine with modifying the spec text so that -0 is disallowed for i31ref. I think having Object.is(ToWebAssemblyValue(ToJSValue(x)), x) always true is a nice property.

[Liedtke]

I think having Object.is(ToWebAssemblyValue(ToJSValue(x)), x) always true is a nice property.

I guess, we could add a spec test like:

let global = new WebAssembly.Global({value: "externref", mutable: true});
global.value = -0;
assert(Object.is(global.value, -0));

If that is agreeable, I can extend #1983 to test for that.

[eqrion]

That's fine with me. I think the JS-API spec text needs to be updated to clarify this too, as the plain reading of it now would permit -0. SpiderMonkey will need to update it's implementation again, but that's fine.

[tlively]

@kmiller68, any concerns about disallowing -0 or i31ref?

@Liedtke, extending the test would be good.

[Ms2ger]

Maybe using https://tc39.es/ecma262/multipage/abstract-operations.html#sec-samevalue would make sense here?