Error more clearly on wasm components #6751
Conversation
| Oh components retconned the 32-bit version marker, which is 1 for core wasm, as the low 16 bits being core wasm version and the upper 16 bits being a possible component version. That means that the binary here isn't a valid component and needs a few bytes shuffled. The smallest component is: whereas the test case in this PR is: |
| Thanks @alexcrichton ! Fixed. |
| auto version = getInt32(); | ||
| if (version != BinaryConsts::Version) { | ||
| if (version == 0x1000d) { | ||
| throwError("this looks like a wasm component, which Binaryen does not " |
There was a problem hiding this comment.
We can change wasm to Wasm in the error string, and it could be nice to give a link to #6728. It allows people to follow the progress and maybe help if they're interested.
There was a problem hiding this comment.
Good idea, I added a link now.
We use lowercase wasm which is more informal in our other error strings, so for consistency I didn't change that. Though I agree in proper English it should be capitalized.
test/lit/binary/component-error.test Outdated
| | ||
| ;; RUN: not wasm-opt %s.wasm 2>&1 | filecheck %s | ||
| | ||
| ;; CHECK: this looks like a wasm component, which Binaryen does not support yet |
There was a problem hiding this comment.
If the string in wasm-binary.cpp is change, adapt the test accordingly.
4 participants
Binary format, which I hope I read correctly:
https://github.com/WebAssembly/component-model/blob/main/design/mvp/Binary.md#component-definitions
Context: #6728 (comment)