[PATCH] D34801: [coverage] Make smaller regions for the first case of a switch.

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In https://reviews.llvm.org/D34801#828382, @efriedma wrote:

> I'm going to look over the overall algorithm one more time to make sure this direction makes sense. The current code for ending regions on break/continue/return/noreturn doesn't really work well,


Maybe it's worth taking a step back and finding a way to fix that. As you alluded to with the FIXME, there are still some basic cases the current patch won't address:

  switch (x) {
    case 1: // Region set to end at the end of the second break.
    case 2:
      break;
    case 3:
    case 4:
      break;
  }



> and the way we handle multiple consecutive case statments isn't really right,

I agree. It'd be worth seeing whether we can avoid pushing a new region for a case if we know fallthrough occurs.

> and I want to make sure I'm not going to end up reverting this.  And I need to run some tests to make sure I'm not introducing any crashes.  If nothing is wrong, I'll commit tomorrow.

I suspect that fixing switch/cases in a more general way would require reverting this and I know I'll revisit this in the coming weeks. Still, this is an improvement and if nothing goes wrong during testing your plan sgtm.


Repository:
  rL LLVM

https://reviews.llvm.org/D34801