Throw Expressions should (probably) insert a new block

[mmha]

Test cases:

void statements() {
  throw 0;
  123 + 456;
}

void paren_expr() {
  (throw 0, 123 + 456);
}

Both fail during verification because CIRGenItaniumCXXABI::emitThrow emits a cir.unreachable after a cir.throw and we expect cir.unreachable to be the last operation within a block.

This is unlike __builtin_unreachable() which inserts a new block:

void statements() {
  __builtin_unreachable();
  123 + 456;
}

void paren_expr() {
  (__builtin_unreachable(), 123 + 456);
}

  cir.func @_Z10statementsv() extra(#fn_attr) {
    cir.unreachable loc(#loc3)
  ^bb1:  // no predecessors
    %0 = cir.const #cir.int<123> : !s32i loc(#loc4)
    %1 = cir.const #cir.int<456> : !s32i loc(#loc5)
    %2 = cir.binop(add, %0, %1) nsw : !s32i loc(#loc4)
    cir.return loc(#loc2)
  } loc(#loc11)
  cir.func @_Z10paren_exprv() extra(#fn_attr) {
    cir.unreachable loc(#loc8)
  ^bb1:  // no predecessors
    %0 = cir.const #cir.int<123> : !s32i loc(#loc9)
    %1 = cir.const #cir.int<456> : !s32i loc(#loc10)
    %2 = cir.binop(add, %0, %1) nsw : !s32i loc(#loc9)
    cir.return loc(#loc7)
  } loc(#loc12)

[bcardosolopes]

(cc @bruteforceboy)

[bruteforceboy]

(cc @bruteforceboy)

I will have a look

[bruteforceboy]

@bcardosolopes a very simple solution to this issue is extending the logic of splitting blocks from emitRethrow to emitThrow. So, we have the UnreachableOp as the terminator for the current block and create a new block for the remaining operations, just like in PR#1343. I've tested the examples with this and it seems to work fine. If you agree with this and do not see any pitfalls, I can work on this issue and extend the implementation to work for emitThrow also. cc: @mmha

[mmha]

That sounds sensible to me. Can the insertion of blocks into an expression have any consequences for sequencing?

[bruteforceboy]

That sounds sensible to me. Can the insertion of blocks into an expression have any consequences for sequencing?

There shouldn't be any problems if implemented properly. I will wait for @bcardosolopes's confirmation before continuing with the issue.

[bcardosolopes]

If you agree with this and do not see any pitfalls, I can work on this issue and extend the implementation to work for emitThrow also

Sounds great!