[test] Account for one-pass implementations in more places#1904
Conversation
rossberg
left a comment
There was a problem hiding this comment.
Hm, to be honest, I don't follow the reason for most of the changes in this PR. Can you elaborate why you think they're needed and how they relate to streaming compilation?
| (module | ||
| (global (export "global-i32") i32 (i32.const 0)) | ||
| (global (export "global-mut-i32") (mut i32) (i32.const 0)) | ||
| ) | ||
| (register "test") | ||
|
|
There was a problem hiding this comment.
This shouldn't be needed. Validation has to happen before linking, so it mustn't matter for any of the tests whether the imported module actually exists.
Same with the other tests.
|
Sorry, I mixed up the terminology in the title - I was referring to implementations that do decoding and validation in one pass. I was generally following the idea that failure tests should only contain one type of error. This test change specifically permits implementations to immediately resolve imports rather than doing that during instantiation. I think this is reasonable and can even be beneficial for some of them. If this addition is undesirable, I can remove that part. |
1416df2 to
23f9c8d
Compare
|
With Wasm's model of separate compilation, imports are not available during validation or compilation. Providing them, i.e., linking, is part of instantiation, and a compiled module can be instantiated multiple times with different imports. Even for API that combines the two phases, an invalid module must be treated as if linking was never started. Because of that, the presence of module imports should never matter for pure validation tests, and we in fact want to test that validation fails regardless. |
23f9c8d to
cca5a5e
Compare
|
Thanks for the clarification! I've removed all linking-related changes. |
No description provided.