WASM Opaque objects VS `for...in`

I have questions
21

It is a V8 bug, strictly speaking, but V8 uses the Chromium bug tracker. It throws in latest Node and on Chrome Canary, as you can verify yourself, so no, it hasn’t been fixed (or if it has it was fixed very recently).

It should be filed as a bug against V8 (I believe you do this by opening an issue on the Chromium bug tracker, but with the component being “JavaScript interpreter” or something like that), including the reproducer above, the link to the spec text for both WebAssembly and JS, and the link to the code that mhofman found; also mention that Safari and Firefox do not exhibit this behavior (they just have an empty loop), so this is a web-compatibility issue.

No. I linked the spec above; it is not just “do whatever”. It’s supposed to be an empty loop, the same way Reflect.ownKeys(obj) is empty.

If you’re being really pedantic the JS spec does allow a certain amount of leeway for for-in over host-defined objects like this, but that’s mostly for historical reasons (I wrote that part of the JS spec) and it’s clearly not the intention of the WebAssembly spec to have this behavior.

22

Issue filed: Chromium

23

Thanks. I added some details to the Chromium issue. I also realized there is an unexpected return value specified for wasm GC object for [[PreventExtensions]] and filed an issue against the spec itself.

24

thanks a lot … it looks like current state is a proper “can of worms"“ indeed … I wonder though, how comes WASM doesn’t have basic tests to validate its exotic objects are actually behaving exactly like specs expect? Are we lacking conformance tests around the most important stack we have in these days?

In short, I am wondering if creating a proper test around all expectations would help, but it kinda puzzles me such test is not in the pipe/stack already for both WASM and vendors :thinking:

25

There are tests that are apparently testing for this throwing behavior on preventExtensions, which is puzzling. We'd have to dig through the historical discussions to know why the WASM folks picked a behavior that is not natural for JS, or if that was on purpose.

26

but PreventExtensions: returns true is part of that commit … it was falsebefore but it looks like that commit should just work?

Reflect.isExtensible(x) || Reflect.preventExtensions(x) should be false || true accordingly to your link, so I think something else is off?

Puzzling indeed, but in that test file I don’t see any for…in involved, so maybe that’s something they should add now!

27

wait a minute … you PR’d that … that’s why, OK, I think that PR makes perfect sense, I also think that file should stick a for…in in somewhere to close the circle of possible shenanigans around those objects, but I still don’t understand how that could fail behind the scene.

edit added

test(() => {
  const struct = functions.makeStruct();
  const array = functions.makeArray();
  assert_true((() => { for (const _ in struct); return true; })());
  assert_true((() => { for (const _ in array); return true; })());
}, "for...in");

as suggestion, but it feels like the wrong issue to fix that too, although I am really curious to understand why things are the way these are right now or why those decisions were made before and how come nobody tested basic for…in before … I can’t read anything hostile on purpose in the current specs/decision, let’s hope it was just a “typo” in specs and/or something never considered as issue to date.