Skip to content
This repository was archived by the owner on Apr 25, 2025. It is now read-only.

The else branch of if_except should not be optional #54

Closed
eholk opened this issue Mar 7, 2018 · 5 comments
Closed

The else branch of if_except should not be optional #54

eholk opened this issue Mar 7, 2018 · 5 comments

Comments

@eholk
Copy link
Contributor

eholk commented Mar 7, 2018

@sunfishcode pointed out that the fact that our current if instruction has an optional else branch complicates decoding because we do not know at the time we see the if instruction whether we are going to parse one block of instructions or two. We should avoid getting into that case here.

One option is to always require an else branch for if_except, which can be empty if its not needed. Another would be to have separate opcodes for the two-armed and one-armed if_except blocks.

What do people think?

@rossberg
Copy link
Member

rossberg commented Mar 8, 2018

I don't understand. This works fine for if, how is it different here? Omitting the else branch is nothing more than a shorthand for an empty else branch. In other words, all this is doing is allowing the ELSE opcode to be omitted if it is immediately followed by END.

@eholk
Copy link
Contributor Author

eholk commented Mar 8, 2018

Yeah, I'm not sure I feel strongly about this issue. In general I like balanced ifs, but "I like it" isn't a very strong argument in my opinion. "It makes decoding more complicated" is stronger, but as you point out, it's a minor complication.

I'll give others a chance to speak up if they feel strongly, otherwise I'm happy to close the issue.

@sunfishcode
Copy link
Member

My observation is that in at least two wasm consumers, something like if_else/end/end would have been easier to deal with than if/else/end, because it would have provided more information up front. However, the reason these consumers care about this information turns out to be that they're trying to avoid creating empty blocks. Requiring producers to add empty blocks would just shift the problem elsewhere.

So, I think what this suggests is: instead of if_except/else/end, add an if_except_else/end/end, or an if_except_else/else/end.

@titzer
Copy link
Contributor

titzer commented Mar 8, 2018

Despite the minor inefficiencies in some consumers, there is value in being consistent with the prior if-else-end rules, so my personal preference would be to continue to allow the else to be optional for if_except.

@aheejin
Copy link
Member

aheejin commented Oct 20, 2019

The current version of the proposal uses br_on_exn instead. Closing this.

@aheejin aheejin closed this as completed Oct 20, 2019
ioannad pushed a commit to ioannad/exception-handling that referenced this issue Jun 6, 2020
This adds the new operations, as well as the new data and element
segment formats.

Implemented:

* Text parsing/writing
* Binary encoding/decoding
* Validation

TODO:

* Tests
* DataCount section
* Proper encoding of passive element segments (using ref.null etc)
* Evaluation
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants