align and offset can be specified in either order#205
Conversation
|
LGTM |
|
Great, thanks! For context, this addresses WebAssembly/wabt#17 |
|
Wouldn't it be just as easy to change s2wasm to emit these in the spec order? Requiring them to be in a consistent order reduces gratuitous variation, which should make code easier for humans to read in general. |
|
@sunfishcode yes it's easy to fix, but IMO a human-oriented format shouldn't mandate an order for named parameters so I figured it was easier to fix it officially than let someone else stumble on it later. |
|
Fixed order: easier for humans to read I think optimizing for readability is more important than writeability. |
|
Not to get philosophical, but is fixed order always easier for humans to read? I don't think it is for me - I would be surprised if the order of these particular things is fixed, i.e., I would not properly reject an invalid input when reading it - and I think I'm human (at least enough to fool captchas). But I don't feel strongly. Happy to update s2wasm if that's what people want. |
|
Perhaps it doesn't for all humans, but I've fooled a few captchas myself ;-), and I find code easier to read when there is less meaningless variation. |
These should probably be commutable so humans don't have to remember which order to read/write things in, but there's pushback and this really doesn't matter so fix it here, bikeshed on github. This will require an associated fix in sexpr-wasm's GCC torture test failure list. Ref: WebAssembly/wabt#17 Ref: WebAssembly/spec#205
|
With WebAssembly/binaryen#66 merged, the immediate concern here is now resolved, so I think we can close this. If people would like to discuss it futher, feel free to re-open or file a new issue. |
This is not the suggestion given in that last review of WebAssembly#143: https://github.com/WebAssembly/exception-handling/pull/143/files#r759907998 but a potential fix of the issue raised there.
No description provided.