[tlively]

In Binaryen IR, Ifs with unreachable conditions are unreachable even if the arms have concrete types, whereas Blocks keep the concrete type of their final element even when an earlier element is unreachable. Unreachable Ifs can validly appear as the reference operand of an instruction like array.set or struct.set, even if their concrete arms have unrelated types.

In OptimizeInstructions::trapOnNull, under --traps-never-happen, when one arm of an If flowed out a null reference, the If was replaced with a drop of the condition followed by the other arm. This created invalid IR when the condition was unreachable and the other arm had an unrelated type. Fix the problem by only doing this optimization when the If condition is not unreachable.

[gemini-code-assist]

Code Review

This pull request prevents the optimization of If instructions when the condition is unreachable to avoid generating invalid intermediate representation, and adds a corresponding test case. The reviewer suggests defensively combining the null check for iff->ifFalse with the condition check instead of using an assertion, which improves robustness against unexpected IR states.

[gemini-code-assist]

Instead of asserting that iff->ifFalse is non-null, we can defensively combine the null check into the if condition. This is safer and avoids potential assertion failures in environments with unexpected or intermediate IR states.

        if (iff->condition->type != Type::unreachable && iff->ifFalse) {

[stevenfontanella]

I think I'm missing context here. Why must an if whose condition isn't unreachable have an else? Can't we have an if without an else?

[tlively]

We know that the If here has a concrete output type because it will flow into some instruction that will do an implicit null check on that output. In Binaryen IR, an If can only have a concrete output type if it has two arms. A single-arm if always has type Type::none or Type::unreachable. (If you tried to have a concrete-typed single-arm If, what value would it produce when the condition is false?)

Notably, Wasm can have concretely typed single-arm ifs when the if takes input parameters that match its results. Binaryen IR doesn't support input parameters, though.