Conversation
aheejin
left a comment
There was a problem hiding this comment.
Thank you! By the way, there seem to be some .wasm files (without matching .wast files) in test/ directory. Do we need to test them too?
| if (!binary.buffer) binary = new Uint8Array(binary); | ||
| } else { | ||
| var args; | ||
| if (typeof scriptArgs != 'undefined') { |
There was a problem hiding this comment.
What is scriptArgs? (I don't know much about the JS part, sorry)
There was a problem hiding this comment.
It's one of the ways various JS shells give us access to the commandline params. I think scriptArgs is for spidermonkey and arguments is for v8 (or maybe the opposite).
|
|
||
| // Test the wasm for validity. The imports aren't there (we can't use a proxy | ||
| // because we don't know the types), but if we get to the imports, that means | ||
| // the wasm is valid. |
There was a problem hiding this comment.
I don't know much about the v8 validation process. Are imports validated after all other things?
There was a problem hiding this comment.
Yes, it compiles first, then uses imports only when creating an instance (at least v8 and sm do - perhaps chakra does lazy validation/compilation?).
Thinking about this again now, I see the code can be much simpler - we can just compile and not even create an instance, so no imports are involved.
|
Thanks, that commit should address those comments. Except for the note about other wasm files in test/ - good point, we should check them too. I'll do it as a followup. |
|
Turns out it was just the wasm-dis checks - I was worried there were more. So I added those other wasm files in this PR. Removed one old invalid file while doing so. |
No description provided.