Type of br_if should be the target's type, not the value

[kripken]

(block $target (result ..type..)
  (br_if $target
    (..value..)
    (..condition..)
  )
)

The type of the br_if should be the type of $target, not of ..value... Binaryen gets this wrong while v8, wabt, and wasp get it right.

In practice the return value of br_if is not that useful - I've only ever seen it dropped, except in fuzzer-generated code. That's probably why this never came up. (The fuzzer found it now.)

For that reason I'm not sure it's worth fixing. Another reason is that this is parallel to WebAssembly/gc#201 where there is discussion on making local.tee's type that of the value and not the local, which is basically the same issue. If we make that change then Binaryen is already doing the right thing.

[aheejin]

I agree; I encountered this problem in Jan 2020 and tried to fix it by giving a type every time we make br_if, but it was a lot harder than giving a type local.tee. Giving every makeBreak a type was not sufficient; there were other passes that changed/gave a type to br_if, such as RemoveUnusedBrs. Making what we're already doing correct sounds good to me.

[tlively]

We now work around this in the binary writer by inserting casts after branches as necessary, so I'll go ahead and close this. We may need to revisit this strategy in the future, for example if we introduce new references that cannot be cast (continuation references are possibly already in this category), but we can discuss that in follow-on issues.