zequanwu added inline comments.

================
Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:942
     pushRegion(Counter::getZero());
-    auto &ZeroRegion = getRegion();
-    ZeroRegion.setDeferred(true);
-    LastTerminatedRegion = {EndLoc, RegionStack.size()};
+    if (!HasTerminateStmt) {
+      auto &ZeroRegion = getRegion();
----------------
vsk wrote:
> zequanwu wrote:
> > vsk wrote:
> > > What's supposed to be the difference between the zero region pushed after 
> > > a `return;` vs. after a `break;`?
> > What I think is `DeferredRegion` is only used for `break;` and `continue`. 
> > Other terminate statements like `return;`, `throw` etc will use the logic 
> > in `VisitStmt` to emit gap region. So, I added this condition to separate 
> > the two cases.
> What do you think of the notion of using the gaps inserted in VisitStmt to 
> replace the whole deferred region system? Is it something that might be 
> feasible (if perhaps out of scope for this patch), or do you see a 
> fundamental reason it can't/shouldn't be done?
I think it is feasible. For break and continue, they only affect the statements 
after them in the same block. For other terminate statements, they affect all 
the statements after them even outside the block, see example below. Also 
instead of emitting a zero gap region in VisitStmt, we need emit gap region 
with (previous counter - current statement counter).

```
while (cond) {
...
break; // This affects statements' count until the end of the while body.
...
}

while (cond) {
...
return; // This affects statements' count until the end of the function body.
...
}
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97101/new/

https://reviews.llvm.org/D97101

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to